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>

http2/server.go

@@ -2347,6 +2347,19 @@ func (rws *responseWriterState) writeChunk(p []byte) (n int, err error) {
 			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,

http2/server_test.go

@@ -3779,3 +3779,65 @@ func TestServer_Rejects_TooSmall(t *testing.T) {
 		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")
+		}
+	})
+}