From 513edb378cfe88b81ed16d25802a4f12602bc065 Mon Sep 17 00:00:00 2001
From: Anthony Ramine <aramine@cloudflare.com>
Date: Thu, 16 Jan 2025 10:38:44 +0100
Subject: [PATCH] Fix window size decrement of send-closed streams

Send-closed streams should be skipped when decreasing window size,
as they are skipped when increasing it.
---
 src/proto/streams/flow_control.rs    |  8 ++++----
 src/proto/streams/send.rs            | 16 +++++++++++-----
 tests/h2-tests/tests/flow_control.rs |  6 ++----
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/src/proto/streams/flow_control.rs b/src/proto/streams/flow_control.rs
index 57a935825..d66df615a 100644
--- a/src/proto/streams/flow_control.rs
+++ b/src/proto/streams/flow_control.rs
@@ -136,16 +136,16 @@ impl FlowControl {
     ///
     /// This is called after receiving a SETTINGS frame with a lower
     /// INITIAL_WINDOW_SIZE value.
-    pub fn dec_send_window(&mut self, sz: WindowSize) -> Result<(), Reason> {
+    pub fn dec_send_window(&mut self, sz: WindowSize) {
         tracing::trace!(
             "dec_window; sz={}; window={}, available={}",
             sz,
             self.window_size,
             self.available
         );
-        // ~~This should not be able to overflow `window_size` from the bottom.~~ wrong. it can.
-        self.window_size.decrease_by(sz)?;
-        Ok(())
+        self.window_size
+            .decrease_by(sz)
+            .expect("window_size should be greater than decrement");
     }
 
     /// Decrement the recv-side window size.
diff --git a/src/proto/streams/send.rs b/src/proto/streams/send.rs
index 2a7abba06..334eea8d9 100644
--- a/src/proto/streams/send.rs
+++ b/src/proto/streams/send.rs
@@ -466,6 +466,16 @@ impl Send {
                     store.try_for_each(|mut stream| {
                         let stream = &mut *stream;
 
+                        if stream.state.is_send_closed() {
+                            tracing::trace!(
+                                "skipping send-closed stream; id={:?}; flow={:?}",
+                                stream.id,
+                                stream.send_flow
+                            );
+
+                            return Ok(());
+                        }
+
                         tracing::trace!(
                             "decrementing stream window; id={:?}; decr={}; flow={:?}",
                             stream.id,
@@ -473,11 +483,7 @@ impl Send {
                             stream.send_flow
                         );
 
-                        // TODO: this decrement can underflow based on received frames!
-                        stream
-                            .send_flow
-                            .dec_send_window(dec)
-                            .map_err(proto::Error::library_go_away)?;
+                        stream.send_flow.dec_send_window(dec);
 
                         // It's possible that decreasing the window causes
                         // `window_size` (the stream-specific window) to fall below
diff --git a/tests/h2-tests/tests/flow_control.rs b/tests/h2-tests/tests/flow_control.rs
index 43347d30c..32cc5a7c4 100644
--- a/tests/h2-tests/tests/flow_control.rs
+++ b/tests/h2-tests/tests/flow_control.rs
@@ -1859,7 +1859,7 @@ async fn poll_capacity_wakeup_after_window_update() {
 }
 
 #[tokio::test]
-async fn window_size_decremented_past_zero() {
+async fn window_size_does_not_underflow() {
     h2_support::trace_init!();
     let (io, mut client) = mock::new();
 
@@ -1891,9 +1891,7 @@ async fn window_size_decremented_past_zero() {
         let builder = server::Builder::new();
         let mut srv = builder.handshake::<_, Bytes>(io).await.expect("handshake");
 
-        // just keep it open
-        let res = poll_fn(move |cx| srv.poll_closed(cx)).await;
-        tracing::debug!("{:?}", res);
+        poll_fn(move |cx| srv.poll_closed(cx)).await.unwrap();
     };
 
     join(client, srv).await;