From 99ef446331a05876a4d105dd82bf425053a045e9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Fri, 20 Oct 2023 22:12:48 +0200 Subject: [PATCH] Yield protocol violation for packets without frames --- quinn-proto/src/connection/mod.rs | 6 +++--- quinn-proto/src/frame.rs | 16 +++++++++++++--- quinn-proto/src/tests/mod.rs | 1 + 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 190341fe8..e0c0c37da 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -2192,7 +2192,7 @@ impl Connection { return Ok(()); } State::Closed(_) => { - for result in frame::Iter::new(packet.payload.freeze()) { + for result in frame::Iter::new(packet.payload.freeze())? { let frame = match result { Ok(frame) => frame, Err(err) => { @@ -2441,7 +2441,7 @@ impl Connection { debug_assert_ne!(packet.header.space(), SpaceId::Data); let payload_len = packet.payload.len(); let mut ack_eliciting = false; - for result in frame::Iter::new(packet.payload.freeze()) { + for result in frame::Iter::new(packet.payload.freeze())? { let frame = result?; let span = match frame { Frame::Padding => continue, @@ -2499,7 +2499,7 @@ impl Connection { let mut close = None; let payload_len = payload.len(); let mut ack_eliciting = false; - for result in frame::Iter::new(payload) { + for result in frame::Iter::new(payload)? { let frame = result?; let span = match frame { Frame::Padding => continue, diff --git a/quinn-proto/src/frame.rs b/quinn-proto/src/frame.rs index e802b3dc9..1916b3929 100644 --- a/quinn-proto/src/frame.rs +++ b/quinn-proto/src/frame.rs @@ -550,11 +550,20 @@ impl From for IterErr { } impl Iter { - pub(crate) fn new(payload: Bytes) -> Self { - Self { + pub(crate) fn new(payload: Bytes) -> Result { + if payload.is_empty() { + // "An endpoint MUST treat receipt of a packet containing no frames as a + // connection error of type PROTOCOL_VIOLATION." + // https://www.rfc-editor.org/rfc/rfc9000.html#name-frames-and-frame-types + return Err(TransportError::PROTOCOL_VIOLATION( + "packet payload is empty", + )); + } + + Ok(Self { bytes: io::Cursor::new(payload), last_ty: None, - } + }) } fn take_len(&mut self) -> Result { @@ -923,6 +932,7 @@ mod test { fn frames(buf: Vec) -> Vec { Iter::new(Bytes::from(buf)) + .unwrap() .collect::, _>>() .unwrap() } diff --git a/quinn-proto/src/tests/mod.rs b/quinn-proto/src/tests/mod.rs index 6f69e3303..f1bfda432 100644 --- a/quinn-proto/src/tests/mod.rs +++ b/quinn-proto/src/tests/mod.rs @@ -2257,6 +2257,7 @@ fn single_ack_eliciting_packet_triggers_ack_after_delay() { // The ACK delay is properly calculated assert_eq!(pair.client.captured_packets.len(), 1); let mut frames = frame::Iter::new(pair.client.captured_packets.remove(0).into()) + .unwrap() .collect::, _>>() .unwrap(); assert_eq!(frames.len(), 1);