Fix some flow-control cases

bearcove · Mar 12, 2024 · 324f999 · 324f999
1 parent a11b9f7
commit 324f999
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 18 deletions.
diff --git a/crates/fluke/src/h2/parse.rs b/crates/fluke/src/h2/parse.rs
@@ -274,19 +274,23 @@ impl Frame {
 }
 
 /// See https://httpwg.org/specs/rfc9113.html#FrameHeader - the first bit
-/// is reserved, and the rest is a 32-bit stream id
-fn parse_reserved_and_stream_id(i: Roll) -> IResult<Roll, (u8, StreamId)> {
+/// is reserved, and the rest is a 31-bit stream id
+pub fn parse_reserved_and_u31(i: Roll) -> IResult<Roll, (u8, u32)> {
     fn reserved(i: (Roll, usize)) -> IResult<(Roll, usize), u8> {
         nom::bits::streaming::take(1_usize)(i)
     }
 
-    fn stream_id(i: (Roll, usize)) -> IResult<(Roll, usize), StreamId> {
-        nom::combinator::map(nom::bits::streaming::take(31_usize), StreamId)(i)
+    fn stream_id(i: (Roll, usize)) -> IResult<(Roll, usize), u32> {
+        nom::bits::streaming::take(31_usize)(i)
     }
 
     nom::bits::bits(tuple((reserved, stream_id)))(i)
 }
 
+fn parse_reserved_and_stream_id(i: Roll) -> IResult<Roll, (u8, StreamId)> {
+    parse_reserved_and_u31(i).map(|(i, (reserved, stream_id))| (i, (reserved, StreamId(stream_id))))
+}
+
 // cf. https://httpwg.org/specs/rfc9113.html#HEADERS
 #[derive(Debug)]
 pub(crate) struct PrioritySpec {

diff --git a/crates/fluke/src/h2/read.rs b/crates/fluke/src/h2/read.rs
@@ -20,7 +20,7 @@ use tracing::{debug, trace, warn};
 
 use crate::{
     h2::{
-        parse::{Frame, FrameType, PrioritySpec},
+        parse::{parse_reserved_and_u31, Frame, FrameType, PrioritySpec},
         types::H2ConnectionError,
     },
     util::read_and_parse,
@@ -465,26 +465,48 @@ impl<D: ServerDriver + 'static> H2ReadContext<D> {
                     }
                 }
                 FrameType::GoAway => {
+                    if frame.stream_id != StreamId::CONNECTION {
+                        return Err(H2ConnectionError::GoAwayWithNonZeroStreamId {
+                            stream_id: frame.stream_id,
+                        });
+                    }
+
                     self.goaway_recv = true;
 
                     // TODO: this should probably have other effects than setting
                     // this flag.
                 }
-                FrameType::WindowUpdate => match frame.stream_id.0 {
-                    0 => {
-                        debug!("TODO: ignoring connection-wide window update");
+                FrameType::WindowUpdate => {
+                    if payload.len() != 4 {
+                        return Err(H2ConnectionError::WindowUpdateInvalidLength {
+                            len: payload.len() as _,
+                        });
                     }
-                    _ => match self.state.streams.get_mut(&frame.stream_id) {
-                        Some(_ss) => {
-                            debug!("TODO: handle window update for stream {}", frame.stream_id)
-                        }
-                        None => {
-                            return Err(H2ConnectionError::WindowUpdateForUnknownStream {
-                                stream_id: frame.stream_id,
-                            });
+
+                    let increment;
+                    (_, (_, increment)) = parse_reserved_and_u31(payload)
+                        .finish()
+                        .map_err(|err| eyre::eyre!("parsing error: {err:?}"))?;
+
+                    if increment == 0 {
+                        return Err(H2ConnectionError::WindowUpdateZeroIncrement);
+                    }
+
+                    if frame.stream_id == StreamId::CONNECTION {
+                        debug!("TODO: ignoring connection-wide window update");
+                    } else {
+                        match self.state.streams.get_mut(&frame.stream_id) {
+                            None => {
+                                return Err(H2ConnectionError::WindowUpdateForUnknownStream {
+                                    stream_id: frame.stream_id,
+                                });
+                            }
+                            Some(_ss) => {
+                                debug!("TODO: handle window update for stream {}", frame.stream_id)
+                            }
                         }
-                    },
-                },
+                    }
+                }
                 FrameType::Continuation(_flags) => {
                     return Err(H2ConnectionError::UnexpectedContinuationFrame {
                         stream_id: frame.stream_id,

diff --git a/crates/fluke/src/h2/types.rs b/crates/fluke/src/h2/types.rs
@@ -152,6 +152,15 @@ pub(crate) enum H2ConnectionError {
 
     #[error("received settings frame with non-zero stream id")]
     SettingsWithNonZeroStreamId { stream_id: StreamId },
+
+    #[error("received goaway frame with non-zero stream id")]
+    GoAwayWithNonZeroStreamId { stream_id: StreamId },
+
+    #[error("zero increment in window update frame for stream")]
+    WindowUpdateZeroIncrement,
+
+    #[error("received window update frame with invalid length {len}")]
+    WindowUpdateInvalidLength { len: usize },
 }
 
 impl H2ConnectionError {
@@ -163,6 +172,7 @@ impl H2ConnectionError {
             H2ConnectionError::PaddedFrameTooShort { .. } => KnownErrorCode::FrameSizeError,
             H2ConnectionError::PingFrameInvalidLength { .. } => KnownErrorCode::FrameSizeError,
             H2ConnectionError::SettingsAckWithPayload { .. } => KnownErrorCode::FrameSizeError,
+            H2ConnectionError::WindowUpdateInvalidLength { .. } => KnownErrorCode::FrameSizeError,
             // compression errors
             H2ConnectionError::CompressionError(_) => KnownErrorCode::CompressionError,
             // stream closed error