From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Thu, 16 Jan 2014 19:43:52 +0000 (-0800)
Subject: net/http: don't allow Content-Type or body on 204 and 1xx
X-Git-Tag: go1.3beta1~953
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=36477291cc13313e816cccfcfa62a6bc0ac43d15;p=gostls13.git

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 7a066ab07a..d76e8167c6 100644
--- 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 778089aa3e..77cbee1dee 100644
--- 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: