http2: make Server send GOAWAY if Handler sets "Connection: close" header
In Go's HTTP/1.x Server, a "Connection: close" response from a handler results
in the TCP connection being closed.
In HTTP/2, a "Connection" header is illegal and we weren't previously
handling it, generating malformed responses to clients. (This is one
of our violations listed in golang/go#25023)
There was also a feature request in golang/go#20977 for a way for
HTTP/2 handlers to close the connection after the response. Since we
already close the connection for "Connection: close" for HTTP/1.x, do
the same for HTTP/2 and map it to a graceful GOAWAY errcode=NO
response.
Updates golang/go#25023 (improves 8.1.2.2. Connection-Specific Header Fields)
Updates golang/go#20977 (fixes after vendor into std)
Change-Id: Iefb33ea73be616052533080c63b54ae679b1d154
Reviewed-on: https://go-review.googlesource.com/121415
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 769c0fe..7938991 100644
--- a/http2/server.go
+++ b/http2/server.go
@@ -2347,6 +2347,19 @@
foreachHeaderElement(v, rws.declareTrailer)
}
+ // "Connection" headers aren't allowed in HTTP/2 (RFC 7540, 8.1.2.2),
+ // but respect "Connection" == "close" to mean sending a GOAWAY and tearing
+ // down the TCP connection when idle, like we do for HTTP/1.
+ // TODO: remove more Connection-specific header fields here, in addition
+ // to "Connection".
+ if _, ok := rws.snapHeader["Connection"]; ok {
+ v := rws.snapHeader.Get("Connection")
+ delete(rws.snapHeader, "Connection")
+ if v == "close" {
+ rws.conn.startGracefulShutdown()
+ }
+ }
+
endStream := (rws.handlerDone && !rws.hasTrailers() && len(p) == 0) || isHeadResp
err = rws.conn.writeHeaders(rws.stream, &writeResHeaders{
streamID: rws.stream.id,
diff --git a/http2/server_test.go b/http2/server_test.go
index 9d7b200..02eb0dc 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -3779,3 +3779,65 @@
st.wantRSTStream(1, ErrCodeProtocol)
})
}
+
+// Tests that a handler setting "Connection: close" results in a GOAWAY being sent,
+// and the connection still completing.
+func TestServerHandlerConnectionClose(t *testing.T) {
+ unblockHandler := make(chan bool, 1)
+ defer close(unblockHandler) // backup; in case of errors
+ testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error {
+ w.Header().Set("Connection", "close")
+ w.Header().Set("Foo", "bar")
+ w.(http.Flusher).Flush()
+ <-unblockHandler
+ return nil
+ }, func(st *serverTester) {
+ st.writeHeaders(HeadersFrameParam{
+ StreamID: 1,
+ BlockFragment: st.encodeHeader(),
+ EndStream: true,
+ EndHeaders: true,
+ })
+ var sawGoAway bool
+ var sawRes bool
+ for {
+ f, err := st.readFrame()
+ if err == io.EOF {
+ break
+ }
+ if err != nil {
+ t.Fatal(err)
+ }
+ switch f := f.(type) {
+ case *GoAwayFrame:
+ sawGoAway = true
+ unblockHandler <- true
+ if f.LastStreamID != 1 || f.ErrCode != ErrCodeNo {
+ t.Errorf("unexpected GOAWAY frame: %v", summarizeFrame(f))
+ }
+ case *HeadersFrame:
+ goth := st.decodeHeader(f.HeaderBlockFragment())
+ if !sawGoAway {
+ t.Fatalf("unexpected Headers frame before GOAWAY: %s, %v", summarizeFrame(f), goth)
+ }
+ wanth := [][2]string{
+ {":status", "200"},
+ {"foo", "bar"},
+ }
+ if !reflect.DeepEqual(goth, wanth) {
+ t.Errorf("got headers %v; want %v", goth, wanth)
+ }
+ sawRes = true
+ case *DataFrame:
+ if f.StreamID != 1 || !f.StreamEnded() || len(f.Data()) != 0 {
+ t.Errorf("unexpected DATA frame: %v", summarizeFrame(f))
+ }
+ default:
+ t.Logf("unexpected frame: %v", summarizeFrame(f))
+ }
+ }
+ if !sawRes {
+ t.Errorf("didn't see response")
+ }
+ })
+}
