From 61b4f8fc34709b7cdfeaf91f3a7a527105c2026b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 20 Aug 2021 14:51:11 +0200 Subject: [PATCH] Support very large headers This completely refactors how headers are hpack-encoded. Instead of trying to be clever, constructing frames on the go while hpack-encoding, we just make a blob of all the hpack-encoded headers first, and then we split that blob in as many frames as necessary. --- src/codec/error.rs | 4 - src/codec/framed_write.rs | 22 +-- src/frame/headers.rs | 188 +++++++++++++----------- src/hpack/decoder.rs | 2 +- src/hpack/encoder.rs | 253 ++++++-------------------------- src/hpack/huffman/mod.rs | 33 ++--- src/hpack/mod.rs | 4 +- src/hpack/test/fixture.rs | 8 +- src/hpack/test/fuzz.rs | 31 +--- src/proto/streams/send.rs | 8 - tests/h2-support/src/prelude.rs | 1 + 11 files changed, 164 insertions(+), 390 deletions(-) diff --git a/src/codec/error.rs b/src/codec/error.rs index 5d66592..f505eb0 100644 --- a/src/codec/error.rs +++ b/src/codec/error.rs @@ -35,9 +35,6 @@ pub enum UserError { /// The payload size is too big PayloadTooBig, - /// A header size is too big - HeaderTooBig, - /// The application attempted to initiate too many streams to remote. Rejected, @@ -130,7 +127,6 @@ impl fmt::Display for UserError { InactiveStreamId => "inactive stream", UnexpectedFrameType => "unexpected frame type", PayloadTooBig => "payload too big", - HeaderTooBig => "header too big", Rejected => "rejected", ReleaseCapacityTooBig => "release capacity too big", OverflowedStreamId => "stream ID overflowed", diff --git a/src/codec/framed_write.rs b/src/codec/framed_write.rs index 30888c6..4b1b4ac 100644 --- a/src/codec/framed_write.rs +++ b/src/codec/framed_write.rs @@ -148,12 +148,6 @@ where match self.encoder.unset_frame() { ControlFlow::Continue => (), ControlFlow::Break => break, - ControlFlow::EndlessLoopHeaderTooBig => { - return Poll::Ready(Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - UserError::HeaderTooBig, - ))); - } } } @@ -199,7 +193,6 @@ where enum ControlFlow { Continue, Break, - EndlessLoopHeaderTooBig, } impl Encoder @@ -221,20 +214,7 @@ where Some(Next::Continuation(frame)) => { // Buffer the continuation frame, then try to write again let mut buf = limited_write_buf!(self); - if let Some(continuation) = frame.encode(&mut self.hpack, &mut buf) { - // We previously had a CONTINUATION, and after encoding - // it, we got *another* one? Let's just double check - // that at least some progress is being made... - if self.buf.get_ref().len() == frame::HEADER_LEN { - // If *only* the CONTINUATION frame header was - // written, and *no* header fields, we're stuck - // in a loop... - tracing::warn!( - "CONTINUATION frame write loop; header value too big to encode" - ); - return ControlFlow::EndlessLoopHeaderTooBig; - } - + if let Some(continuation) = frame.encode(&mut buf) { self.next = Some(Next::Continuation(continuation)); } ControlFlow::Continue diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 0b900de..cfc6a1a 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -5,17 +5,12 @@ use crate::hpack::{self, BytesStr}; use http::header::{self, HeaderName, HeaderValue}; use http::{uri, HeaderMap, Method, Request, StatusCode, Uri}; -use bytes::BytesMut; +use bytes::{BufMut, Bytes, BytesMut}; use std::fmt; use std::io::Cursor; type EncodeBuf<'a> = bytes::buf::Limit<&'a mut BytesMut>; - -// Minimum MAX_FRAME_SIZE is 16kb, so save some arbitrary space for frame -// head and other header bits. -const MAX_HEADER_LENGTH: usize = 1024 * 16 - 100; - /// Header frame /// /// This could be either a request or a response. @@ -100,11 +95,7 @@ struct HeaderBlock { #[derive(Debug)] struct EncodingHeaderBlock { - /// Argument to pass to the HPACK encoder to resume encoding - hpack: Option, - - /// remaining headers to encode - headers: Iter, + hpack: Bytes, } const END_STREAM: u8 = 0x1; @@ -241,10 +232,6 @@ impl Headers { self.header_block.is_over_size } - pub(crate) fn has_too_big_field(&self) -> bool { - self.header_block.has_too_big_field() - } - pub fn into_parts(self) -> (Pseudo, HeaderMap) { (self.header_block.pseudo, self.header_block.fields) } @@ -279,8 +266,8 @@ impl Headers { let head = self.head(); self.header_block - .into_encoding() - .encode(&head, encoder, dst, |_| {}) + .into_encoding(encoder) + .encode(&head, dst, |_| {}) } fn head(&self) -> Head { @@ -480,8 +467,6 @@ impl PushPromise { encoder: &mut hpack::Encoder, dst: &mut EncodeBuf<'_>, ) -> Option { - use bytes::BufMut; - // At this point, the `is_end_headers` flag should always be set debug_assert!(self.flags.is_end_headers()); @@ -489,8 +474,8 @@ impl PushPromise { let promised_id = self.promised_id; self.header_block - .into_encoding() - .encode(&head, encoder, dst, |dst| { + .into_encoding(encoder) + .encode(&head, dst, |dst| { dst.put_u32(promised_id.into()); }) } @@ -529,15 +514,11 @@ impl Continuation { Head::new(Kind::Continuation, END_HEADERS, self.stream_id) } - pub fn encode( - self, - encoder: &mut hpack::Encoder, - dst: &mut EncodeBuf<'_>, - ) -> Option { + pub fn encode(self, dst: &mut EncodeBuf<'_>) -> Option { // Get the CONTINUATION frame head let head = self.head(); - self.header_block.encode(&head, encoder, dst, |_| {}) + self.header_block.encode(&head, dst, |_| {}) } } @@ -617,13 +598,7 @@ impl Pseudo { // ===== impl EncodingHeaderBlock ===== impl EncodingHeaderBlock { - fn encode( - mut self, - head: &Head, - encoder: &mut hpack::Encoder, - dst: &mut EncodeBuf<'_>, - f: F, - ) -> Option + fn encode(mut self, head: &Head, dst: &mut EncodeBuf<'_>, f: F) -> Option where F: FnOnce(&mut EncodeBuf<'_>), { @@ -639,15 +614,17 @@ impl EncodingHeaderBlock { f(dst); // Now, encode the header payload - let continuation = match encoder.encode(self.hpack, &mut self.headers, dst) { - hpack::Encode::Full => None, - hpack::Encode::Partial(state) => Some(Continuation { + let continuation = if self.hpack.len() > dst.remaining_mut() { + dst.put_slice(&self.hpack.split_to(dst.remaining_mut())); + + Some(Continuation { stream_id: head.stream_id(), - header_block: EncodingHeaderBlock { - hpack: Some(state), - headers: self.headers, - }, - }), + header_block: self, + }) + } else { + dst.put_slice(&self.hpack); + + None }; // Compute the header block length @@ -910,13 +887,17 @@ impl HeaderBlock { Ok(()) } - fn into_encoding(self) -> EncodingHeaderBlock { + fn into_encoding(self, encoder: &mut hpack::Encoder) -> EncodingHeaderBlock { + let mut hpack = BytesMut::new(); + let headers = Iter { + pseudo: Some(self.pseudo), + fields: self.fields.into_iter(), + }; + + encoder.encode(headers, &mut hpack); + EncodingHeaderBlock { - hpack: None, - headers: Iter { - pseudo: Some(self.pseudo), - fields: self.fields.into_iter(), - }, + hpack: hpack.freeze(), } } @@ -949,48 +930,79 @@ impl HeaderBlock { .map(|(name, value)| decoded_header_size(name.as_str().len(), value.len())) .sum::() } - - /// Iterate over all pseudos and headers to see if any individual pair - /// would be too large to encode. - pub(crate) fn has_too_big_field(&self) -> bool { - macro_rules! pseudo_size { - ($name:ident) => {{ - self.pseudo - .$name - .as_ref() - .map(|m| decoded_header_size(stringify!($name).len() + 1, m.as_str().len())) - .unwrap_or(0) - }}; - } - - if pseudo_size!(method) > MAX_HEADER_LENGTH { - return true; - } - - if pseudo_size!(scheme) > MAX_HEADER_LENGTH { - return true; - } - - if pseudo_size!(authority) > MAX_HEADER_LENGTH { - return true; - } - - if pseudo_size!(path) > MAX_HEADER_LENGTH { - return true; - } - - // skip :status, its never going to be too big - - for (name, value) in &self.fields { - if decoded_header_size(name.as_str().len(), value.len()) > MAX_HEADER_LENGTH { - return true; - } - } - - false - } } fn decoded_header_size(name: usize, value: usize) -> usize { name + value + 32 } + +#[cfg(test)] +mod test { + use std::iter::FromIterator; + + use http::HeaderValue; + + use super::*; + use crate::frame; + use crate::hpack::{huffman, Encoder}; + + #[test] + fn test_nameless_header_at_resume() { + let mut encoder = Encoder::default(); + let mut dst = BytesMut::new(); + + let headers = Headers::new( + StreamId::ZERO, + Default::default(), + HeaderMap::from_iter(vec![ + ( + HeaderName::from_static("hello"), + HeaderValue::from_static("world"), + ), + ( + HeaderName::from_static("hello"), + HeaderValue::from_static("zomg"), + ), + ( + HeaderName::from_static("hello"), + HeaderValue::from_static("sup"), + ), + ]), + ); + + let continuation = headers + .encode(&mut encoder, &mut (&mut dst).limit(frame::HEADER_LEN + 8)) + .unwrap(); + + assert_eq!(17, dst.len()); + assert_eq!([0, 0, 8, 1, 0, 0, 0, 0, 0], &dst[0..9]); + assert_eq!(&[0x40, 0x80 | 4], &dst[9..11]); + assert_eq!("hello", huff_decode(&dst[11..15])); + assert_eq!(0x80 | 4, dst[15]); + + let mut world = dst[16..17].to_owned(); + + dst.clear(); + + assert!(continuation + .encode(&mut (&mut dst).limit(frame::HEADER_LEN + 16)) + .is_none()); + + world.extend_from_slice(&dst[9..12]); + assert_eq!("world", huff_decode(&world)); + + assert_eq!(24, dst.len()); + assert_eq!([0, 0, 15, 9, 4, 0, 0, 0, 0], &dst[0..9]); + + // // Next is not indexed + assert_eq!(&[15, 47, 0x80 | 3], &dst[12..15]); + assert_eq!("zomg", huff_decode(&dst[15..18])); + assert_eq!(&[15, 47, 0x80 | 3], &dst[18..21]); + assert_eq!("sup", huff_decode(&dst[21..])); + } + + fn huff_decode(src: &[u8]) -> BytesMut { + let mut buf = BytesMut::new(); + huffman::decode(src, &mut buf).unwrap() + } +} diff --git a/src/hpack/decoder.rs b/src/hpack/decoder.rs index dacbbd9..e4b34d1 100644 --- a/src/hpack/decoder.rs +++ b/src/hpack/decoder.rs @@ -847,7 +847,7 @@ mod test { fn huff_encode(src: &[u8]) -> BytesMut { let mut buf = BytesMut::new(); - huffman::encode(src, &mut buf).unwrap(); + huffman::encode(src, &mut buf); buf } } diff --git a/src/hpack/encoder.rs b/src/hpack/encoder.rs index 15a1175..bdff0a0 100644 --- a/src/hpack/encoder.rs +++ b/src/hpack/encoder.rs @@ -1,34 +1,21 @@ use super::table::{Index, Table}; use super::{huffman, Header}; -use bytes::{buf::Limit, BufMut, BytesMut}; +use bytes::{BufMut, BytesMut}; use http::header::{HeaderName, HeaderValue}; -type DstBuf<'a> = Limit<&'a mut BytesMut>; - #[derive(Debug)] pub struct Encoder { table: Table, size_update: Option, } -#[derive(Debug)] -pub enum Encode { - Full, - Partial(EncodeState), -} - #[derive(Debug)] pub struct EncodeState { index: Index, value: Option, } -#[derive(Debug, PartialEq, Eq)] -pub enum EncoderError { - BufferOverflow, -} - #[derive(Debug, Copy, Clone, Eq, PartialEq)] enum SizeUpdate { One(usize), @@ -77,60 +64,24 @@ impl Encoder { } /// Encode a set of headers into the provide buffer - pub fn encode( - &mut self, - resume: Option, - headers: &mut I, - dst: &mut DstBuf<'_>, - ) -> Encode + pub fn encode(&mut self, headers: I, dst: &mut BytesMut) where - I: Iterator>>, + I: IntoIterator>>, { let span = tracing::trace_span!("hpack::encode"); let _e = span.enter(); - let pos = position(dst); - tracing::trace!(pos, "encoding at"); - - if let Err(e) = self.encode_size_updates(dst) { - if e == EncoderError::BufferOverflow { - rewind(dst, pos); - } - - unreachable!("encode_size_updates errored"); - } + self.encode_size_updates(dst); let mut last_index = None; - if let Some(resume) = resume { - let pos = position(dst); - - let res = match resume.value { - Some(ref value) => self.encode_header_without_name(&resume.index, value, dst), - None => self.encode_header(&resume.index, dst), - }; - - if res.is_err() { - rewind(dst, pos); - return Encode::Partial(resume); - } - last_index = Some(resume.index); - } - for header in headers { - let pos = position(dst); - match header.reify() { // The header has an associated name. In which case, try to // index it in the table. Ok(header) => { let index = self.table.index(header); - let res = self.encode_header(&index, dst); - - if res.is_err() { - rewind(dst, pos); - return Encode::Partial(EncodeState { index, value: None }); - } + self.encode_header(&index, dst); last_index = Some(index); } @@ -139,77 +90,61 @@ impl Encoder { // which case, we skip table lookup and just use the same index // as the previous entry. Err(value) => { - let res = self.encode_header_without_name( + self.encode_header_without_name( last_index.as_ref().unwrap_or_else(|| { panic!("encoding header without name, but no previous index to use for name"); }), &value, dst, ); - - if res.is_err() { - rewind(dst, pos); - return Encode::Partial(EncodeState { - index: last_index.unwrap(), // checked just above - value: Some(value), - }); - } } - }; + } } - - Encode::Full } - fn encode_size_updates(&mut self, dst: &mut DstBuf<'_>) -> Result<(), EncoderError> { + fn encode_size_updates(&mut self, dst: &mut BytesMut) { match self.size_update.take() { Some(SizeUpdate::One(val)) => { self.table.resize(val); - encode_size_update(val, dst)?; + encode_size_update(val, dst); } Some(SizeUpdate::Two(min, max)) => { self.table.resize(min); self.table.resize(max); - encode_size_update(min, dst)?; - encode_size_update(max, dst)?; + encode_size_update(min, dst); + encode_size_update(max, dst); } None => {} } - - Ok(()) } - fn encode_header(&mut self, index: &Index, dst: &mut DstBuf<'_>) -> Result<(), EncoderError> { + fn encode_header(&mut self, index: &Index, dst: &mut BytesMut) { match *index { Index::Indexed(idx, _) => { - encode_int(idx, 7, 0x80, dst)?; + encode_int(idx, 7, 0x80, dst); } Index::Name(idx, _) => { let header = self.table.resolve(&index); - encode_not_indexed(idx, header.value_slice(), header.is_sensitive(), dst)?; + encode_not_indexed(idx, header.value_slice(), header.is_sensitive(), dst); } Index::Inserted(_) => { let header = self.table.resolve(&index); assert!(!header.is_sensitive()); - if !dst.has_remaining_mut() { - return Err(EncoderError::BufferOverflow); - } - dst.put_u8(0b0100_0000); - encode_str(header.name().as_slice(), dst)?; - encode_str(header.value_slice(), dst)?; + encode_str(header.name().as_slice(), dst); + encode_str(header.value_slice(), dst); } Index::InsertedValue(idx, _) => { let header = self.table.resolve(&index); assert!(!header.is_sensitive()); - encode_int(idx, 6, 0b0100_0000, dst)?; - encode_str(header.value_slice(), dst)?; + encode_int(idx, 6, 0b0100_0000, dst); + encode_str(header.value_slice(), dst); } Index::NotIndexed(_) => { let header = self.table.resolve(&index); @@ -219,19 +154,17 @@ impl Encoder { header.value_slice(), header.is_sensitive(), dst, - )?; + ); } } - - Ok(()) } fn encode_header_without_name( &mut self, last: &Index, value: &HeaderValue, - dst: &mut DstBuf<'_>, - ) -> Result<(), EncoderError> { + dst: &mut BytesMut, + ) { match *last { Index::Indexed(..) | Index::Name(..) @@ -239,7 +172,7 @@ impl Encoder { | Index::InsertedValue(..) => { let idx = self.table.resolve_idx(last); - encode_not_indexed(idx, value.as_ref(), value.is_sensitive(), dst)?; + encode_not_indexed(idx, value.as_ref(), value.is_sensitive(), dst); } Index::NotIndexed(_) => { let last = self.table.resolve(last); @@ -249,11 +182,9 @@ impl Encoder { value.as_ref(), value.is_sensitive(), dst, - )?; + ); } } - - Ok(()) } } @@ -263,52 +194,32 @@ impl Default for Encoder { } } -fn encode_size_update(val: usize, dst: &mut B) -> Result<(), EncoderError> { +fn encode_size_update(val: usize, dst: &mut BytesMut) { encode_int(val, 5, 0b0010_0000, dst) } -fn encode_not_indexed( - name: usize, - value: &[u8], - sensitive: bool, - dst: &mut DstBuf<'_>, -) -> Result<(), EncoderError> { +fn encode_not_indexed(name: usize, value: &[u8], sensitive: bool, dst: &mut BytesMut) { if sensitive { - encode_int(name, 4, 0b10000, dst)?; + encode_int(name, 4, 0b10000, dst); } else { - encode_int(name, 4, 0, dst)?; + encode_int(name, 4, 0, dst); } - encode_str(value, dst)?; - Ok(()) + encode_str(value, dst); } -fn encode_not_indexed2( - name: &[u8], - value: &[u8], - sensitive: bool, - dst: &mut DstBuf<'_>, -) -> Result<(), EncoderError> { - if !dst.has_remaining_mut() { - return Err(EncoderError::BufferOverflow); - } - +fn encode_not_indexed2(name: &[u8], value: &[u8], sensitive: bool, dst: &mut BytesMut) { if sensitive { dst.put_u8(0b10000); } else { dst.put_u8(0); } - encode_str(name, dst)?; - encode_str(value, dst)?; - Ok(()) + encode_str(name, dst); + encode_str(value, dst); } -fn encode_str(val: &[u8], dst: &mut DstBuf<'_>) -> Result<(), EncoderError> { - if !dst.has_remaining_mut() { - return Err(EncoderError::BufferOverflow); - } - +fn encode_str(val: &[u8], dst: &mut BytesMut) { if !val.is_empty() { let idx = position(dst); @@ -316,13 +227,13 @@ fn encode_str(val: &[u8], dst: &mut DstBuf<'_>) -> Result<(), EncoderError> { dst.put_u8(0); // Encode with huffman - huffman::encode(val, dst)?; + huffman::encode(val, dst); let huff_len = position(dst) - (idx + 1); if encode_int_one_byte(huff_len, 7) { // Write the string head - dst.get_mut()[idx] = 0x80 | huff_len as u8; + dst[idx] = 0x80 | huff_len as u8; } else { // Write the head to a placeholder const PLACEHOLDER_LEN: usize = 8; @@ -330,36 +241,29 @@ fn encode_str(val: &[u8], dst: &mut DstBuf<'_>) -> Result<(), EncoderError> { let head_len = { let mut head_dst = &mut buf[..]; - encode_int(huff_len, 7, 0x80, &mut head_dst)?; + encode_int(huff_len, 7, 0x80, &mut head_dst); PLACEHOLDER_LEN - head_dst.remaining_mut() }; - if dst.remaining_mut() < head_len { - return Err(EncoderError::BufferOverflow); - } - // This is just done to reserve space in the destination dst.put_slice(&buf[1..head_len]); - let written = dst.get_mut(); // Shift the header forward for i in 0..huff_len { let src_i = idx + 1 + (huff_len - (i + 1)); let dst_i = idx + head_len + (huff_len - (i + 1)); - written[dst_i] = written[src_i]; + dst[dst_i] = dst[src_i]; } // Copy in the head for i in 0..head_len { - written[idx + i] = buf[i]; + dst[idx + i] = buf[i]; } } } else { // Write an empty string dst.put_u8(0); } - - Ok(()) } /// Encode an integer into the given destination buffer @@ -368,16 +272,10 @@ fn encode_int( prefix_bits: usize, // The number of bits in the prefix first_byte: u8, // The base upon which to start encoding the int dst: &mut B, -) -> Result<(), EncoderError> { - let mut rem = dst.remaining_mut(); - - if rem == 0 { - return Err(EncoderError::BufferOverflow); - } - +) { if encode_int_one_byte(value, prefix_bits) { dst.put_u8(first_byte | value as u8); - return Ok(()); + return; } let low = (1 << prefix_bits) - 1; @@ -385,26 +283,14 @@ fn encode_int( value -= low; dst.put_u8(first_byte | low as u8); - rem -= 1; while value >= 128 { - if rem == 0 { - return Err(EncoderError::BufferOverflow); - } - dst.put_u8(0b1000_0000 | value as u8); - rem -= 1; value >>= 7; } - if rem == 0 { - return Err(EncoderError::BufferOverflow); - } - dst.put_u8(value as u8); - - Ok(()) } /// Returns true if the in the int can be fully encoded in the first byte. @@ -412,19 +298,14 @@ fn encode_int_one_byte(value: usize, prefix_bits: usize) -> bool { value < (1 << prefix_bits) - 1 } -fn position(buf: &DstBuf<'_>) -> usize { - buf.get_ref().len() -} - -fn rewind(buf: &mut DstBuf<'_>, pos: usize) { - buf.get_mut().truncate(pos); +fn position(buf: &BytesMut) -> usize { + buf.len() } #[cfg(test)] mod test { use super::*; use crate::hpack::Header; - use bytes::buf::BufMut; use http::*; #[test] @@ -801,52 +682,6 @@ mod test { assert_eq!("zomg", huff_decode(&res[14..])); } - #[test] - fn test_nameless_header_at_resume() { - let mut encoder = Encoder::default(); - let max_len = 15; - let mut dst = BytesMut::with_capacity(64); - - let mut input = vec![ - Header::Field { - name: Some("hello".parse().unwrap()), - value: HeaderValue::from_bytes(b"world").unwrap(), - }, - Header::Field { - name: None, - value: HeaderValue::from_bytes(b"zomg").unwrap(), - }, - Header::Field { - name: None, - value: HeaderValue::from_bytes(b"sup").unwrap(), - }, - ] - .into_iter(); - - let resume = match encoder.encode(None, &mut input, &mut (&mut dst).limit(max_len)) { - Encode::Partial(r) => r, - _ => panic!("encode should be partial"), - }; - - assert_eq!(&[0x40, 0x80 | 4], &dst[0..2]); - assert_eq!("hello", huff_decode(&dst[2..6])); - assert_eq!(0x80 | 4, dst[6]); - assert_eq!("world", huff_decode(&dst[7..11])); - - dst.clear(); - - match encoder.encode(Some(resume), &mut input, &mut (&mut dst).limit(max_len)) { - Encode::Full => {} - unexpected => panic!("resume returned unexpected: {:?}", unexpected), - } - - // Next is not indexed - assert_eq!(&[15, 47, 0x80 | 3], &dst[0..3]); - assert_eq!("zomg", huff_decode(&dst[3..6])); - assert_eq!(&[15, 47, 0x80 | 3], &dst[6..9]); - assert_eq!("sup", huff_decode(&dst[9..])); - } - #[test] fn test_large_size_update() { let mut encoder = Encoder::default(); @@ -855,9 +690,7 @@ mod test { assert_eq!(Some(SizeUpdate::One(1912930560)), encoder.size_update); let mut dst = BytesMut::with_capacity(6); - encoder - .encode_size_updates(&mut (&mut dst).limit(6)) - .unwrap(); + encoder.encode_size_updates(&mut dst); assert_eq!([63, 225, 129, 148, 144, 7], &dst[..]); } @@ -869,7 +702,7 @@ mod test { fn encode(e: &mut Encoder, hdrs: Vec>>) -> BytesMut { let mut dst = BytesMut::with_capacity(1024); - e.encode(None, &mut hdrs.into_iter(), &mut (&mut dst).limit(1024)); + e.encode(&mut hdrs.into_iter(), &mut dst); dst } diff --git a/src/hpack/huffman/mod.rs b/src/hpack/huffman/mod.rs index b8db8b4..07b3fd9 100644 --- a/src/hpack/huffman/mod.rs +++ b/src/hpack/huffman/mod.rs @@ -1,7 +1,7 @@ mod table; use self::table::{DECODE_TABLE, ENCODE_TABLE}; -use crate::hpack::{DecoderError, EncoderError}; +use crate::hpack::DecoderError; use bytes::{BufMut, BytesMut}; @@ -40,11 +40,9 @@ pub fn decode(src: &[u8], buf: &mut BytesMut) -> Result Ok(buf.split()) } -// TODO: return error when there is not enough room to encode the value -pub fn encode(src: &[u8], dst: &mut B) -> Result<(), EncoderError> { +pub fn encode(src: &[u8], dst: &mut BytesMut) { let mut bits: u64 = 0; let mut bits_left = 40; - let mut rem = dst.remaining_mut(); for &b in src { let (nbits, code) = ENCODE_TABLE[b as usize]; @@ -53,29 +51,18 @@ pub fn encode(src: &[u8], dst: &mut B) -> Result<(), EncoderError> { bits_left -= nbits; while bits_left <= 32 { - if rem == 0 { - return Err(EncoderError::BufferOverflow); - } - dst.put_u8((bits >> 32) as u8); bits <<= 8; bits_left += 8; - rem -= 1; } } if bits_left != 40 { - if rem == 0 { - return Err(EncoderError::BufferOverflow); - } - // This writes the EOS token bits |= (1 << bits_left) - 1; dst.put_u8((bits >> 32) as u8); } - - Ok(()) } impl Decoder { @@ -144,17 +131,17 @@ mod test { #[test] fn encode_single_byte() { - let mut dst = Vec::with_capacity(1); + let mut dst = BytesMut::with_capacity(1); - encode(b"o", &mut dst).unwrap(); + encode(b"o", &mut dst); assert_eq!(&dst[..], &[0b00111111]); dst.clear(); - encode(b"0", &mut dst).unwrap(); + encode(b"0", &mut dst); assert_eq!(&dst[..], &[0x0 + 7]); dst.clear(); - encode(b"A", &mut dst).unwrap(); + encode(b"A", &mut dst); assert_eq!(&dst[..], &[(0x21 << 2) + 3]); } @@ -185,9 +172,9 @@ mod test { ]; for s in DATA { - let mut dst = Vec::with_capacity(s.len()); + let mut dst = BytesMut::with_capacity(s.len()); - encode(s.as_bytes(), &mut dst).unwrap(); + encode(s.as_bytes(), &mut dst); let decoded = decode(&dst).unwrap(); @@ -201,9 +188,9 @@ mod test { &[b"\0", b"\0\0\0", b"\0\x01\x02\x03\x04\x05", b"\xFF\xF8"]; for s in DATA { - let mut dst = Vec::with_capacity(s.len()); + let mut dst = BytesMut::with_capacity(s.len()); - encode(s, &mut dst).unwrap(); + encode(s, &mut dst); let decoded = decode(&dst).unwrap(); diff --git a/src/hpack/mod.rs b/src/hpack/mod.rs index 365b005..b808e52 100644 --- a/src/hpack/mod.rs +++ b/src/hpack/mod.rs @@ -1,12 +1,12 @@ mod decoder; mod encoder; pub(crate) mod header; -mod huffman; +pub(crate) mod huffman; mod table; #[cfg(test)] mod test; pub use self::decoder::{Decoder, DecoderError, NeedMore}; -pub use self::encoder::{Encode, EncodeState, Encoder, EncoderError}; +pub use self::encoder::{EncodeState, Encoder}; pub use self::header::{BytesStr, Header}; diff --git a/src/hpack/test/fixture.rs b/src/hpack/test/fixture.rs index 9828f04..6d04484 100644 --- a/src/hpack/test/fixture.rs +++ b/src/hpack/test/fixture.rs @@ -1,6 +1,6 @@ use crate::hpack::{Decoder, Encoder, Header}; -use bytes::{buf::BufMut, BytesMut}; +use bytes::BytesMut; use hex::FromHex; use serde_json::Value; @@ -107,11 +107,7 @@ fn test_story(story: Value) { }) .collect(); - encoder.encode( - None, - &mut input.clone().into_iter(), - &mut (&mut buf).limit(limit), - ); + encoder.encode(&mut input.clone().into_iter(), &mut buf); decoder .decode(&mut Cursor::new(&mut buf), |e| { diff --git a/src/hpack/test/fuzz.rs b/src/hpack/test/fuzz.rs index d4e6534..0f84adc 100644 --- a/src/hpack/test/fuzz.rs +++ b/src/hpack/test/fuzz.rs @@ -1,8 +1,8 @@ -use crate::hpack::{Decoder, Encode, Encoder, Header}; +use crate::hpack::{Decoder, Encoder, Header}; use http::header::{HeaderName, HeaderValue}; -use bytes::{buf::BufMut, BytesMut}; +use bytes::BytesMut; use quickcheck::{Arbitrary, Gen, QuickCheck, TestResult}; use rand::{Rng, SeedableRng, StdRng}; @@ -144,7 +144,6 @@ impl FuzzHpack { } fn run(self) { - let mut chunks = self.chunks; let frames = self.frames; let mut expect = vec![]; @@ -173,11 +172,7 @@ impl FuzzHpack { } } - let mut input = frame.headers.into_iter(); - let mut index = None; - - let mut max_chunk = chunks.pop().unwrap_or(MAX_CHUNK); - let mut buf = BytesMut::with_capacity(max_chunk); + let mut buf = BytesMut::new(); if let Some(max) = frame.resizes.iter().max() { decoder.queue_size_update(*max); @@ -188,25 +183,7 @@ impl FuzzHpack { encoder.update_max_size(*resize); } - loop { - match encoder.encode(index.take(), &mut input, &mut (&mut buf).limit(max_chunk)) { - Encode::Full => break, - Encode::Partial(i) => { - index = Some(i); - - // Decode the chunk! - decoder - .decode(&mut Cursor::new(&mut buf), |h| { - let e = expect.remove(0); - assert_eq!(h, e); - }) - .expect("partial decode"); - - max_chunk = chunks.pop().unwrap_or(MAX_CHUNK); - buf = BytesMut::with_capacity(max_chunk); - } - } - } + encoder.encode(frame.headers, &mut buf); // Decode the chunk! decoder diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs index 10934de..e880412 100644 --- a/src/proto/streams/send.rs +++ b/src/proto/streams/send.rs @@ -133,10 +133,6 @@ impl Send { Self::check_headers(frame.fields())?; - if frame.has_too_big_field() { - return Err(UserError::HeaderTooBig); - } - let end_stream = frame.is_end_stream(); // Update the state @@ -270,10 +266,6 @@ impl Send { return Err(UserError::UnexpectedFrameType); } - if frame.has_too_big_field() { - return Err(UserError::HeaderTooBig); - } - stream.state.send_close(); tracing::trace!("send_trailers -- queuing; frame={:?}", frame); diff --git a/tests/h2-support/src/prelude.rs b/tests/h2-support/src/prelude.rs index f4b2e82..1fcb0dc 100644 --- a/tests/h2-support/src/prelude.rs +++ b/tests/h2-support/src/prelude.rs @@ -121,6 +121,7 @@ pub fn build_large_headers() -> Vec<(&'static str, String)> { ("eight", build_large_string('8', 4 * 1024)), ("nine", "nine".to_string()), ("ten", build_large_string('0', 4 * 1024)), + ("eleven", build_large_string('1', 32 * 1024)), ] }