Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Further implement ls-qpack, especially around stream data in/out #12

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 55 additions & 17 deletions ls-qpack/src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,36 @@ use std::pin::Pin;
/// Error during decoding operations.
pub struct DecoderError;

/// The result of the encoding operation.
///
/// This is the result of [`Encoder::encode_all`] or [`EncodingBlock::encode`].
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is called BuffersDecoded but doc mentions encode_all?

pub struct BuffersDecoded {
headers: Vec<Header>,
stream: Box<[u8]>,
}

impl BuffersDecoded {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would improve this `impl.

Before it was possible to obtain ownership of data. With BuffersDecoded is not possible anymore.

I'd start adding a take method to consume the object and get the memory ownership:

/// The data buffer of decoded headers.
pub fn headers(&self) -> &Vec<Header> {
&self.headers
}

/// The buffer of the stream data for the encoder.
pub fn stream(&self) -> &[u8] {
&self.stream
}
}

/// The result of a decode operation.
///
/// Generally, this is function's output for [`Decoder::decode`].
///
/// When header data are decoded,
pub enum DecoderOutput {
/// The header block has been correctly decoded.
Done(Vec<Header>),
Done(BuffersDecoded),

/// The deocding stream is blocked.
/// The decoding stream is blocked.
/// More data are needed in order to proceed with decoding operation.
/// Generally, you need to feed the encoder via [`Decoder::feed`].
BlockedStream,
Expand All @@ -55,7 +75,7 @@ pub enum DecoderOutput {
impl DecoderOutput {
/// If the result is unblocked, it will return `Some(Vec<header>)`.
/// Otherwise `None`.
pub fn take(self) -> Option<Vec<Header>> {
pub fn take(self) -> Option<BuffersDecoded> {
match self {
Self::Done(v) => Some(v),
Self::BlockedStream => None,
Expand Down Expand Up @@ -181,6 +201,9 @@ impl InnerDecoder {
let header_block_len = encoded_cursor.len();
let mut cursor_after = encoded_cursor.as_ptr();

let mut buffer = vec![0; ls_qpack_sys::LSQPACK_LONGEST_SDTC as usize];
let mut sdtc_buffer_size = buffer.len();

let result = unsafe {
ls_qpack_sys::lsqpack_dec_header_in(
&mut this.decoder,
Expand All @@ -189,8 +212,8 @@ impl InnerDecoder {
header_block_len,
&mut cursor_after,
encoded_cursor_len,
std::ptr::null_mut(),
&mut 0,
buffer.as_mut_ptr(),
&mut sdtc_buffer_size,
)
};

Expand All @@ -200,25 +223,23 @@ impl InnerDecoder {
debug_assert!(!hblock_ctx.as_ref().is_error());

let hblock_ctx = unsafe { Pin::into_inner_unchecked(hblock_ctx) };
Ok(DecoderOutput::Done(hblock_ctx.decoded_headers()))

buffer.truncate(sdtc_buffer_size);
Ok(DecoderOutput::Done(BuffersDecoded { headers: hblock_ctx.decoded_headers(), stream: buffer.into_boxed_slice()}))
}

ls_qpack_sys::lsqpack_read_header_status_LQRHS_BLOCKED => {
ls_qpack_sys::lsqpack_read_header_status_LQRHS_BLOCKED | ls_qpack_sys::lsqpack_read_header_status_LQRHS_NEED => {
let offset = unsafe {
cursor_after.offset_from(hblock_ctx.as_ref().encoded_cursor().as_ptr())
};

debug_assert!(offset > 0);

hblock_ctx.as_mut().advance_cursor(offset as usize);
hblock_ctx.as_mut().set_blocked(true);
this.header_blocks.insert(stream_id, hblock_ctx);

Ok(DecoderOutput::BlockedStream)
}

ls_qpack_sys::lsqpack_read_header_status_LQRHS_NEED => unimplemented!(),

_ => Err(DecoderError),
}
}
Expand Down Expand Up @@ -258,7 +279,7 @@ impl InnerDecoder {
}

let hdbk = unsafe { Pin::into_inner_unchecked(hdbk) };
Some(Ok(DecoderOutput::Done(hdbk.decoded_headers())))
Some(Ok(DecoderOutput::Done(BuffersDecoded { headers: hdbk.decoded_headers(), stream: hdbk.stream_data().into_boxed_slice()})))
}

hash_map::Entry::Vacant(_) => None,
Expand Down Expand Up @@ -295,6 +316,7 @@ mod callbacks {
header: ls_qpack_sys::lsxpack_header,
blocked: bool,
error: bool,
stream_data: Vec<u8>,
decoded_headers: Vec<Header>,
_marker: PhantomPinned,
}
Expand All @@ -311,6 +333,7 @@ mod callbacks {
encoded_data,
encoded_data_offset: 0,
decoding_buffer: Vec::new(),
stream_data: Vec::new(),
header: Default::default(),
blocked: false,
error: false,
Expand Down Expand Up @@ -339,6 +362,11 @@ mod callbacks {
this.blocked = blocked;
}

pub(super) fn set_stream_data(self: Pin<&mut Self>, data: &Vec<u8>) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why passing &Vec<u8>? If you need ownership, just pass Vec<u8>

let this = unsafe { self.get_unchecked_mut() };
this.stream_data = data.to_vec();
}

pub(super) fn enable_error(self: Pin<&mut Self>) {
let this = unsafe { self.get_unchecked_mut() };
debug_assert!(!this.error);
Expand All @@ -353,8 +381,12 @@ mod callbacks {
self.error
}

pub(super) fn decoded_headers(self) -> Vec<Header> {
self.decoded_headers
pub(super) fn decoded_headers(&self) -> Vec<Header> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change self -> &self?

self.decoded_headers.clone()
}

pub(super) fn stream_data(&self) -> Vec<u8> {
self.stream_data.clone()
}

unsafe fn from_void_ptr(ptr: *mut libc::c_void) -> Pin<&'static mut Self> {
Expand Down Expand Up @@ -410,19 +442,25 @@ mod callbacks {
let encoded_cursor_len = encoded_cursor.len();
let mut cursor_after = encoded_cursor.as_ptr();

let mut buffer = vec![0; ls_qpack_sys::LSQPACK_LONGEST_SDTC as usize];
let mut sdtc_buffer_size = buffer.len();

let result = unsafe {
ls_qpack_sys::lsqpack_dec_header_read(
hblock_ctx.decoder,
hblock_ctx.as_mut().as_mut_ptr() as *mut libc::c_void,
&mut cursor_after,
encoded_cursor_len,
std::ptr::null_mut(),
std::ptr::null_mut(),
buffer.as_mut_ptr(),
&mut sdtc_buffer_size,
)
};

match result {
ls_qpack_sys::lsqpack_read_header_status_LQRHS_DONE => {}
ls_qpack_sys::lsqpack_read_header_status_LQRHS_DONE => {
buffer.truncate(sdtc_buffer_size);
hblock_ctx.as_mut().set_stream_data(&buffer);
}

ls_qpack_sys::lsqpack_read_header_status_LQRHS_BLOCKED => {
let offset = unsafe {
Expand Down
22 changes: 22 additions & 0 deletions ls-qpack/src/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,14 @@ impl Encoder {
EncodingBlock::new(self, stream_id, seqno)
}

/// Feeds data from decoder's buffer stream.
pub fn feed<D>(&mut self, data: D) -> Result<(), EncoderError>
where
D: AsRef<[u8]>,
{
self.inner.as_mut().feed_decoder_data(data.as_ref())
}

/// Return estimated compression ratio until this point.
///
/// Compression ratio is defined as size of the output divided by the size of the
Expand Down Expand Up @@ -423,6 +431,20 @@ impl InnerEncoder {
}
}

fn feed_decoder_data(self: Pin<&mut Self>, data: &[u8]) -> Result<(), EncoderError> {
let this = unsafe { self.get_unchecked_mut() };

let result = unsafe {
ls_qpack_sys::lsqpack_enc_decoder_in(&mut this.encoder, data.as_ptr(), data.len())
};

if result == 0 {
Ok(())
} else {
Err(EncoderError)
}
}

fn ratio(self: Pin<&Self>) -> f32 {
unsafe { ls_qpack_sys::lsqpack_enc_ratio(&self.encoder) }
}
Expand Down
1 change: 1 addition & 0 deletions ls-qpack/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub enum HeaderError {
/// QPACK Header.
///
/// A `Header` essentially is a pair composed by two strings. That is, a name and a value.
#[derive(Clone)]
pub struct Header {
buffer: Box<[u8]>,
name_offset: usize,
Expand Down