Commit 36477291 authored by Brad Fitzpatrick's avatar Brad Fitzpatrick

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
parent 18d64411
...@@ -2062,30 +2062,35 @@ func TestServerReaderFromOrder(t *testing.T) { ...@@ -2062,30 +2062,35 @@ func TestServerReaderFromOrder(t *testing.T) {
} }
} }
// Issue 6157 // Issue 6157, Issue 6685
func TestNoContentTypeOnNotModified(t *testing.T) { func TestCodesPreventingContentTypeAndBody(t *testing.T) {
ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) { for _, code := range []int{StatusNotModified, StatusNoContent, StatusContinue} {
if r.URL.Path == "/header" { ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) {
w.Header().Set("Content-Length", "123") if r.URL.Path == "/header" {
} w.Header().Set("Content-Length", "123")
w.WriteHeader(StatusNotModified) }
if r.URL.Path == "/more" { w.WriteHeader(code)
w.Write([]byte("stuff")) if r.URL.Path == "/more" {
} w.Write([]byte("stuff"))
})) }
for _, req := range []string{ }))
"GET / HTTP/1.0", for _, req := range []string{
"GET /header HTTP/1.0", "GET / HTTP/1.0",
"GET /more HTTP/1.0", "GET /header HTTP/1.0",
"GET / HTTP/1.1", "GET /more HTTP/1.0",
"GET /header HTTP/1.1", "GET / HTTP/1.1",
"GET /more HTTP/1.1", "GET /header HTTP/1.1",
} { "GET /more HTTP/1.1",
got := ht.rawResponse(req) } {
if !strings.Contains(got, "304 Not Modified") { got := ht.rawResponse(req)
t.Errorf("Non-304 Not Modified for %q: %s", req, got) wantStatus := fmt.Sprintf("%d %s", code, StatusText(code))
} else if strings.Contains(got, "Content-Length") { if !strings.Contains(got, wantStatus) {
t.Errorf("Got a Content-Length from %q: %s", req, got) 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)
}
} }
} }
} }
......
...@@ -735,7 +735,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { ...@@ -735,7 +735,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
// response header and this is our first (and last) write, set // response header and this is our first (and last) write, set
// it, even to zero. This helps HTTP/1.0 clients keep their // it, even to zero. This helps HTTP/1.0 clients keep their
// "keep-alive" connections alive. // "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 // it was a HEAD request, we don't know the difference between
// 0 actual bytes and 0 bytes because the handler noticed it // 0 actual bytes and 0 bytes because the handler noticed it
// was a HEAD request and chose not to write anything. So for // was a HEAD request and chose not to write anything. So for
...@@ -743,7 +743,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { ...@@ -743,7 +743,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
// write non-zero bytes. If it's actually 0 bytes and the // write non-zero bytes. If it's actually 0 bytes and the
// handler never looked at the Request.Method, we just don't // handler never looked at the Request.Method, we just don't
// send a Content-Length header. // 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)) w.contentLength = int64(len(p))
setHeader.contentLength = strconv.AppendInt(cw.res.clenBuf[:0], int64(len(p)), 10) setHeader.contentLength = strconv.AppendInt(cw.res.clenBuf[:0], int64(len(p)), 10)
} }
...@@ -792,7 +792,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { ...@@ -792,7 +792,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
} }
code := w.status code := w.status
if code == StatusNotModified { if !bodyAllowedForStatus(code) {
// Must not have body. // Must not have body.
// RFC 2616 section 10.3.5: "the response MUST NOT include other entity-headers" // RFC 2616 section 10.3.5: "the response MUST NOT include other entity-headers"
for _, k := range []string{"Content-Type", "Content-Length", "Transfer-Encoding"} { for _, k := range []string{"Content-Type", "Content-Length", "Transfer-Encoding"} {
...@@ -821,7 +821,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { ...@@ -821,7 +821,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
hasCL = false hasCL = false
} }
if w.req.Method == "HEAD" || code == StatusNotModified { if w.req.Method == "HEAD" || !bodyAllowedForStatus(code) {
// do nothing // do nothing
} else if code == StatusNoContent { } else if code == StatusNoContent {
delHeader("Transfer-Encoding") delHeader("Transfer-Encoding")
...@@ -915,7 +915,7 @@ func (w *response) bodyAllowed() bool { ...@@ -915,7 +915,7 @@ func (w *response) bodyAllowed() bool {
if !w.wroteHeader { if !w.wroteHeader {
panic("") panic("")
} }
return w.status != StatusNotModified return bodyAllowedForStatus(w.status)
} }
// The Life Of A Write is like this: // The Life Of A Write is like this:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment