Skip to content

Commit 596e3d9

Browse files
committed
net/http: don't validate WriteHeader code if header's already been sent
Also vendors x/net/http git rev 42fe2e1c for: http2: don't check WriteHeader status if we've already sent the header https://golang.org/cl/86255 Fixes #23010 Change-Id: I4f3dd63acb52d5a34a0350aaf847a7a376d6968f Reviewed-on: https://go-review.googlesource.com/86275 Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 301714d commit 596e3d9

File tree

3 files changed

+58
-2
lines changed

3 files changed

+58
-2
lines changed

src/net/http/clientserver_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,3 +1424,59 @@ func testWriteHeader0(t *testing.T, h2 bool) {
14241424
t.Error("expected panic in handler")
14251425
}
14261426
}
1427+
1428+
// Issue 23010: don't be super strict checking WriteHeader's code if
1429+
// it's not even valid to call WriteHeader then anyway.
1430+
func TestWriteHeaderNoCodeCheck_h1(t *testing.T) { testWriteHeaderAfterWrite(t, h1Mode, false) }
1431+
func TestWriteHeaderNoCodeCheck_h1hijack(t *testing.T) { testWriteHeaderAfterWrite(t, h1Mode, true) }
1432+
func TestWriteHeaderNoCodeCheck_h2(t *testing.T) { testWriteHeaderAfterWrite(t, h2Mode, false) }
1433+
func testWriteHeaderAfterWrite(t *testing.T, h2, hijack bool) {
1434+
setParallel(t)
1435+
defer afterTest(t)
1436+
1437+
var errorLog lockedBytesBuffer
1438+
cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) {
1439+
if hijack {
1440+
conn, _, _ := w.(Hijacker).Hijack()
1441+
defer conn.Close()
1442+
conn.Write([]byte("HTTP/1.1 200 OK\r\nContent-Length: 6\r\n\r\nfoo"))
1443+
w.WriteHeader(0) // verify this doesn't panic if there's already output; Issue 23010
1444+
conn.Write([]byte("bar"))
1445+
return
1446+
}
1447+
io.WriteString(w, "foo")
1448+
w.(Flusher).Flush()
1449+
w.WriteHeader(0) // verify this doesn't panic if there's already output; Issue 23010
1450+
io.WriteString(w, "bar")
1451+
}), func(ts *httptest.Server) {
1452+
ts.Config.ErrorLog = log.New(&errorLog, "", 0)
1453+
})
1454+
defer cst.close()
1455+
res, err := cst.c.Get(cst.ts.URL)
1456+
if err != nil {
1457+
t.Fatal(err)
1458+
}
1459+
defer res.Body.Close()
1460+
body, err := ioutil.ReadAll(res.Body)
1461+
if err != nil {
1462+
t.Fatal(err)
1463+
}
1464+
if got, want := string(body), "foobar"; got != want {
1465+
t.Errorf("got = %q; want %q", got, want)
1466+
}
1467+
1468+
// Also check the stderr output:
1469+
if h2 {
1470+
// TODO: also emit this log message for HTTP/2?
1471+
// We historically haven't, so don't check.
1472+
return
1473+
}
1474+
gotLog := strings.TrimSpace(errorLog.String())
1475+
wantLog := "http: multiple response.WriteHeader calls"
1476+
if hijack {
1477+
wantLog = "http: response.WriteHeader on hijacked connection"
1478+
}
1479+
if gotLog != wantLog {
1480+
t.Errorf("stderr output = %q; want %q", gotLog, wantLog)
1481+
}
1482+
}

src/net/http/h2_bundle.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/net/http/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,6 @@ func checkWriteHeaderCode(code int) {
10721072
}
10731073

10741074
func (w *response) WriteHeader(code int) {
1075-
checkWriteHeaderCode(code)
10761075
if w.conn.hijacked() {
10771076
w.conn.server.logf("http: response.WriteHeader on hijacked connection")
10781077
return
@@ -1081,6 +1080,7 @@ func (w *response) WriteHeader(code int) {
10811080
w.conn.server.logf("http: multiple response.WriteHeader calls")
10821081
return
10831082
}
1083+
checkWriteHeaderCode(code)
10841084
w.wroteHeader = true
10851085
w.status = code
10861086

0 commit comments

Comments
 (0)