net/http: keep HTTP/1.0 keep-alive conns open if response can't have a body

Fixes #15647

Change-Id: I588bfa4eb336d1da1fcda8d06e32ed13c0b51c70
Reviewed-on: https://go-review.googlesource.com/23061
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index b34875f061a06260e9018ae31b1198a869b5286e..95983e4b02e75684f8bc3e05a96a84f5ba217c9c 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -714,6 +714,31 @@ func testTCPConnectionCloses(t *testing.T, req string, h Handler) {
        }
 }
 
+func testTCPConnectionStaysOpen(t *testing.T, req string, handler Handler) {
+       defer afterTest(t)
+       ts := httptest.NewServer(handler)
+       defer ts.Close()
+       conn, err := net.Dial("tcp", ts.Listener.Addr().String())
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer conn.Close()
+       br := bufio.NewReader(conn)
+       for i := 0; i < 2; i++ {
+               if _, err := io.WriteString(conn, req); err != nil {
+                       t.Fatal(err)
+               }
+               res, err := ReadResponse(br, nil)
+               if err != nil {
+                       t.Fatalf("res %d: %v", i+1, err)
+               }
+               if _, err := io.Copy(ioutil.Discard, res.Body); err != nil {
+                       t.Fatalf("res %d body copy: %v", i+1, err)
+               }
+               res.Body.Close()
+       }
+}
+
 // TestServeHTTP10Close verifies that HTTP/1.0 requests won't be kept alive.
 func TestServeHTTP10Close(t *testing.T) {
        testTCPConnectionCloses(t, "GET / HTTP/1.0\r\n\r\n", HandlerFunc(func(w ResponseWriter, r *Request) {
@@ -749,6 +774,24 @@ func TestHTTP2UpgradeClosesConnection(t *testing.T) {
        }))
 }
 
+func send204(w ResponseWriter, r *Request) { w.WriteHeader(204) }
+func send304(w ResponseWriter, r *Request) { w.WriteHeader(304) }
+
+// Issue 15647: 204 responses can't have bodies, so HTTP/1.0 keep-alive conns should stay open.
+func TestHTTP10KeepAlive204Response(t *testing.T) {
+       testTCPConnectionStaysOpen(t, "GET / HTTP/1.0\r\nConnection: keep-alive\r\n\r\n", HandlerFunc(send204))
+}
+
+func TestHTTP11KeepAlive204Response(t *testing.T) {
+       testTCPConnectionStaysOpen(t, "GET / HTTP/1.1\r\nHost: foo\r\n\r\n", HandlerFunc(send204))
+}
+
+func TestHTTP10KeepAlive304Response(t *testing.T) {
+       testTCPConnectionStaysOpen(t,
+               "GET / HTTP/1.0\r\nConnection: keep-alive\r\nIf-Modified-Since: Mon, 02 Jan 2006 15:04:05 GMT\r\n\r\n",
+               HandlerFunc(send304))
+}
+
 func TestSetsRemoteAddr_h1(t *testing.T) { testSetsRemoteAddr(t, h1Mode) }
 func TestSetsRemoteAddr_h2(t *testing.T) { testSetsRemoteAddr(t, h2Mode) }
 

diff --git a/src/net/http/server.go b/src/net/http/server.go
index e24777421cf34188ad6004ae1aa38352517ff369..d0be7d01dbd0aad54e6c252e29fc82723c2f13c8 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1008,7 +1008,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
        // Check for a explicit (and valid) Content-Length header.
        hasCL := w.contentLength != -1
 
-       if w.wants10KeepAlive && (isHEAD || hasCL) {
+       if w.wants10KeepAlive && (isHEAD || hasCL || !bodyAllowedForStatus(w.status)) {
                _, connectionHeaderSet := header["Connection"]
                if !connectionHeaderSet {
                        setHeader.connection = "keep-alive"