http2: make Server return conn protocol errors on bad idle stream frames
Per RFC 7540, section 5.1 discussing the "idle" state: "Receiving any
frame other than HEADERS or PRIORITY on a stream in this state MUST be
treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR."x
Updates golang/go#16046
Change-Id: Ie0696059e76a092e483aea5ee017d9729339d309
Reviewed-on: https://go-review.googlesource.com/31736
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/http2/server.go b/http2/server.go
index 11d3b65..0e670de 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -1122,7 +1122,14 @@
sc.serveG.check()
switch {
case f.StreamID != 0: // stream-level flow control
- st := sc.streams[f.StreamID]
+ state, st := sc.state(f.StreamID)
+ if state == stateIdle {
+ // Section 5.1: "Receiving any frame other than HEADERS
+ // or PRIORITY on a stream in this state MUST be
+ // treated as a connection error (Section 5.4.1) of
+ // type PROTOCOL_ERROR."
+ return ConnectionError(ErrCodeProtocol)
+ }
if st == nil {
// "WINDOW_UPDATE can be sent by a peer that has sent a
// frame bearing the END_STREAM flag. This means that a
@@ -1274,8 +1281,15 @@
// or "half closed (local)" state, the recipient MUST respond
// with a stream error (Section 5.4.2) of type STREAM_CLOSED."
id := f.Header().StreamID
- st, ok := sc.streams[id]
- if !ok || st.state != stateOpen || st.gotTrailerHeader {
+ state, st := sc.state(id)
+ if id == 0 || state == stateIdle {
+ // Section 5.1: "Receiving any frame other than HEADERS
+ // or PRIORITY on a stream in this state MUST be
+ // treated as a connection error (Section 5.4.1) of
+ // type PROTOCOL_ERROR."
+ return ConnectionError(ErrCodeProtocol)
+ }
+ if st == nil || state != stateOpen || st.gotTrailerHeader {
// This includes sending a RST_STREAM if the stream is
// in stateHalfClosedLocal (which currently means that
// the http.Handler returned, so it's done reading &
diff --git a/http2/server_test.go b/http2/server_test.go
index 22aabf2..31d12f2 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -937,7 +937,7 @@
func testRejectRequest(t *testing.T, send func(*serverTester)) {
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
- t.Fatal("server request made it to handler; should've been rejected")
+ t.Error("server request made it to handler; should've been rejected")
})
defer st.Close()
@@ -946,6 +946,39 @@
st.wantRSTStream(1, ErrCodeProtocol)
}
+func testRejectRequestWithProtocolError(t *testing.T, send func(*serverTester)) {
+ st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+ t.Error("server request made it to handler; should've been rejected")
+ }, optQuiet)
+ defer st.Close()
+
+ st.greet()
+ send(st)
+ gf := st.wantGoAway()
+ if gf.ErrCode != ErrCodeProtocol {
+ t.Errorf("err code = %v; want %v", gf.ErrCode, ErrCodeProtocol)
+ }
+}
+
+// Section 5.1, on idle connections: "Receiving any frame other than
+// HEADERS or PRIORITY on a stream in this state MUST be treated as a
+// connection error (Section 5.4.1) of type PROTOCOL_ERROR."
+func TestRejectFrameOnIdle_WindowUpdate(t *testing.T) {
+ testRejectRequestWithProtocolError(t, func(st *serverTester) {
+ st.fr.WriteWindowUpdate(123, 456)
+ })
+}
+func TestRejectFrameOnIdle_Data(t *testing.T) {
+ testRejectRequestWithProtocolError(t, func(st *serverTester) {
+ st.fr.WriteData(123, true, nil)
+ })
+}
+func TestRejectFrameOnIdle_RSTStream(t *testing.T) {
+ testRejectRequestWithProtocolError(t, func(st *serverTester) {
+ st.fr.WriteRSTStream(123, ErrCodeCancel)
+ })
+}
+
func TestServer_Request_Connect(t *testing.T) {
testServerRequest(t, func(st *serverTester) {
st.writeHeaders(HeadersFrameParam{
