From 10d8ed742972d61e98d264450d8e173ae29e148a Mon Sep 17 00:00:00 2001
From: Sean McArthur <sean@seanmonstar.com>
Date: Tue, 19 Dec 2017 20:07:39 -0800
Subject: [PATCH] Add test that a window update can be received in reserved
 state (#195)

---
 src/proto/streams/prioritize.rs |  5 ++++
 src/proto/streams/recv.rs       |  4 +++
 src/proto/streams/state.rs      |  7 +++++
 tests/stream_states.rs          | 52 +++++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+)

diff --git a/src/proto/streams/prioritize.rs b/src/proto/streams/prioritize.rs
index 10ecff7..3282ef8 100644
--- a/src/proto/streams/prioritize.rs
+++ b/src/proto/streams/prioritize.rs
@@ -238,6 +238,11 @@ impl Prioritize {
             stream.send_flow
         );
 
+        if stream.state.is_send_closed() && stream.buffered_send_data == 0 {
+            // We can't send any data, so don't bother doing anything else.
+            return Ok(());
+        }
+
         // Update the stream level flow control.
         stream.send_flow.inc_window(inc)?;
 
diff --git a/src/proto/streams/recv.rs b/src/proto/streams/recv.rs
index b5f1158..fb67a7a 100644
--- a/src/proto/streams/recv.rs
+++ b/src/proto/streams/recv.rs
@@ -745,6 +745,10 @@ impl Recv {
             if !stream.state.is_recv_streaming() {
                 // No need to send window updates on the stream if the stream is
                 // no longer receiving data.
+                //
+                // TODO: is this correct? We could possibly send a window
+                // update on a ReservedRemote stream if we already know
+                // we want to stream the data faster...
                 continue;
             }
 
diff --git a/src/proto/streams/state.rs b/src/proto/streams/state.rs
index 3fdda30..d4b15aa 100644
--- a/src/proto/streams/state.rs
+++ b/src/proto/streams/state.rs
@@ -361,6 +361,13 @@ impl State {
         }
     }
 
+    pub fn is_send_closed(&self) -> bool {
+        match self.inner {
+            Closed(..) | HalfClosedLocal(..) | ReservedRemote => true,
+            _ => false,
+        }
+    }
+
     pub fn is_idle(&self) -> bool {
         match self.inner {
             Idle => true,
diff --git a/tests/stream_states.rs b/tests/stream_states.rs
index e599da0..d01e5bb 100644
--- a/tests/stream_states.rs
+++ b/tests/stream_states.rs
@@ -715,6 +715,58 @@ fn rst_stream_max() {
         });
 
 
+    client.join(srv).wait().expect("wait");
+}
+
+#[test]
+fn reserved_state_recv_window_update() {
+    let _ = ::env_logger::init();
+    let (io, srv) = mock::new();
+
+    let srv = srv.assert_client_handshake()
+        .unwrap()
+        .recv_settings()
+        .recv_frame(
+            frames::headers(1)
+                .request("GET", "https://example.com/")
+                .eos(),
+        )
+        .send_frame(
+            frames::push_promise(1, 2)
+                .request("GET", "https://example.com/push")
+        )
+        // it'd be weird to send a window update on a push promise,
+        // since the client can't send us data, but whatever. The
+        // point is that it's allowed, so we're testing it.
+        .send_frame(frames::window_update(2, 128))
+        .send_frame(frames::headers(1).response(200).eos())
+        // ping pong to ensure no goaway
+        .ping_pong([1; 8])
+        .close();
+
+    let client = Client::handshake(io)
+        .expect("handshake")
+        .and_then(|(mut client, conn)| {
+            let request = Request::builder()
+                .method(Method::GET)
+                .uri("https://example.com/")
+                .body(())
+                .unwrap();
+
+            let req = client.send_request(request, true)
+                .unwrap()
+                .0.expect("response")
+                .and_then(|resp| {
+                    assert_eq!(resp.status(), StatusCode::OK);
+                    Ok(())
+                });
+
+
+            conn.drive(req)
+                .and_then(|(conn, _)| conn.expect("client"))
+        });
+
+
     client.join(srv).wait().expect("wait");
 }
 /*