net/http: don't allow Content-Type or body on 204 and 1xx

Status codes 204, 304, and 1xx don't allow bodies. We already
had a function for this, but we were hard-coding just 304
(StatusNotModified) in a few places.  Use the function
instead, and flesh out tests for all codes.

Fixes #6685

R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/53290044

diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go
index 7a066ab07abc13aa7eab37d06b8fc65d7cbc76e4..d76e8167c6ffd6c2e0c927f57d4b77e8df0521fc 100644 (file)
--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -2062,30 +2062,35 @@ func TestServerReaderFromOrder(t *testing.T) {
        }
 }
 
-// Issue 6157
-func TestNoContentTypeOnNotModified(t *testing.T) {
-       ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) {
-               if r.URL.Path == "/header" {
-                       w.Header().Set("Content-Length", "123")
-               }
-               w.WriteHeader(StatusNotModified)
-               if r.URL.Path == "/more" {
-                       w.Write([]byte("stuff"))
-               }
-       }))
-       for _, req := range []string{
-               "GET / HTTP/1.0",
-               "GET /header HTTP/1.0",
-               "GET /more HTTP/1.0",
-               "GET / HTTP/1.1",
-               "GET /header HTTP/1.1",
-               "GET /more HTTP/1.1",
-       } {
-               got := ht.rawResponse(req)
-               if !strings.Contains(got, "304 Not Modified") {
-                       t.Errorf("Non-304 Not Modified for %q: %s", req, got)
-               } else if strings.Contains(got, "Content-Length") {
-                       t.Errorf("Got a Content-Length from %q: %s", req, got)
+// Issue 6157, Issue 6685
+func TestCodesPreventingContentTypeAndBody(t *testing.T) {
+       for _, code := range []int{StatusNotModified, StatusNoContent, StatusContinue} {
+               ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) {
+                       if r.URL.Path == "/header" {
+                               w.Header().Set("Content-Length", "123")
+                       }
+                       w.WriteHeader(code)
+                       if r.URL.Path == "/more" {
+                               w.Write([]byte("stuff"))
+                       }
+               }))
+               for _, req := range []string{
+                       "GET / HTTP/1.0",
+                       "GET /header HTTP/1.0",
+                       "GET /more HTTP/1.0",
+                       "GET / HTTP/1.1",
+                       "GET /header HTTP/1.1",
+                       "GET /more HTTP/1.1",
+               } {
+                       got := ht.rawResponse(req)
+                       wantStatus := fmt.Sprintf("%d %s", code, StatusText(code))
+                       if !strings.Contains(got, wantStatus) {
+                               t.Errorf("Code %d: Wanted %q Modified for %q: %s", code, req, got)
+                       } else if strings.Contains(got, "Content-Length") {
+                               t.Errorf("Code %d: Got a Content-Length from %q: %s", code, req, got)
+                       } else if strings.Contains(got, "stuff") {
+                               t.Errorf("Code %d: Response contains a body from %q: %s", code, req, got)
+                       }
                }
        }
 }

diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go
index 778089aa3efcf98e9dde6b49f186a9c057543828..77cbee1dee3ae45185195ab22652edce7cd97e3c 100644 (file)
--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -735,7 +735,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
        // response header and this is our first (and last) write, set
        // it, even to zero. This helps HTTP/1.0 clients keep their
        // "keep-alive" connections alive.
-       // Exceptions: 304 responses never get Content-Length, and if
+       // Exceptions: 304/204/1xx responses never get Content-Length, and if
        // it was a HEAD request, we don't know the difference between
        // 0 actual bytes and 0 bytes because the handler noticed it
        // was a HEAD request and chose not to write anything.  So for
@@ -743,7 +743,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
        // write non-zero bytes.  If it's actually 0 bytes and the
        // handler never looked at the Request.Method, we just don't
        // send a Content-Length header.
-       if w.handlerDone && w.status != StatusNotModified && header.get("Content-Length") == "" && (!isHEAD || len(p) > 0) {
+       if w.handlerDone && bodyAllowedForStatus(w.status) && header.get("Content-Length") == "" && (!isHEAD || len(p) > 0) {
                w.contentLength = int64(len(p))
                setHeader.contentLength = strconv.AppendInt(cw.res.clenBuf[:0], int64(len(p)), 10)
        }
@@ -792,7 +792,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
        }
 
        code := w.status
-       if code == StatusNotModified {
+       if !bodyAllowedForStatus(code) {
                // Must not have body.
                // RFC 2616 section 10.3.5: "the response MUST NOT include other entity-headers"
                for _, k := range []string{"Content-Type", "Content-Length", "Transfer-Encoding"} {
@@ -821,7 +821,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
                hasCL = false
        }
 
-       if w.req.Method == "HEAD" || code == StatusNotModified {
+       if w.req.Method == "HEAD" || !bodyAllowedForStatus(code) {
                // do nothing
        } else if code == StatusNoContent {
                delHeader("Transfer-Encoding")
@@ -915,7 +915,7 @@ func (w *response) bodyAllowed() bool {
        if !w.wroteHeader {
                panic("")
        }
-       return w.status != StatusNotModified
+       return bodyAllowedForStatus(w.status)
 }
 
 // The Life Of A Write is like this: