From c48da250e237b8355b2943875449ef071bee07bb Mon Sep 17 00:00:00 2001 From: "Nicholas S. Husin" Date: Wed, 18 Mar 2026 15:34:19 -0400 Subject: [PATCH] internal/http3: panic in server handler if given status that is not 3 digits Our HTTP/1 and HTTP/2 server implementations will panic when WriteHeader is called from a handler with a status code that is not 3 digits. For consistency, do this too for HTTP/3. For golang/go#70914 Change-Id: I65aca23ea186481ae06c7388db487e476a6a6964 Reviewed-on: https://go-review.googlesource.com/c/net/+/759300 Reviewed-by: Nicholas Husin Reviewed-by: Damien Neil LUCI-TryBot-Result: Go LUCI --- internal/http3/server.go | 19 ++++++++++++++++ internal/http3/server_test.go | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/internal/http3/server.go b/internal/http3/server.go index d6a366be..7b68684e 100644 --- a/internal/http3/server.go +++ b/internal/http3/server.go @@ -7,6 +7,7 @@ package http3 import ( "context" "crypto/tls" + "fmt" "io" "maps" "net/http" @@ -485,6 +486,23 @@ func isInfoStatus(status int) bool { return status >= 100 && status < 200 } +// checkWriteHeaderCode is a copy of net/http's checkWriteHeaderCode. +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 http://httpwg.org/specs/rfc7231.html#status.codes). + // 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/3, + // 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)) + } +} + func (rw *responseWriter) WriteHeader(statusCode int) { // TODO: handle sending informational status headers (e.g. 103). rw.mu.Lock() @@ -492,6 +510,7 @@ func (rw *responseWriter) WriteHeader(statusCode int) { if rw.statusCodeSet { return } + checkWriteHeaderCode(statusCode) // Informational headers can be sent multiple times, and should be flushed // immediately. diff --git a/internal/http3/server_test.go b/internal/http3/server_test.go index 6d6fec62..9529d624 100644 --- a/internal/http3/server_test.go +++ b/internal/http3/server_test.go @@ -6,6 +6,7 @@ package http3 import ( "errors" + "fmt" "io" "maps" "net/http" @@ -163,6 +164,47 @@ func TestServerInvalidHeader(t *testing.T) { }) } +func TestServerInvalidStatus(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + gotpanic := make(chan bool) + ts := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.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. + w.WriteHeader(503) + + // Verify that writing invalid status will not panic if a + // status is already set anyways. + w.WriteHeader(0) + } + }() + w.WriteHeader(0) // Invalid. Will panic. + })) + tc := ts.connect() + tc.greet() + + reqStream := tc.newStream(streamTypeRequest) + reqStream.writeHeaders(requestHeader(nil)) + if !<-gotpanic { + t.Error("expected panic in handler") + } + synctest.Wait() + reqStream.wantSomeHeaders(http.Header{ + ":status": {"503"}, + }) + reqStream.wantClosed("request is complete") + }) +} + func TestServerBody(t *testing.T) { synctest.Test(t, func(t *testing.T) { ts := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {