diff --git a/examples/server.rs b/examples/server.rs index a38c873a..ee5ec568 100644 --- a/examples/server.rs +++ b/examples/server.rs @@ -112,7 +112,9 @@ async fn main() -> Result<(), Box> { Ok(conn) => { info!("new connection established"); - let mut h3_conn = h3::server::Connection::new(h3_quinn::Connection::new(conn)) + let mut h3_conn = h3::server::builder() + .max_field_section_size(1024 * 16) + .build(h3_quinn::Connection::new(conn)) .await .unwrap(); diff --git a/h3/src/client/connection.rs b/h3/src/client/connection.rs index 82b800e3..a5a60445 100644 --- a/h3/src/client/connection.rs +++ b/h3/src/client/connection.rs @@ -216,7 +216,11 @@ where let request_stream = RequestStream { inner: connection::RequestStream::new( - FrameStream::new(BufRecvStream::new(stream)), + FrameStream::new_with_max_buffered_frame( + BufRecvStream::new(stream), + self.max_field_section_size + .max(crate::frame::DEFAULT_MAX_BUFFERED_FRAME), + ), self.max_field_section_size, self.conn_state.clone(), self.send_grease_frame, diff --git a/h3/src/client/stream.rs b/h3/src/client/stream.rs index b1a9641f..90f41907 100644 --- a/h3/src/client/stream.rs +++ b/h3/src/client/stream.rs @@ -12,6 +12,7 @@ use crate::{ internal_error::InternalConnectionError, Code, StreamError, }, + frame::FrameStreamError, proto::{frame::Frame, headers::Header}, qpack, quic::{self}, @@ -97,8 +98,17 @@ where /// [`recv_data()`]: #method.recv_data #[cfg_attr(feature = "tracing", instrument(skip_all, level = "trace"))] pub async fn recv_response(&mut self) -> Result, StreamError> { - let mut frame = future::poll_fn(|cx| self.inner.stream.poll_next(cx)) - .await + let frame = future::poll_fn(|cx| self.inner.stream.poll_next(cx)).await; + if matches!( + frame, + Err(FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + .. + }) + ) { + self.inner.stop_sending(Code::H3_REQUEST_CANCELLED); + } + let mut frame = frame .map_err(|e| self.handle_frame_stream_error_on_request_stream(e))? .ok_or_else(|| { //= https://www.rfc-editor.org/rfc/rfc9114#section-4.1 diff --git a/h3/src/connection.rs b/h3/src/connection.rs index 44302f1a..b19ff254 100644 --- a/h3/src/connection.rs +++ b/h3/src/connection.rs @@ -565,6 +565,22 @@ where ), ))); } + Err(FrameStreamError::PayloadTooLarge { + frame_type: _ty, + actual_size: _actual, + max_size: _max, + }) => { + // Unlike a request stream, there is no message to reject here: + // every frame the control stream may legitimately carry is tens + // of bytes, and the stream itself is critical. + return Poll::Ready(Err(self.handle_connection_error( + InternalConnectionError::new( + Code::H3_EXCESSIVE_LOAD, + "control frame declared a payload larger than this endpoint will buffer" + .to_string(), + ), + ))); + } Err(FrameStreamError::Proto(frame_error)) => { return Poll::Ready(Err(self.handle_connection_error( InternalConnectionError::got_frame_error(frame_error), @@ -858,9 +874,18 @@ where if !self.stream.has_data() { match ready!(self.stream.poll_next(cx)) { Err(frame_stream_error) => { + if matches!( + frame_stream_error, + FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + .. + } + ) { + self.stream.stop_sending(Code::H3_REQUEST_CANCELLED); + } return Poll::Ready(Err( self.handle_frame_stream_error_on_request_stream(frame_stream_error) - )) + )); } Ok(None) => return Poll::Ready(Ok(None)), Ok(Some(Frame::Headers(encoded))) => { @@ -919,9 +944,18 @@ where } else { match ready!(self.stream.poll_next(cx)) { Err(frame_stream_error) => { + if matches!( + frame_stream_error, + FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + .. + } + ) { + self.stream.stop_sending(Code::H3_REQUEST_CANCELLED); + } return Poll::Ready(Err( self.handle_frame_stream_error_on_request_stream(frame_stream_error) - )) + )); } Ok(None) => return Poll::Ready(Ok(None)), Ok(Some(Frame::Headers(encoded))) => encoded, @@ -967,9 +1001,18 @@ where match self.stream.poll_next(cx) { Poll::Ready(Err(frame_stream_error)) => { + if matches!( + frame_stream_error, + FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + .. + } + ) { + self.stream.stop_sending(Code::H3_REQUEST_CANCELLED); + } return Poll::Ready(Err( self.handle_frame_stream_error_on_request_stream(frame_stream_error) - )) + )); } Poll::Ready(Ok(Some(trailing_frame))) => { // Received a known frame after trailers -> fail. diff --git a/h3/src/error/connection_error_creators.rs b/h3/src/error/connection_error_creators.rs index 93193e20..c99e95fc 100644 --- a/h3/src/error/connection_error_creators.rs +++ b/h3/src/error/connection_error_creators.rs @@ -197,6 +197,22 @@ where FrameStreamError::Proto(frame_error) => self.handle_connection_error_on_stream( InternalConnectionError::got_frame_error(frame_error), ), + // A HEADERS limit is per message: rejecting one request must not + // close unrelated streams on the same connection. + FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + actual_size, + max_size, + } => StreamError::HeaderTooBig { + actual_size, + max_size, + }, + FrameStreamError::PayloadTooLarge { .. } => { + self.handle_connection_error_on_stream(InternalConnectionError::new( + Code::H3_EXCESSIVE_LOAD, + "frame declared a payload larger than this endpoint will buffer".to_string(), + )) + } FrameStreamError::UnexpectedEnd => { self.handle_connection_error_on_stream(InternalConnectionError::new( Code::H3_FRAME_ERROR, diff --git a/h3/src/frame.rs b/h3/src/frame.rs index f9efc57b..61fd5fdc 100644 --- a/h3/src/frame.rs +++ b/h3/src/frame.rs @@ -29,9 +29,23 @@ pub struct FrameStream { impl FrameStream { pub fn new(stream: BufRecvStream) -> Self { + Self::new_with_max_buffered_frame(stream, DEFAULT_MAX_BUFFERED_FRAME) + } + + /// `max_buffered_frame` is a memory bound, not a policy limit: it must not + /// be tighter than what this endpoint has advertised it will accept, or a + /// field section would be refused from its encoded length before QPACK can + /// report the uncompressed size that [`RFC 9114 §4.2.2`] is written in + /// terms of, and that a peer needs in order to retry. + /// + /// [`RFC 9114 §4.2.2`]: https://www.rfc-editor.org/rfc/rfc9114#section-4.2.2 + pub(crate) fn new_with_max_buffered_frame( + stream: BufRecvStream, + max_buffered_frame: u64, + ) -> Self { Self { stream, - decoder: FrameDecoder::default(), + decoder: FrameDecoder::new(max_buffered_frame), remaining_data: 0, } } @@ -88,7 +102,7 @@ where Poll::Ready(false) => continue, Poll::Pending => return Poll::Pending, Poll::Ready(true) => { - if self.stream.buf_mut().has_remaining() { + if self.stream.buf_mut().has_remaining() || self.decoder.is_incomplete() { // Reached the end of receive stream, but there is still some data: // The frame is incomplete. return Poll::Ready(Err(FrameStreamError::UnexpectedEnd)); @@ -212,12 +226,51 @@ where } } -#[derive(Default)] +/// Default ceiling on the declared length of a frame that must be buffered in +/// full before it can be parsed. +/// +/// Generous next to anything legitimate — field sections in the wild are a few +/// kilobytes, and the control frames are tens of bytes — and finite, which is +/// the property that was missing. Nothing that works today against a +/// well-behaved peer stops working. +pub const DEFAULT_MAX_BUFFERED_FRAME: u64 = 1024 * 1024; + pub struct FrameDecoder { expected: Option, + /// Payload bytes of an unknown frame still to be discarded. + /// + /// Ignoring an unknown frame used to mean buffering it in full and then + /// dropping it, because the type was only recognised once `Frame::decode` + /// had the whole payload. It is recognised from the header now, and this is + /// what is left to throw away as it arrives. + skipping: u64, + /// See [`DEFAULT_MAX_BUFFERED_FRAME`]. + max_buffered: u64, +} + +impl Default for FrameDecoder { + fn default() -> Self { + Self { + expected: None, + skipping: 0, + max_buffered: DEFAULT_MAX_BUFFERED_FRAME, + } + } } impl FrameDecoder { + fn new(max_buffered: u64) -> Self { + Self { + expected: None, + skipping: 0, + max_buffered, + } + } + + fn is_incomplete(&self) -> bool { + self.expected.is_some() || self.skipping > 0 + } + fn decode( &mut self, src: &mut BufList, @@ -229,6 +282,18 @@ impl FrameDecoder { return Ok(None); } + // An unknown frame's payload, discarded as it arrives rather than + // after all of it has been stored. + if self.skipping > 0 { + let discard = self.skipping.min(src.remaining() as u64); + src.advance(discard as usize); + self.skipping -= discard; + if self.skipping > 0 { + return Ok(None); + } + continue; + } + if let Some(min) = self.expected { if src.remaining() < min { return Ok(None); @@ -237,12 +302,12 @@ impl FrameDecoder { let (pos, decoded) = { let mut cur = src.cursor(); - let decoded = Frame::decode(&mut cur); + let decoded = Frame::decode(&mut cur, self.max_buffered); (cur.position(), decoded) }; match decoded { - Err(frame::FrameError::UnknownFrame(_ty)) => { + Err(frame::FrameError::SkipUnknown { ty: _ty, len }) => { //= https://www.rfc-editor.org/rfc/rfc9114#section-4.1 //# Frames of unknown types (Section 9), including reserved frames //# (Section 7.2.8) MAY be sent on a request or push stream before, @@ -250,6 +315,42 @@ impl FrameDecoder { //= https://www.rfc-editor.org/rfc/rfc9114#section-7.2.8 //# Endpoints MUST //# NOT consider these frames to have any meaning upon receipt. + #[cfg(feature = "tracing")] + trace!("ignore unknown frame type {:#x}, {} bytes", _ty, len); + + // Only the header is consumed here. The payload goes to the + // branch above as it arrives, so ignoring an enormous + // unknown frame costs nothing to hold. + src.advance(pos); + self.expected = None; + self.skipping = len; + continue; + } + Err(frame::FrameError::TooLarge { + ty: _ty, + len: _len, + limit: _limit, + }) => { + #[cfg(feature = "tracing")] + trace!( + "frame type {:#x} declared {} bytes, over the {} ceiling", + _ty, + _len, + _limit + ); + + // Nothing of this frame was accumulated, and what came + // before it is released rather than left for a caller that + // is about to tear the stream down anyway. + src.advance(src.remaining()); + self.expected = None; + return Err(FrameStreamError::PayloadTooLarge { + frame_type: _ty, + actual_size: _len, + max_size: _limit, + }); + } + Err(frame::FrameError::UnknownFrame(_ty)) => { #[cfg(feature = "tracing")] trace!("ignore unknown frame type {:#x}", _ty); @@ -304,6 +405,14 @@ pub enum FrameStreamError { Proto(FrameProtocolError), Quic(StreamErrorIncoming), UnexpectedEnd, + /// A frame that must be buffered to be parsed declared a payload past the + /// decoder's ceiling. The owner decides from the frame context whether + /// this is a message-level rejection or a connection error. + PayloadTooLarge { + frame_type: u64, + actual_size: u64, + max_size: u64, + }, } #[derive(Debug, PartialEq)] @@ -399,6 +508,81 @@ mod tests { assert_matches!(decoder.decode(&mut buf), Ok(None)); } + #[test] + fn skips_unknown_payload_incrementally_then_decodes_next_frame() { + // Unknown type 22, declared payload length 63. Only ten payload bytes + // arrive with the header; they must be discarded, not retained. + let mut first = vec![22, 63]; + first.extend([0_u8; 10]); + let mut buf = BufList::from(Bytes::from(first)); + let mut decoder = FrameDecoder::default(); + + assert_matches!(decoder.decode(&mut buf), Ok(None)); + assert_eq!(buf.remaining(), 0); + assert_eq!(decoder.skipping, 53); + + // The rest of the unknown payload and a valid DATA header arrive + // together. The decoder must discard exactly the former and preserve + // framing for the latter. + let mut rest = vec![0_u8; 53]; + rest.extend([0, 4]); + buf.push(Bytes::from(rest)); + + assert_matches!( + decoder.decode(&mut buf), + Ok(Some(Frame::Data(PayloadLen(4)))) + ); + assert_eq!(decoder.skipping, 0); + assert_eq!(buf.remaining(), 0); + } + + #[test] + fn an_enormous_unknown_frame_costs_nothing_to_ignore() { + // 0x21 is the first of the reserved GREASE frame types: an unknown + // frame is not by itself an attack, it is something h3 sends on + // purpose, and it must be ignored whatever length it declares. + // Ignoring it is what must not cost the declared length. + const DECLARED: u64 = 4 * 1024 * 1024 * 1024; + const CHUNK: usize = 1024 * 1024; + + let mut header = BytesMut::new(); + VarInt::from_u64(0x21).unwrap().encode(&mut header); + VarInt::from_u64(DECLARED).unwrap().encode(&mut header); + + let mut buf = BufList::from(header.freeze()); + let mut decoder = FrameDecoder::default(); + + // The header on its own settles what happens to the payload. Note that + // the declared length exceeds u32 and, on a 32-bit target, `usize`: + // what is left to discard is counted, never sized into an allocation. + assert_matches!(decoder.decode(&mut buf), Ok(None)); + assert_eq!(decoder.skipping, DECLARED); + assert_eq!(buf.remaining(), 0); + + // Stream the payload past the decoder. `BufList::advance` drops the + // chunks it consumes, so "nothing remaining" is "nothing retained": + // the peak cost here is one chunk, not the four gigabytes declared. + let chunk = Bytes::from(vec![0_u8; CHUNK]); + for _ in 0..(DECLARED / CHUNK as u64) - 1 { + buf.push(chunk.clone()); + assert_matches!(decoder.decode(&mut buf), Ok(None)); + assert_eq!(buf.remaining(), 0, "a payload being ignored was retained"); + } + assert_eq!(decoder.skipping, CHUNK as u64); + + // The last of the ignored payload arrives in the same chunk as a real + // frame. The decoder has to discard exactly the former — one byte + // either way desynchronises the stream — and then parse the latter. + let mut tail = BytesMut::with_capacity(CHUNK + 16); + tail.put_bytes(0, CHUNK); + Frame::headers(&b"salut"[..]).encode_with_payload(&mut tail); + buf.push(tail.freeze()); + + assert_matches!(decoder.decode(&mut buf), Ok(Some(Frame::Headers(_)))); + assert_eq!(decoder.skipping, 0); + assert_eq!(buf.remaining(), 0); + } + // FrameStream macro_rules! assert_poll_matches { @@ -496,6 +680,37 @@ mod tests { ); } + #[tokio::test] + async fn truncated_unknown_frame_is_an_unexpected_end() { + let mut recv = FakeRecv::default(); + // Unknown type 22 declares four payload bytes but sends only two. + recv.chunk(Bytes::from_static(&[22, 4, b'a', b'b'])); + let mut stream: FrameStream<_, ()> = FrameStream::new(BufRecvStream::new(recv)); + + assert_poll_matches!( + |cx| stream.poll_next(cx), + Err(FrameStreamError::UnexpectedEnd) + ); + } + + #[tokio::test] + async fn oversized_buffered_frame_fails_from_header_alone() { + let mut recv = FakeRecv::default(); + // HEADERS declares 63 bytes and sends no payload. + recv.chunk(Bytes::from_static(&[1, 63])); + let mut stream: FrameStream<_, ()> = + FrameStream::new_with_max_buffered_frame(BufRecvStream::new(recv), 62); + + assert_poll_matches!( + |cx| stream.poll_next(cx), + Err(FrameStreamError::PayloadTooLarge { + frame_type: 1, + actual_size: 63, + max_size: 62 + }) + ); + } + #[tokio::test] #[should_panic( expected = "There is still data to read, please call poll_data() until it returns None" diff --git a/h3/src/proto/frame.rs b/h3/src/proto/frame.rs index a09520bb..5e51b373 100644 --- a/h3/src/proto/frame.rs +++ b/h3/src/proto/frame.rs @@ -25,6 +25,27 @@ pub enum FrameError { Settings(SettingsError), InvalidStreamId(InvalidStreamId), InvalidPushId(InvalidPushId), + /// A buffered frame declared a payload larger than the configured ceiling. + /// + /// Returned from the declared length alone, before any of the payload is + /// accumulated — which is the whole point. Every other error here can be + /// reached only once the frame is in memory. + TooLarge { + ty: u64, + len: u64, + limit: u64, + }, + /// An unknown frame type, reported *before* its payload has been read so + /// the caller can discard it as it arrives. + /// + /// Distinct from [`FrameError::UnknownFrame`], which is returned once the + /// payload is already buffered. RFC 9114 requires unknown frames to be + /// ignored; accumulating one in full in order to ignore it satisfies the + /// letter and not the substance. + SkipUnknown { + ty: u64, + len: u64, + }, } impl std::error::Error for FrameError {} @@ -40,6 +61,14 @@ impl fmt::Display for FrameError { FrameError::Settings(x) => write!(f, "invalid settings: {}", x), FrameError::InvalidStreamId(x) => write!(f, "{}", x), FrameError::InvalidPushId(x) => write!(f, "{}", x), + FrameError::TooLarge { ty, len, limit } => write!( + f, + "frame 0x{:x} declared {} bytes, over the {} byte ceiling", + ty, len, limit + ), + FrameError::SkipUnknown { ty, len } => { + write!(f, "frame 0x{:x} ignored, {} bytes skipped", ty, len) + } } } } @@ -79,7 +108,22 @@ impl Frame { pub const MAX_ENCODED_SIZE: usize = VarInt::MAX_SIZE * 7; /// Decodes a Frame from the stream according to - pub fn decode(buf: &mut T) -> Result { + /// + /// `max_buffered` bounds the declared payload length of a frame this + /// function has to hold in memory to parse. It is checked against the + /// length field, before the caller is ever told to accumulate — which is + /// the difference that matters. Previously the completeness check came + /// first, so a peer declaring a large `HEADERS` had its payload buffered in + /// full before `max_field_section_size` could reject it, and a peer + /// declaring a large *unknown* frame had it buffered in full before it + /// could be ignored. Neither was bounded by flow control: the caller + /// consumes each chunk as it arrives, which is what releases the credit for + /// the next one. + /// + /// `DATA` is unaffected. Its payload is never held here — the frame carries + /// only a length and the body is streamed — so the ceiling does not apply + /// to it and request bodies keep whatever bound their reader already has. + pub fn decode(buf: &mut T, max_buffered: u64) -> Result { let remaining = buf.remaining(); let ty = FrameType::decode(buf).map_err(|_| FrameError::Incomplete(remaining + 1))?; @@ -101,6 +145,28 @@ impl Frame { return Ok(Frame::Data((len as usize).into())); } + // Everything below is decided from the type and the declared length + // alone, before anything asks the caller to hold a payload. + + // A reserved HTTP/2 type is a connection error whatever it carries, so + // reading what it carries is pure cost. + if ty.is_reserved() { + return Err(FrameError::UnsupportedFrame(ty.0)); + } + + // Unknown: to be ignored, and ignoring it must not mean storing it. + if !ty.is_buffered() { + return Err(FrameError::SkipUnknown { ty: ty.0, len }); + } + + if len > max_buffered { + return Err(FrameError::TooLarge { + ty: ty.0, + len, + limit: max_buffered, + }); + } + if buf.remaining() < len as usize { return Err(FrameError::Incomplete(2 + len as usize)); } @@ -317,6 +383,43 @@ impl FrameType { pub fn grease() -> Self { FrameType(fastrand::u64(0..0x210842108421083) * 0x1f + 0x21) } + + /// Whether parsing this type requires its whole payload in memory. + /// + /// The list is exactly the arms of the match in [`Frame::decode`] that read + /// from `payload`. `DATA` is absent because its body is streamed rather + /// than held, and `WEBTRANSPORT_BI_STREAM` because it carries no length at + /// all — both are handled before this is consulted. + /// + /// Kept beside the type list rather than derived from the match so that + /// adding a frame type to one and not the other is a compile-time + /// omission with a visible home, instead of a type that silently becomes + /// unbounded. + fn is_buffered(self) -> bool { + matches!( + self, + FrameType::HEADERS + | FrameType::SETTINGS + | FrameType::CANCEL_PUSH + | FrameType::PUSH_PROMISE + | FrameType::GOAWAY + | FrameType::MAX_PUSH_ID + ) + } + + /// The HTTP/2 types RFC 9114 §7.2.8 requires be treated as a connection + /// error of type `H3_FRAME_UNEXPECTED`. + /// + /// Decided by the type, so the payload is never worth reading. + fn is_reserved(self) -> bool { + matches!( + self, + FrameType::H2_PRIORITY + | FrameType::H2_PING + | FrameType::H2_WINDOW_UPDATE + | FrameType::H2_CONTINUATION + ) + } } #[derive(Copy, Clone, Eq, PartialEq, Debug)] @@ -619,31 +722,85 @@ mod tests { use assert_matches::assert_matches; use std::io::Cursor; + /// An unknown type is now reported from its header, with the payload left + /// for the caller to discard as it arrives. + /// + /// It used to be reported as `UnknownFrame` *after* the whole payload had + /// been buffered, which is how ignoring an unknown frame came to cost its + /// full declared length in memory. `FrameDecoder` does the skipping, and + /// `frame::tests` covers reading the frame that follows. #[test] fn unknown_frame_type() { let mut buf = Cursor::new(&[22, 4, 0, 255, 128, 0, 3, 1, 2]); - assert_matches!(Frame::decode(&mut buf), Err(FrameError::UnknownFrame(22))); - assert_matches!(Frame::decode(&mut buf), Ok(Frame::CancelPush(PushId(2)))); + assert_matches!( + Frame::decode(&mut buf, u64::MAX), + Err(FrameError::SkipUnknown { ty: 22, len: 4 }) + ); + } + + /// The ceiling is applied to the declared length, with no payload present. + /// + /// The buffer holds a two-byte header and nothing else, so a decoder that + /// checked completeness first would answer `Incomplete` and wait — which is + /// precisely the accumulation this exists to prevent. + #[test] + fn oversized_buffered_frame_is_refused_from_its_length_alone() { + // HEADERS, declared length 63, no payload. 63 is the largest value a + // single-byte QUIC varint holds, which keeps the fixture two bytes. + let mut buf = Cursor::new(&[1, 63]); + assert_matches!( + Frame::decode(&mut buf, 62), + Err(FrameError::TooLarge { + ty: 1, + len: 63, + limit: 62 + }) + ); + + // Exactly at the ceiling is not over it: this must reach the ordinary + // completeness path instead. + let mut buf = Cursor::new(&[1, 63]); + assert_matches!(Frame::decode(&mut buf, 63), Err(FrameError::Incomplete(_))); + } + + /// A reserved HTTP/2 type is a connection error decided by the type, so its + /// payload must never be waited for either. + #[test] + fn a_reserved_frame_is_refused_without_its_payload() { + // H2_PING (0x6), declared length 63, no payload. + let mut buf = Cursor::new(&[6, 63]); + assert_matches!( + Frame::decode(&mut buf, u64::MAX), + Err(FrameError::UnsupportedFrame(6)) + ); + } + + /// DATA is untouched: it declares a length and streams its body, so the + /// ceiling does not apply and request bodies keep their own bound. + #[test] + fn data_frames_are_not_subject_to_the_buffered_ceiling() { + let mut buf = Cursor::new(&[0, 63]); + assert_matches!(Frame::decode(&mut buf, 1), Ok(Frame::Data(PayloadLen(63)))); } #[test] fn len_unexpected_end() { let mut buf = Cursor::new(&[0, 255]); - let decoded = Frame::decode(&mut buf); + let decoded = Frame::decode(&mut buf, u64::MAX); assert_matches!(decoded, Err(FrameError::Incomplete(3))); } #[test] fn type_unexpected_end() { let mut buf = Cursor::new(&[255]); - let decoded = Frame::decode(&mut buf); + let decoded = Frame::decode(&mut buf, u64::MAX); assert_matches!(decoded, Err(FrameError::Incomplete(2))); } #[test] fn buffer_too_short() { let mut buf = Cursor::new(&[4, 4, 0, 255, 128]); - let decoded = Frame::decode(&mut buf); + let decoded = Frame::decode(&mut buf, u64::MAX); assert_matches!(decoded, Err(FrameError::Incomplete(6))); } @@ -653,7 +810,7 @@ mod tests { assert_eq!(&buf, &wire); let mut read = Cursor::new(&buf); - let decoded = Frame::decode(&mut read).unwrap(); + let decoded = Frame::decode(&mut read, u64::MAX).unwrap(); assert_eq!(check_frame, decoded); } @@ -756,7 +913,10 @@ mod tests { VarInt::from_u32(0x21 + 2 * 0x1f).encode(&mut raw); raw.extend(&[6, 0, 255, 128, 0, 250, 218]); let mut buf = Cursor::new(&raw); - let decoded = Frame::decode(&mut buf); - assert_matches!(decoded, Err(FrameError::UnknownFrame(95))); + let decoded = Frame::decode(&mut buf, u64::MAX); + // Reported from the header now rather than after the payload has been + // buffered: a GREASE type is unknown, and ignoring an unknown frame no + // longer costs its declared length in memory. + assert_matches!(decoded, Err(FrameError::SkipUnknown { ty: 95, len: 6 })); } } diff --git a/h3/src/server/connection.rs b/h3/src/server/connection.rs index 1a3a35d8..f4dee369 100644 --- a/h3/src/server/connection.rs +++ b/h3/src/server/connection.rs @@ -127,7 +127,11 @@ where pub async fn accept(&mut self) -> Result>, ConnectionError> { // Accept the incoming stream let stream = match poll_fn(|cx| self.poll_accept_request_stream_internal(cx)).await? { - Some(s) => FrameStream::new(BufRecvStream::new(s)), + Some(s) => FrameStream::new_with_max_buffered_frame( + BufRecvStream::new(s), + self.max_field_section_size + .max(crate::frame::DEFAULT_MAX_BUFFERED_FRAME), + ), None => { // We always send a last GoAway frame to the client, so it knows which was the last // non-rejected request. diff --git a/h3/src/server/request.rs b/h3/src/server/request.rs index 01599921..bf291aa5 100644 --- a/h3/src/server/request.rs +++ b/h3/src/server/request.rs @@ -125,6 +125,16 @@ where ); } Err(e) => { + if matches!( + e, + FrameStreamError::PayloadTooLarge { + frame_type: 0x1, + .. + } + ) { + self.frame_stream.stop_sending(Code::H3_REQUEST_CANCELLED); + self.frame_stream.reset(Code::H3_REQUEST_CANCELLED.value()); + } return Err(self.handle_frame_stream_error_on_request_stream(e)); } };