net/http/httptest: panic on non-3 digit (XXX) status code in Recorder.WriteHeader

This change conforms Recorder with net/http servers, to panic
when a handler writes a non-3 digit XXX status code.

Fixes #45353

Change-Id: Id5ed4af652e8c150ae86bf50402b800d935e2203
Reviewed-on: https://go-review.googlesource.com/c/go/+/308950
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>

diff --git a/src/net/http/httptest/recorder.go b/src/net/http/httptest/recorder.go
index 24284826122eb7a422a117e9c5c19be9cd65003f..1b712ef2b0a8e8a471ca69f3af76b8d56b18fe5a 100644 (file)
--- a/src/net/http/httptest/recorder.go
+++ b/src/net/http/httptest/recorder.go
@@ -122,11 +122,30 @@ func (rw *ResponseRecorder) WriteString(str string) (int, error) {
        return len(str), nil
 }
 
+func checkWriteHeaderCode(code int) {
+       // Issue 22880: require valid WriteHeader status codes.
+       // For now we only enforce that it's three digits.
+       // In the future we might block things over 599 (600 and above aren't defined
+       // at https://httpwg.org/specs/rfc7231.html#status.codes)
+       // and we might block under 200 (once we have more mature 1xx support).
+       // But for now any three digits.
+       //
+       // We used to send "HTTP/1.1 000 0" on the wire in responses but there's
+       // no equivalent bogus thing we can realistically send in HTTP/2,
+       // so we'll consistently panic instead and help people find their bugs
+       // early. (We can't return an error from WriteHeader even if we wanted to.)
+       if code < 100 || code > 999 {
+               panic(fmt.Sprintf("invalid WriteHeader code %v", code))
+       }
+}
+
 // WriteHeader implements http.ResponseWriter.
 func (rw *ResponseRecorder) WriteHeader(code int) {
        if rw.wroteHeader {
                return
        }
+
+       checkWriteHeaderCode(code)
        rw.Code = code
        rw.wroteHeader = true
        if rw.HeaderMap == nil {

diff --git a/src/net/http/httptest/recorder_test.go b/src/net/http/httptest/recorder_test.go
index a865e878b9b3bf327f804945dccb415241dbba97..8cb32dd7403683375f292ab9285d8bb1d7f33696 100644 (file)
--- a/src/net/http/httptest/recorder_test.go
+++ b/src/net/http/httptest/recorder_test.go
@@ -345,3 +345,28 @@ func TestParseContentLength(t *testing.T) {
                }
        }
 }
+
+// Ensure that httptest.Recorder panics when given a non-3 digit (XXX)
+// status HTTP code. See https://golang.org/issues/45353
+func TestRecorderPanicsOnNonXXXStatusCode(t *testing.T) {
+       badCodes := []int{
+               -100, 0, 99, 1000, 20000,
+       }
+       for _, badCode := range badCodes {
+               badCode := badCode
+               t.Run(fmt.Sprintf("Code=%d", badCode), func(t *testing.T) {
+                       defer func() {
+                               if r := recover(); r == nil {
+                                       t.Fatal("Expected a panic")
+                               }
+                       }()
+
+                       handler := func(rw http.ResponseWriter, _ *http.Request) {
+                               rw.WriteHeader(badCode)
+                       }
+                       r, _ := http.NewRequest("GET", "http://example.org/", nil)
+                       rw := NewRecorder()
+                       handler(rw, r)
+               })
+       }
+}