From 4874d1f0bfeded6d568d85ff4a3664d14d833bb2 Mon Sep 17 00:00:00 2001 From: Vlad Krasnov Date: Thu, 16 Nov 2023 16:40:18 -0500 Subject: [PATCH] Initial QPACK dynamic table decode This change adds a basic dynamic table support for QPACK decoder. It only implements the table without the blocking. Still in many cases it allows for much better compression than the static only table. --- apps/src/common.rs | 5 +- fuzz/src/qpack_decode.rs | 8 +- quiche/examples/qpack-decode.rs | 4 +- quiche/src/h3/mod.rs | 117 +++-- quiche/src/h3/qpack/decoder.rs | 735 ++++++++++++++++++++++++++++---- quiche/src/h3/qpack/encoder.rs | 2 +- quiche/src/h3/qpack/mod.rs | 26 +- quiche/src/h3/stream.rs | 17 +- quiche/src/stream/mod.rs | 2 +- 9 files changed, 770 insertions(+), 146 deletions(-) diff --git a/apps/src/common.rs b/apps/src/common.rs index 746e27a7b6..3139a82c8d 100644 --- a/apps/src/common.rs +++ b/apps/src/common.rs @@ -734,12 +734,11 @@ fn make_h3_config( } if let Some(v) = qpack_max_table_capacity { - // quiche doesn't support dynamic QPACK, so clamp to 0 for now. - config.set_qpack_max_table_capacity(v.clamp(0, 0)); + config.set_qpack_max_table_capacity(v); } if let Some(v) = qpack_blocked_streams { - // quiche doesn't support dynamic QPACK, so clamp to 0 for now. + // quiche doesn't support dynamic QPACK blocking, so clamp to 0 for now. config.set_qpack_blocked_streams(v.clamp(0, 0)); } diff --git a/fuzz/src/qpack_decode.rs b/fuzz/src/qpack_decode.rs index 69fdf9c290..1664864dad 100644 --- a/fuzz/src/qpack_decode.rs +++ b/fuzz/src/qpack_decode.rs @@ -13,10 +13,10 @@ use quiche::h3::NameValue; // input. However, that transformation is not guaranteed to be the identify // function, as there are multiple ways the same hdr list could be encoded. fuzz_target!(|data: &[u8]| { - let mut decoder = quiche::h3::qpack::Decoder::new(); + let mut decoder = quiche::h3::qpack::Decoder::new(0); let mut encoder = quiche::h3::qpack::Encoder::new(); - let hdrs = match decoder.decode(&mut data.to_vec(), u64::MAX) { + let hdrs = match decoder.decode(&mut data.to_vec(), u64::MAX, 0) { Err(_) => return, Ok(hdrs) => hdrs, }; @@ -25,7 +25,7 @@ fuzz_target!(|data: &[u8]| { let encoded_size = encoder.encode(&hdrs, &mut encoded_hdrs).unwrap(); let decoded_hdrs = decoder - .decode(&encoded_hdrs[..encoded_size], u64::MAX) + .decode(&encoded_hdrs[..encoded_size], u64::MAX, 0) .unwrap(); let mut expected_hdrs = Vec::new(); @@ -35,7 +35,7 @@ fuzz_target!(|data: &[u8]| { for h in &hdrs { let name = h.name().to_ascii_lowercase(); - expected_hdrs.push(quiche::h3::Header::new(&name, h.value())); + expected_hdrs.push(quiche::h3::Header::new(name, h.value())); } assert_eq!(expected_hdrs, decoded_hdrs) diff --git a/quiche/examples/qpack-decode.rs b/quiche/examples/qpack-decode.rs index 6d1cbf42a6..012cc7c37e 100644 --- a/quiche/examples/qpack-decode.rs +++ b/quiche/examples/qpack-decode.rs @@ -49,7 +49,7 @@ fn main() { let mut file = File::open(args.next().unwrap()).unwrap(); - let mut dec = qpack::Decoder::new(); + let mut dec = qpack::Decoder::new(0); loop { let mut stream_id: [u8; 8] = [0; 8]; @@ -76,7 +76,7 @@ fn main() { continue; } - for hdr in dec.decode(&data[..len], u64::MAX).unwrap() { + for hdr in dec.decode(&data[..len], u64::MAX, 0).unwrap() { let name = std::str::from_utf8(hdr.name()).unwrap(); let value = std::str::from_utf8(hdr.value()).unwrap(); println!("{name}\t{value}"); diff --git a/quiche/src/h3/mod.rs b/quiche/src/h3/mod.rs index 47c5d62c23..0491299f67 100644 --- a/quiche/src/h3/mod.rs +++ b/quiche/src/h3/mod.rs @@ -312,6 +312,8 @@ use qlog::events::EventImportance; #[cfg(feature = "qlog")] use qlog::events::EventType; +use crate::Shutdown; + /// List of ALPN tokens of supported HTTP/3 versions. /// /// This can be passed directly to the [`Config::set_application_protos()`] @@ -570,6 +572,11 @@ pub trait NameValue { /// Returns the object's value. fn value(&self) -> &[u8]; + + /// Return the qpack cost of the pair + fn qpack_cost(&self) -> u64 { + self.name().len() as u64 + self.value().len() as u64 + 32 + } } impl NameValue for (N, V) @@ -611,8 +618,11 @@ impl Header { /// Creates a new header. /// /// Both `name` and `value` will be cloned. - pub fn new(name: &[u8], value: &[u8]) -> Self { - Self(name.to_vec(), value.to_vec()) + pub fn new(name: N, value: V) -> Self + where + Vec: From + From, + { + Self(name.into(), value.into()) } } @@ -881,7 +891,9 @@ impl Connection { peer_control_stream_id: None, qpack_encoder: qpack::Encoder::new(), - qpack_decoder: qpack::Decoder::new(), + qpack_decoder: qpack::Decoder::new( + config.qpack_max_table_capacity.unwrap_or(0), + ), local_qpack_streams: QpackStreams { encoder_stream_id: None, @@ -1590,6 +1602,25 @@ impl Connection { }; } + // Send any outstanding QPACK decoder instructions + if let Some(stream_id) = self.local_qpack_streams.decoder_stream_id { + let mut buf = [0u8; 64]; + + while self.qpack_decoder.has_instructions() { + let cap = conn.stream_capacity(stream_id)?; + if cap == 0 { + break; + } + + let n = self.qpack_decoder.emit_instructions(&mut buf); + if n == 0 { + break; + } + + conn.stream_send(stream_id, &buf[..n], false)?; + } + } + // Process finished streams list. if let Some(finished) = self.finished_streams.pop_front() { return Ok((finished, Event::Finished)); @@ -1606,8 +1637,10 @@ impl Connection { // Return early if the stream was reset, to avoid returning // a Finished event later as well. - Err(Error::TransportError(crate::Error::StreamReset(e))) => - return Ok((s, Event::Reset(e))), + Err(Error::TransportError(crate::Error::StreamReset(e))) => { + self.qpack_decoder.cancel_stream(s); + return Ok((s, Event::Reset(e))); + }, Err(e) => return Err(e), }; @@ -2296,8 +2329,19 @@ impl Connection { return Ok((stream_id, Event::Data)); }, - stream::State::QpackInstruction => { - let mut d = [0; 4096]; + stream::State::QpackEncoderInstruction => { + let mut d = [0; 1024]; + + loop { + let (n, _) = conn.stream_recv(stream_id, &mut d)?; + self.qpack_decoder + .control(&d[..n]) + .map_err(|_| Error::QpackDecompressionFailed)?; + } + }, + + stream::State::QpackDecoderInstruction => { + let mut d = [0; 1024]; // Read data from the stream and discard immediately. loop { @@ -2423,10 +2467,11 @@ impl Connection { .max_field_section_size .unwrap_or(u64::MAX); - let headers = match self - .qpack_decoder - .decode(&header_block[..], max_size) - { + let headers = match self.qpack_decoder.decode( + &header_block[..], + max_size, + stream_id, + ) { Ok(v) => v, Err(e) => { @@ -2713,6 +2758,23 @@ impl Connection { Err(Error::Done) } + + /// Shuts down reading or writing from/to the specified stream. This method + /// should be preferred over the equivalent `quiche::stream_shutdown` + /// method, as the other method may prevent evictions from the QPACK + /// decoder. + pub fn stream_shutdown( + &mut self, conn: &mut super::Connection, stream_id: u64, + direction: Shutdown, err: u64, + ) -> Result<()> { + let is_read = direction == Shutdown::Read; + conn.stream_shutdown(stream_id, direction, err)?; + if is_read { + self.qpack_decoder.cancel_stream(stream_id); + } + + Ok(()) + } } /// Generates an HTTP/3 GREASE variable length integer. @@ -4471,39 +4533,6 @@ mod tests { assert!(qpack_stream_closed); } - #[test] - /// Client sends QPACK data. - fn qpack_data() { - // TODO: QPACK instructions are ignored until dynamic table support is - // added so we just test that the data is safely ignored. - let mut s = Session::new().unwrap(); - s.handshake().unwrap(); - - let e_stream_id = s.client.local_qpack_streams.encoder_stream_id.unwrap(); - let d_stream_id = s.client.local_qpack_streams.decoder_stream_id.unwrap(); - let d = [0; 20]; - - s.pipe.client.stream_send(e_stream_id, &d, false).unwrap(); - s.advance().ok(); - - s.pipe.client.stream_send(d_stream_id, &d, false).unwrap(); - s.advance().ok(); - - loop { - match s.server.poll(&mut s.pipe.server) { - Ok(_) => (), - - Err(Error::Done) => { - break; - }, - - Err(_) => { - panic!(); - }, - } - } - } - #[test] /// Tests limits for the stream state buffer maximum size. fn max_state_buf_size() { diff --git a/quiche/src/h3/qpack/decoder.rs b/quiche/src/h3/qpack/decoder.rs index 5ab9d1ad99..a13714e565 100644 --- a/quiche/src/h3/qpack/decoder.rs +++ b/quiche/src/h3/qpack/decoder.rs @@ -24,10 +24,18 @@ // NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +use std::borrow::Cow; +use std::collections::VecDeque; + +use super::encoder::encode_int; use super::Error; use super::Result; +use super::INSERT_WITH_LITERAL_NAME; +use super::INSERT_WITH_NAME_REF; +use super::SET_DYNAMIC_TABLE_CAPACITY; use crate::h3::Header; +use crate::h3::NameValue; use super::INDEXED; use super::INDEXED_WITH_POST_BASE; @@ -65,60 +73,348 @@ impl Representation { } } +#[derive(Clone, Copy, Debug, PartialEq)] +enum EncoderInstruction { + SetDynamicTableCapacity, + InsertWithNameRef, + InsertWithLiteralName, + Duplicate, +} + +impl EncoderInstruction { + pub fn from_byte(b: u8) -> EncoderInstruction { + if b & INSERT_WITH_NAME_REF == INSERT_WITH_NAME_REF { + return EncoderInstruction::InsertWithNameRef; + } + + if b & INSERT_WITH_LITERAL_NAME == INSERT_WITH_LITERAL_NAME { + return EncoderInstruction::InsertWithLiteralName; + } + + if b & SET_DYNAMIC_TABLE_CAPACITY == SET_DYNAMIC_TABLE_CAPACITY { + return EncoderInstruction::SetDynamicTableCapacity; + } + + EncoderInstruction::Duplicate + } +} + /// A QPACK decoder. #[derive(Default)] -pub struct Decoder {} +pub struct Decoder { + dynamic_table: VecDeque<(Cow<'static, [u8]>, Vec)>, + /// The number of insertions into the table + insert_cnt: u64, + /// The number of entries removed from the table + base: u64, + /// The computed size of the table as per rfc9204 + tbl_sz: u64, + /// The current capacity requested by the peer + capacity: u64, + /// The capacity limit imposed by settings + max_capacity: u64, + + /// The value of `insert_cnt` last time "Insert Count Increment" was emmited + last_acked_insert: u64, + /// A list of streams pending "Section Acknowledgment" + unacked_sections: VecDeque, + /// A list of streams pending "Stream Cancellation" + unacked_cancellations: VecDeque, + + /// Internal buffer in case the decoder receives a partial buffer + inner_buffer: Vec, +} impl Decoder { /// Creates a new QPACK decoder. - pub fn new() -> Decoder { - Decoder::default() + pub fn new(max_capacity: u64) -> Decoder { + Decoder { + max_capacity: max_capacity.min(u32::MAX as u64), + ..Decoder::default() + } + } + + /// Check if the decoder wants to emit any instructions on the instruction + /// stream + pub fn has_instructions(&self) -> bool { + self.last_acked_insert != self.insert_cnt || + !self.unacked_sections.is_empty() || + !self.unacked_cancellations.is_empty() + } + + /// Emit any pending instructions on the instruction stream, once emitted + /// the buffer must be fully sent to the peer, or else the encoder and + /// decoder may get out of sync. + pub fn emit_instructions(&mut self, buf: &mut [u8]) -> usize { + const INSERT_CNT_INC: u8 = 0x00; + const SECTION_ACK: u8 = 0x80; + const STREAM_CANCEL: u8 = 0x40; + + let mut b = octets::OctetsMut::with_slice(buf); + + let inc_req_count = self.insert_cnt - self.last_acked_insert; + + if inc_req_count > 0 && + encode_int(inc_req_count, INSERT_CNT_INC, 6, &mut b).is_ok() + { + self.last_acked_insert = self.insert_cnt; + } + + while let Some(section) = self.unacked_sections.front() { + if encode_int(*section, SECTION_ACK, 7, &mut b).is_ok() { + self.unacked_sections.pop_front(); + } else { + break; + } + } + + if self.capacity > 0 { + // Those notifications MAY be ommited if table size is 0, so omit them + while let Some(section) = self.unacked_cancellations.front() { + if encode_int(*section, STREAM_CANCEL, 6, &mut b).is_ok() { + self.unacked_cancellations.pop_front(); + } else { + break; + } + } + } else { + self.unacked_cancellations.clear(); + } + + b.off() + } + + pub fn cancel_stream(&mut self, stream: u64) { + self.unacked_cancellations.push_back(stream); + } + + fn process_control(&mut self, b: &mut octets::Octets) -> Result<()> { + let first = b.peek_u8()?; + + match EncoderInstruction::from_byte(first) { + EncoderInstruction::SetDynamicTableCapacity => { + let capacity = decode_int(b, 5)?; + + trace!("SetDynamicTableCapacity size={capacity}"); + + self.resize_table(capacity)?; + }, + + EncoderInstruction::InsertWithNameRef => { + let is_static = first & 0x40 == 0x40; + let idx = decode_int(b, 6)?; + + let value = decode_str(b, 7)?; + + trace!("InsertWithNameRef index={idx} static={is_static} value={value:?}"); + + if is_static { + let (name, _) = lookup_static(idx)?; + self.insert((Cow::Borrowed(name), value))?; + } else { + let (name, _) = self.lookup_dynamic( + self.insert_cnt + .checked_sub(idx + 1) + .ok_or(Error::InvalidDynamicTableIndex)?, + )?; + self.insert((Cow::Owned(name.to_vec()), value))?; + } + }, + + EncoderInstruction::InsertWithLiteralName => { + let name = decode_str(b, 5)?; + let value = decode_str(b, 7)?; + + trace!("InsertWithLiteralName name={name:?} value={value:?}"); + + self.insert((Cow::Owned(name), value))?; + }, + + EncoderInstruction::Duplicate => { + let idx = decode_int(b, 5)?; + + trace!("Duplicate index={idx}"); + + let (name, value) = self.lookup_dynamic( + self.insert_cnt + .checked_sub(idx + 1) + .ok_or(Error::InvalidDynamicTableIndex)?, + )?; + + self.insert((Cow::Owned(name.to_vec()), value.to_vec()))?; + }, + } + + Ok(()) } /// Processes control instructions from the encoder. - pub fn control(&mut self, _buf: &mut [u8]) -> Result<()> { - // TODO: process control instructions + pub fn control(&mut self, buf: &[u8]) -> Result<()> { + let mut maybe_buf = Vec::new(); + + let mut b = if !self.inner_buffer.is_empty() { + // Have a buffered partial instruction, should concatenate the + // provided buffer and try again + std::mem::swap(&mut maybe_buf, &mut self.inner_buffer); + maybe_buf.extend_from_slice(buf); + octets::Octets::with_slice(maybe_buf.as_slice()) + } else { + octets::Octets::with_slice(buf) + }; + + while b.cap() > 0 { + let pos = b.off(); + match self.process_control(&mut b) { + Ok(_) => {}, + Err(Error::BufferTooShort) => { + // Have partial instruction, have to buffer it now + self.inner_buffer.extend_from_slice(&b.buf()[pos..]); + return Ok(()); + }, + Err(err) => return Err(err), + } + } + + Ok(()) + } + + /// Evict the oldest entry in the table + fn evict_one(&mut self) -> Result<()> { + let entry = self + .dynamic_table + .pop_front() + .ok_or(Error::DynamicTableTooBig)?; + + self.tbl_sz -= entry.qpack_cost(); + self.base += 1; + Ok(()) + } + + fn insert(&mut self, entry: (Cow<'static, [u8]>, Vec)) -> Result<()> { + self.tbl_sz += entry.qpack_cost(); + + while self.tbl_sz > self.capacity { + self.evict_one()?; + } + + self.dynamic_table.push_back(entry); + self.insert_cnt += 1; + + trace!("Insert insert_cnt={} size={}", self.insert_cnt, self.tbl_sz); + Ok(()) } + fn resize_table(&mut self, new_capacity: u64) -> Result<()> { + if new_capacity > self.max_capacity { + return Err(Error::DynamicTableTooBig); + } + + self.capacity = new_capacity; + while self.tbl_sz > self.capacity { + self.evict_one()?; + } + + Ok(()) + } + + fn lookup_dynamic(&self, idx: u64) -> Result<(&[u8], &[u8])> { + let idx = idx + .checked_sub(self.base) + .ok_or(Error::InvalidDynamicTableIndex)?; + + self.dynamic_table + .get(idx as usize) + .ok_or(Error::InvalidDynamicTableIndex) + .map(|(n, v)| (&n[..], &v[..])) + } + + fn decode_insert_cnt_and_base( + &self, b: &mut octets::Octets, + ) -> Result<(u64, u64)> { + let mut req_insert_count = decode_int(b, 8)?; + if req_insert_count != 0 { + let max_entries = self.max_capacity / 32; + let full_range = max_entries * 2; + if req_insert_count > full_range { + return Err(Error::InvalidDynamicTableIndex); + } + + let max_value = self.insert_cnt + max_entries; + + let max_wrapped = (max_value / full_range) * full_range; + + req_insert_count = max_wrapped + req_insert_count - 1; + + // If req_insert_count exceeds max_value, the Encoder's value must + // have wrapped one fewer time + if req_insert_count > max_value { + if req_insert_count <= full_range { + return Err(Error::InvalidDynamicTableIndex); + } + req_insert_count -= full_range; + } + + // Value of 0 must be encoded as 0. + if req_insert_count == 0 { + return Err(Error::InvalidDynamicTableIndex); + } + } + + let delta_negative = b.peek_u8()? & 0x80 == 0x80; + let delta = decode_int(b, 7)?; + + let base = if delta_negative { + req_insert_count + .checked_sub(delta + 1) + .ok_or(Error::InvalidDynamicTableIndex)? + } else { + req_insert_count + delta + }; + + Ok((req_insert_count as u64, base as u64)) + } + /// Decodes a QPACK header block into a list of headers. - pub fn decode(&mut self, buf: &[u8], max_size: u64) -> Result> { + pub fn decode( + &mut self, buf: &[u8], max_size: u64, stream_id: u64, + ) -> Result> { let mut b = octets::Octets::with_slice(buf); let mut out = Vec::new(); let mut left = max_size; - let req_insert_count = decode_int(&mut b, 8)?; - let base = decode_int(&mut b, 7)?; + let (req_insert_count, base) = self.decode_insert_cnt_and_base(&mut b)?; trace!("Header count={} base={}", req_insert_count, base); + if req_insert_count > self.insert_cnt { + return Err(Error::DynamicTableWouldBlock); + } + while b.cap() > 0 { let first = b.peek_u8()?; - match Representation::from_byte(first) { + let hdr = match Representation::from_byte(first) { Representation::Indexed => { const STATIC: u8 = 0x40; - let s = first & STATIC == STATIC; + let is_static = first & STATIC == STATIC; let index = decode_int(&mut b, 6)?; - trace!("Indexed index={} static={}", index, s); - - if !s { - // TODO: implement dynamic table - return Err(Error::InvalidHeaderValue); - } + trace!("Indexed index={} static={}", index, is_static); - let (name, value) = lookup_static(index)?; - - left = left - .checked_sub((name.len() + value.len()) as u64) - .ok_or(Error::HeaderListTooLarge)?; + let (name, value) = if !is_static { + self.lookup_dynamic( + base.checked_sub(index + 1) + .ok_or(Error::InvalidDynamicTableIndex)?, + )? + } else { + lookup_static(index)? + }; - let hdr = Header::new(name, value); - out.push(hdr); + Header::new(name, value) }, Representation::IndexedWithPostBase => { @@ -126,80 +422,64 @@ impl Decoder { trace!("Indexed With Post Base index={}", index); - // TODO: implement dynamic table - return Err(Error::InvalidHeaderValue); + let (name, value) = self.lookup_dynamic(base + index)?; + + Header::new(name, value) }, Representation::Literal => { - let name_huff = b.as_ref()[0] & 0x08 == 0x08; - let name_len = decode_int(&mut b, 3)? as usize; - - let mut name = b.get_bytes(name_len)?; - - let name = if name_huff { - super::huffman::decode(&mut name)? - } else { - name.to_vec() - }; + let name = decode_str(&mut b, 3)?; + let value = decode_str(&mut b, 7)?; - let name = name.to_vec(); - let value = decode_str(&mut b)?; + trace!("Literal Without Name Reference name={name:?} value={value:?}"); - trace!( - "Literal Without Name Reference name={:?} value={:?}", - name, - value, - ); - - left = left - .checked_sub((name.len() + value.len()) as u64) - .ok_or(Error::HeaderListTooLarge)?; - - // Instead of calling Header::new(), create Header directly - // from `name` and `value`, which are already String. - let hdr = Header(name, value); - out.push(hdr); + Header::new(name, value) }, Representation::LiteralWithNameRef => { const STATIC: u8 = 0x10; - let s = first & STATIC == STATIC; + let is_static = first & STATIC == STATIC; let name_idx = decode_int(&mut b, 4)?; - let value = decode_str(&mut b)?; + let value = decode_str(&mut b, 7)?; - trace!( - "Literal name_idx={} static={} value={:?}", - name_idx, - s, - value - ); - - if !s { - // TODO: implement dynamic table - return Err(Error::InvalidHeaderValue); - } + trace!("Literal name_idx={name_idx} static={is_static} value={value:?}"); - let (name, _) = lookup_static(name_idx)?; - - left = left - .checked_sub((name.len() + value.len()) as u64) - .ok_or(Error::HeaderListTooLarge)?; + let (name, _) = if !is_static { + self.lookup_dynamic( + base.checked_sub(name_idx + 1) + .ok_or(Error::InvalidDynamicTableIndex)?, + )? + } else { + lookup_static(name_idx)? + }; - // Instead of calling Header::new(), create Header directly - // from `value`, which is already String, but clone `name` - // as it is just a reference. - let hdr = Header(name.to_vec(), value); - out.push(hdr); + Header::new(name, value) }, Representation::LiteralWithPostBase => { - trace!("Literal With Post Base"); + let index = decode_int(&mut b, 3)?; + let value = decode_str(&mut b, 7)?; + + trace!( + "Literal With Post Base index={index} value={value:?}" + ); - // TODO: implement dynamic table - return Err(Error::InvalidHeaderValue); + let (name, _) = self.lookup_dynamic(base + index)?; + + Header::new(name, value) }, - } + }; + + left = left + .checked_sub((hdr.0.len() + hdr.1.len()) as u64) + .ok_or(Error::HeaderListTooLarge)?; + + out.push(hdr); + } + + if req_insert_count > 0 { + self.unacked_sections.push_back(stream_id); } Ok(out) @@ -245,12 +525,12 @@ fn decode_int(b: &mut octets::Octets, prefix: usize) -> Result { Err(Error::BufferTooShort) } -fn decode_str(b: &mut octets::Octets) -> Result> { +fn decode_str(b: &mut octets::Octets, prefix: usize) -> Result> { let first = b.peek_u8()?; - let huff = first & 0x80 == 0x80; + let huff = first & (1 << prefix) != 0; - let len = decode_int(b, 7)? as usize; + let len = decode_int(b, prefix)? as usize; let mut val = b.get_bytes(len)?; @@ -290,4 +570,297 @@ mod tests { assert_eq!(decode_int(&mut b, 8), Ok(42)); } + + #[test] + fn decode_dynamic1() { + let mut decoder = Decoder::new(300); + assert!(!decoder.has_instructions()); + + // Stream: Encoder + // 3fbd01 | Set Dynamic Table Capacity=220 + // c00f 7777 772e 6578 | Insert With Name Reference + // 616d 706c 652e 636f | Static Table, Index=0 + // 6d | (:authority=www.example.com) + // c10c 2f73 616d 706c | Insert With Name Reference + // 652f 7061 7468 | Static Table, Index=1 + // | (:path=/sample/path) + // + // Abs Ref Name Value + // ^-- acknowledged --^ + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // Size=106 + // + // Stream: 4 + // 0381 | Required Insert Count = 2, Base = 0 + // 10 | Indexed Field Line With Post-Base Index + // | Absolute Index = Base(0) + Index(0) = 0 + // | (:authority=www.example.com) + // 11 | Indexed Field Line With Post-Base Index + // | Absolute Index = Base(0) + Index(1) = 1 + // | (:path=/sample/path) + // + // Abs Ref Name Value + // ^-- acknowledged --^ + // 0 1 :authority www.example.com + // 1 1 :path /sample/path + // Size=106 + // + // Stream: Decoder + // 84 | Section Acknowledgment (stream=4) + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // ^-- acknowledged --^ + // Size=106 + + let mut decoder_stream = [0u8; 16]; + let encoder_stream = [ + 0x3f, 0xbd, 0x01, 0xc0, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65, 0x78, + 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0xc1, 0x0c, + 0x2f, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2f, 0x70, 0x61, 0x74, + 0x68, + ]; + + decoder.control(&encoder_stream).unwrap(); + + assert_eq!(decoder.insert_cnt, 2); + assert_eq!(decoder.tbl_sz, 106); + + let n = decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(n, 1); + assert_eq!(decoder_stream[0], 0x02); + + let enc_headers = [0x03, 0x81, 0x10, 0x11]; + let headers = decoder.decode(&enc_headers, u64::MAX, 4).unwrap(); + + assert_eq!(headers[0], Header::new(":authority", "www.example.com")); + assert_eq!(headers[1], Header::new(":path", "/sample/path")); + assert!(decoder.has_instructions()); + + let n = decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(n, 1); + assert_eq!(decoder_stream[0], 0x84); + } + + #[test] + fn decode_dynamic2() { + let mut decoder = Decoder::new(300); + let mut decoder_stream = [0u8; 16]; + + let encoder_stream = [ + 0x3f, 0xbd, 0x01, 0xc0, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65, 0x78, + 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0xc1, 0x0c, + 0x2f, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2f, 0x70, 0x61, 0x74, + 0x68, + ]; + + assert!(!decoder.has_instructions()); + decoder.control(&encoder_stream).unwrap(); + decoder.emit_instructions(&mut decoder_stream); + + // Stream: Encoder + // 4a63 7573 746f 6d2d | Insert With Literal Name + // 6b65 790c 6375 7374 | (custom-key=custom-value) + // 6f6d 2d76 616c 7565 | + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // ^-- acknowledged --^ + // 2 0 custom-key custom-value + // Size=160 + // + // Stream: Decoder + // 01 | Insert Count Increment (1) + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // 2 0 custom-key custom-value + // ^-- acknowledged --^ + // Size=160 + + let encoder_stream = [ + 0x4a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x6b, 0x65, 0x79, + 0x0c, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x76, 0x61, 0x6c, + 0x75, 0x65, + ]; + decoder.control(&encoder_stream).unwrap(); + + assert_eq!(decoder.insert_cnt, 3); + assert_eq!(decoder.tbl_sz, 160); + + let n = decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(n, 1); + assert_eq!(decoder_stream[0], 0x01); + + let chk_hdr = |idx: u64, n: &str, v: &str| -> bool { + let hdr = decoder.lookup_dynamic(idx).unwrap(); + hdr.0 == n.as_bytes() && hdr.1 == v.as_bytes() + }; + + assert!(chk_hdr(0, ":authority", "www.example.com")); + assert!(chk_hdr(1, ":path", "/sample/path")); + assert!(chk_hdr(2, "custom-key", "custom-value")); + } + + #[test] + fn decode_dynamic3() { + let mut decoder = Decoder::new(300); + let mut decoder_stream = [0u8; 16]; + + let encoder_stream = [ + 0x3f, 0xbd, 0x01, 0xc0, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65, 0x78, + 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0xc1, 0x0c, + 0x2f, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2f, 0x70, 0x61, 0x74, + 0x68, 0x4a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x6b, 0x65, + 0x79, 0x0c, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x76, 0x61, + 0x6c, 0x75, 0x65, + ]; + + assert!(!decoder.has_instructions()); + decoder.control(&encoder_stream).unwrap(); + decoder.emit_instructions(&mut decoder_stream); + + // Stream: Encoder + // 02 | Duplicate (Relative Index = 2) + // | Absolute Index = + // | Insert Count(3) - Index(2) - 1 = 0 + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // 2 0 custom-key custom-value + // ^-- acknowledged --^ + // 3 0 :authority www.example.com + // Size=217 + // + // Stream: 8 + // 0500 | Required Insert Count = 4, Base = 4 + // 80 | Indexed Field Line, Dynamic Table + // | Absolute Index = Base(4) - Index(0) - 1 = 3 + // | (:authority=www.example.com) + // c1 | Indexed Field Line, Static Table Index = 1 + // | (:path=/) + // 81 | Indexed Field Line, Dynamic Table + // | Absolute Index = Base(4) - Index(1) - 1 = 2 + // | (custom-key=custom-value) + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // 2 1 custom-key custom-value + // ^-- acknowledged --^ + // 3 1 :authority www.example.com + // Size=217 + // + // Stream: Decoder + // 48 | Stream Cancellation (Stream=8) + // + // Abs Ref Name Value + // 0 0 :authority www.example.com + // 1 0 :path /sample/path + // 2 0 custom-key custom-value + // ^-- acknowledged --^ + // 3 0 :authority www.example.com + // Size=217 + + let encoder_stream = [0x02]; + decoder.control(&encoder_stream).unwrap(); + decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(decoder.insert_cnt, 4); + assert_eq!(decoder.tbl_sz, 217); + + let chk_hdr = |idx: u64, n: &str, v: &str| -> bool { + let hdr = decoder.lookup_dynamic(idx).unwrap(); + hdr.0 == n.as_bytes() && hdr.1 == v.as_bytes() + }; + + assert!(chk_hdr(3, ":authority", "www.example.com")); + + let enc_headers = [0x05, 0x00, 0x80, 0xc1, 0x81]; + let headers = decoder.decode(&enc_headers, u64::MAX, 8).unwrap(); + + assert_eq!(headers[0], Header::new(":authority", "www.example.com")); + assert_eq!(headers[1], Header::new(":path", "/")); + assert_eq!(headers[2], Header::new("custom-key", "custom-value")); + + let n = decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(n, 1); + assert_eq!(decoder_stream[0], 0x88); + + assert!(!decoder.has_instructions()); + decoder.cancel_stream(8); + assert!(decoder.has_instructions()); + + let n = decoder.emit_instructions(&mut decoder_stream); + + assert_eq!(n, 1); + assert_eq!(decoder_stream[0], 0x48); + } + + #[test] + /// Test partial instructions are properly buffered + fn decode_dynamic4() { + let mut decoder = Decoder::new(300); + + let encoder_stream = [ + 0x3f, 0xbd, 0x01, 0xc0, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65, 0x78, + 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0xc1, 0x0c, + 0x2f, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2f, 0x70, 0x61, 0x74, + 0x68, 0x4a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x6b, 0x65, + 0x79, 0x0c, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x76, 0x61, + 0x6c, 0x75, 0x65, 0x02, + ]; + + assert!(!decoder.has_instructions()); + decoder.control(&encoder_stream[..7]).unwrap(); + decoder.control(&encoder_stream[7..31]).unwrap(); + decoder.control(&encoder_stream[31..]).unwrap(); + + let chk_hdr = |idx: u64, n: &str, v: &str| -> bool { + let hdr = decoder.lookup_dynamic(idx).unwrap(); + hdr.0 == n.as_bytes() && hdr.1 == v.as_bytes() + }; + + assert!(chk_hdr(0, ":authority", "www.example.com")); + assert!(chk_hdr(1, ":path", "/sample/path")); + assert!(chk_hdr(2, "custom-key", "custom-value")); + assert!(chk_hdr(3, ":authority", "www.example.com")); + } + + #[test] + /// Test entries are evicted + fn decode_dynamic5() { + let mut decoder = Decoder::new(200); + + let encoder_stream = [ + 0x3f, 0xa9, 0x01, 0xc0, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x65, 0x78, + 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, 0xc1, 0x0c, + 0x2f, 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2f, 0x70, 0x61, 0x74, + 0x68, 0x4a, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x6b, 0x65, + 0x79, 0x0c, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x2d, 0x76, 0x61, + 0x6c, 0x75, 0x65, 0x02, + ]; + + assert!(!decoder.has_instructions()); + decoder.control(&encoder_stream).unwrap(); + let chk_hdr = |idx: u64, n: &str, v: &str| -> bool { + let hdr = decoder.lookup_dynamic(idx).unwrap(); + hdr.0 == n.as_bytes() && hdr.1 == v.as_bytes() + }; + + assert_eq!(decoder.tbl_sz, 160); + assert!(decoder.lookup_dynamic(0).is_err()); + assert!(chk_hdr(1, ":path", "/sample/path")); + assert!(chk_hdr(2, "custom-key", "custom-value")); + assert!(chk_hdr(3, ":authority", "www.example.com")); + } } diff --git a/quiche/src/h3/qpack/encoder.rs b/quiche/src/h3/qpack/encoder.rs index d5b749910e..a93597073e 100644 --- a/quiche/src/h3/qpack/encoder.rs +++ b/quiche/src/h3/qpack/encoder.rs @@ -117,7 +117,7 @@ fn lookup_static(h: &T) -> Option<(u64, bool)> { None } -fn encode_int( +pub(crate) fn encode_int( mut v: u64, first: u8, prefix: usize, b: &mut octets::OctetsMut, ) -> Result<()> { let mask = 2u64.pow(prefix as u32) - 1; diff --git a/quiche/src/h3/qpack/mod.rs b/quiche/src/h3/qpack/mod.rs index 2cbcf2d3a4..090e25a3b3 100644 --- a/quiche/src/h3/qpack/mod.rs +++ b/quiche/src/h3/qpack/mod.rs @@ -31,6 +31,10 @@ const INDEXED_WITH_POST_BASE: u8 = 0b0001_0000; const LITERAL: u8 = 0b0010_0000; const LITERAL_WITH_NAME_REF: u8 = 0b0100_0000; +const SET_DYNAMIC_TABLE_CAPACITY: u8 = 0b0010_0000; +const INSERT_WITH_NAME_REF: u8 = 0b1000_0000; +const INSERT_WITH_LITERAL_NAME: u8 = 0b0100_0000; + /// A specialized [`Result`] type for quiche QPACK operations. /// /// This type is used throughout quiche's QPACK public API for any operation @@ -59,6 +63,16 @@ pub enum Error { /// The decoded header list exceeded the size limit. HeaderListTooLarge, + + /// The QPACK encountered an error related to the dynamic table. + InvalidDynamicTableIndex, + + /// The peer set the dynamic table capacity too high. + DynamicTableTooBig, + + /// The peer wants to use a dynamic table entry that was not inserted yet, + /// and would block the stream TODO: implement blocking + DynamicTableWouldBlock, } impl std::fmt::Display for Error { @@ -104,8 +118,8 @@ mod tests { let mut enc = Encoder::new(); assert_eq!(enc.encode(&headers, &mut encoded), Ok(240)); - let mut dec = Decoder::new(); - assert_eq!(dec.decode(&encoded, u64::MAX), Ok(headers)); + let mut dec = Decoder::new(0); + assert_eq!(dec.decode(&encoded, u64::MAX, 0), Ok(headers)); } #[test] @@ -132,8 +146,8 @@ mod tests { let mut enc = Encoder::new(); assert_eq!(enc.encode(&headers_in, &mut encoded), Ok(35)); - let mut dec = Decoder::new(); - let headers_out = dec.decode(&encoded, u64::MAX).unwrap(); + let mut dec = Decoder::new(0); + let headers_out = dec.decode(&encoded, u64::MAX, 0).unwrap(); assert_eq!(headers_expected, headers_out); @@ -149,8 +163,8 @@ mod tests { let mut enc = Encoder::new(); assert_eq!(enc.encode(&headers_in, &mut encoded), Ok(35)); - let mut dec = Decoder::new(); - let headers_out = dec.decode(&encoded, u64::MAX).unwrap(); + let mut dec = Decoder::new(0); + let headers_out = dec.decode(&encoded, u64::MAX, 0).unwrap(); assert_eq!(headers_expected, headers_out); } diff --git a/quiche/src/h3/stream.rs b/quiche/src/h3/stream.rs index f23bf34d52..830d38a7a9 100644 --- a/quiche/src/h3/stream.rs +++ b/quiche/src/h3/stream.rs @@ -80,8 +80,11 @@ pub enum State { /// Reading the push ID. PushId, - /// Reading a QPACK instruction. - QpackInstruction, + /// Reading a QPACK decoder instruction. + QpackDecoderInstruction, + + /// Reading a QPACK encoder instruction. + QpackEncoderInstruction, /// Reading and discarding data. Drain, @@ -218,10 +221,16 @@ impl Stream { Type::Push => State::PushId, - Type::QpackEncoder | Type::QpackDecoder => { + Type::QpackEncoder => { + self.remote_initialized = true; + + State::QpackEncoderInstruction + }, + + Type::QpackDecoder => { self.remote_initialized = true; - State::QpackInstruction + State::QpackDecoderInstruction }, Type::Unknown => State::Drain, diff --git a/quiche/src/stream/mod.rs b/quiche/src/stream/mod.rs index 1267d43cc4..c272385125 100644 --- a/quiche/src/stream/mod.rs +++ b/quiche/src/stream/mod.rs @@ -784,7 +784,7 @@ impl Eq for StreamPriorityKey {} impl PartialOrd for StreamPriorityKey { // Priority ordering is complex, disable Clippy warning. - #[allow(clippy::incorrect_partial_ord_impl_on_ord_type)] + #[allow(clippy::non_canonical_partial_ord_impl)] fn partial_cmp(&self, other: &Self) -> Option { // Ignore priority if ID matches. if self.id == other.id {