From 18ae4c834bdb33903dbf6774f57536c73de923bb Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Mon, 27 Nov 2017 22:48:11 +0000
Subject: net/http: panic on invalid WriteHeader status code

Panic if an http Handler does:

    rw.WriteHeader(0)

... or other invalid values. (for a forgiving range of valid)

I previously made it kinda work in https://golang.org/cl/19130 but
there's no good way to fake it in HTTP/2, and we want HTTP/1 and
HTTP/2 behavior to be the same, regardless of what programs do.
Currently HTTP/2 omitted the :status header altogether, which was a
protocol violation. In fixing that, I found CL 19130 added a test
about bogus WriteHeader values with the comment:

  // This might change at some point, but not yet in Go 1.6.

This now changes. Time to be strict.

Updates golang/go#228800

Change-Id: I20eb6c0e514a31f4bba305ac4c24266f39b95fd5
Reviewed-on: https://go-review.googlesource.com/80077
Reviewed-by: Tom Bergan <tombergan@google.com>
---
 src/net/http/clientserver_test.go | 61 +++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 21 deletions(-)

(limited to 'src/net/http/clientserver_test.go')

diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go
index 8738c8ff7c..5017ebe468 100644
--- a/src/net/http/clientserver_test.go
+++ b/src/net/http/clientserver_test.go
@@ -1141,27 +1141,6 @@ func testTransportRejectsInvalidHeaders(t *testing.T, h2 bool) {
 	}
 }
 
-// Tests that we support bogus under-100 HTTP statuses, because we historically
-// have. This might change at some point, but not yet in Go 1.6.
-func TestBogusStatusWorks_h1(t *testing.T) { testBogusStatusWorks(t, h1Mode) }
-func TestBogusStatusWorks_h2(t *testing.T) { testBogusStatusWorks(t, h2Mode) }
-func testBogusStatusWorks(t *testing.T, h2 bool) {
-	defer afterTest(t)
-	const code = 7
-	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
-		w.WriteHeader(code)
-	}))
-	defer cst.close()
-
-	res, err := cst.c.Get(cst.ts.URL)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if res.StatusCode != code {
-		t.Errorf("StatusCode = %d; want %d", res.StatusCode, code)
-	}
-}
-
 func TestInterruptWithPanic_h1(t *testing.T)     { testInterruptWithPanic(t, h1Mode, "boom") }
 func TestInterruptWithPanic_h2(t *testing.T)     { testInterruptWithPanic(t, h2Mode, "boom") }
 func TestInterruptWithPanic_nil_h1(t *testing.T) { testInterruptWithPanic(t, h1Mode, nil) }
@@ -1408,3 +1387,43 @@ func TestBadResponseAfterReadingBody(t *testing.T) {
 		t.Errorf("closes = %d; want 1", closes)
 	}
 }
+
+func TestWriteHeader0_h1(t *testing.T) { testWriteHeader0(t, h1Mode) }
+func TestWriteHeader0_h2(t *testing.T) { testWriteHeader0(t, h2Mode) }
+func testWriteHeader0(t *testing.T, h2 bool) {
+	if h2 {
+		t.Skip("skipping until CL 80076 is vendored into std")
+	}
+	defer afterTest(t)
+	gotpanic := make(chan bool, 1)
+	cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
+		defer close(gotpanic)
+		defer func() {
+			if e := recover(); e != nil {
+				got := fmt.Sprintf("%T, %v", e, e)
+				want := "string, invalid WriteHeader code 0"
+				if got != want {
+					t.Errorf("unexpected panic value:\n got: %v\nwant: %v\n", got, want)
+				}
+				gotpanic <- true
+
+				// Set an explicit 503. This also tests that the WriteHeader call panics
+				// before it recorded that an explicit value was set and that bogus
+				// value wasn't stuck.
+				w.WriteHeader(503)
+			}
+		}()
+		w.WriteHeader(0)
+	}))
+	defer cst.close()
+	res, err := cst.c.Get(cst.ts.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res.StatusCode != 503 {
+		t.Errorf("Response: %v %q; want 503", res.StatusCode, res.Status)
+	}
+	if !<-gotpanic {
+		t.Error("expected panic in handler")
+	}
+}
-- 
cgit v1.3