Browse Source

Don't send stream WINDOW_UPDATEs when client has sent END_STREAM

It's useless to tell them they can send more, when they've already
said they're done.
Brad Fitzpatrick 11 years ago
parent
commit
0f1a865e18
2 changed files with 32 additions and 28 deletions
  1. 5 4
      server.go
  2. 27 24
      server_test.go

+ 5 - 4
server.go

@@ -1337,10 +1337,11 @@ func (sc *serverConn) noteBodyReadFromHandler(st *stream, n int) {
 func (sc *serverConn) noteBodyRead(st *stream, n int) {
 	sc.serveG.check()
 	sc.sendWindowUpdate(nil, n) // conn-level
-	// TODO: don't send this WINDOW_UPDATE if the stream is in
-	// stateClosedRemote.  No need to tell them they can send more
-	// if they've already said they're done.
-	sc.sendWindowUpdate(st, n)
+	if st.state != stateHalfClosedRemote && st.state != stateClosed {
+		// Don't send this WINDOW_UPDATE if the stream is closed
+		// remotely.
+		sc.sendWindowUpdate(st, n)
+	}
 }
 
 // st may be nil for conn-level

+ 27 - 24
server_test.go

@@ -785,33 +785,20 @@ func TestServer_Handler_Sends_WindowUpdate(t *testing.T) {
 		EndStream:     false, // data coming
 		EndHeaders:    true,
 	})
-	st.writeData(1, true, []byte("abcdef"))
-	puppet.do(func(w http.ResponseWriter, r *http.Request) {
-		buf := make([]byte, 3)
-		_, err := io.ReadFull(r.Body, buf)
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		if string(buf) != "abc" {
-			t.Errorf("read %q; want abc", buf)
-		}
-	})
+	st.writeData(1, false, []byte("abcdef"))
+	puppet.do(readBodyHandler(t, "abc"))
 	st.wantWindowUpdate(0, 3)
 	st.wantWindowUpdate(1, 3)
-	puppet.do(func(w http.ResponseWriter, r *http.Request) {
-		buf := make([]byte, 3)
-		_, err := io.ReadFull(r.Body, buf)
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		if string(buf) != "def" {
-			t.Errorf("read %q; want abc", buf)
-		}
-	})
+
+	puppet.do(readBodyHandler(t, "def"))
 	st.wantWindowUpdate(0, 3)
 	st.wantWindowUpdate(1, 3)
+
+	st.writeData(1, true, []byte("ghijkl")) // END_STREAM here
+	puppet.do(readBodyHandler(t, "ghi"))
+	puppet.do(readBodyHandler(t, "jkl"))
+	st.wantWindowUpdate(0, 3)
+	st.wantWindowUpdate(0, 3) // no more stream-level, since END_STREAM
 }
 
 func TestServer_Send_GoAway_After_Bogus_WindowUpdate(t *testing.T) {
@@ -1640,7 +1627,6 @@ func TestServer_Response_Automatic100Continue(t *testing.T) {
 		st.writeData(1, true, []byte(msg))
 
 		st.wantWindowUpdate(0, uint32(len(msg)))
-		st.wantWindowUpdate(1, uint32(len(msg)))
 
 		hf = st.wantHeaders()
 		if hf.StreamEnded() {
@@ -1889,6 +1875,23 @@ func testServerResponse(t *testing.T,
 	}
 }
 
+// readBodyHandler returns an http Handler func that reads len(want)
+// bytes from r.Body and fails t if the contents read were not
+// the value of want.
+func readBodyHandler(t *testing.T, want string) func(w http.ResponseWriter, r *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		buf := make([]byte, len(want))
+		_, err := io.ReadFull(r.Body, buf)
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		if string(buf) != want {
+			t.Errorf("read %q; want %q", buf, want)
+		}
+	}
+}
+
 func TestServerWithCurl(t *testing.T) {
 	if runtime.GOOS == "darwin" {
 		t.Skip("skipping Docker test on Darwin; requires --net which won't work with boot2docker anyway")