diff --git a/h2_bundle.go b/h2_bundle.go index aa58c1d8..06f0afc2 100644 --- a/h2_bundle.go +++ b/h2_bundle.go @@ -5532,6 +5532,7 @@ func (sc *http2serverConn) closeStream(st *http2stream, err error) { p.CloseWithError(err) } + st.cancelCtx() st.cw.Close() // signals Handler's CloseNotifier, unblocks writes, etc sc.writeSched.CloseStream(st.id) } diff --git a/h2_bundle_server_test.go b/h2_bundle_server_test.go new file mode 100644 index 00000000..bfcabfa3 --- /dev/null +++ b/h2_bundle_server_test.go @@ -0,0 +1,118 @@ +//go:build !nethttpomithttp2 + +package http + +import ( + "bytes" + "net" + "testing" + "time" + + "github.com/bogdanfinn/fhttp/http2/hpack" +) + +// bundledServerConn is a raw client connection to the HTTP/2 server bundled +// in h2_bundle.go, for tests that need frames a real client would not send. +type bundledServerConn struct { + t *testing.T + fr *http2Framer + hbuf bytes.Buffer + encoder *hpack.Encoder +} + +func newBundledServerConn(t *testing.T, handler HandlerFunc) *bundledServerConn { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go func() { + c, err := ln.Accept() + if err != nil { + return + } + defer c.Close() + new(http2Server).ServeConn(c, &http2ServeConnOpts{Handler: handler}) + }() + c, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + ln.Close() + t.Fatal(err) + } + t.Cleanup(func() { + c.Close() + ln.Close() + }) + c.SetDeadline(time.Now().Add(10 * time.Second)) + + bc := &bundledServerConn{t: t, fr: http2NewFramer(c, c)} + bc.encoder = hpack.NewEncoder(&bc.hbuf) + if _, err := c.Write(http2clientPreface); err != nil { + t.Fatal(err) + } + if err := bc.fr.WriteSettings(); err != nil { + t.Fatal(err) + } + for { + f, err := bc.fr.ReadFrame() + if err != nil { + t.Fatalf("reading server SETTINGS: %v", err) + } + if sf, ok := f.(*http2SettingsFrame); ok && !sf.IsAck() { + break + } + } + if err := bc.fr.WriteSettingsAck(); err != nil { + t.Fatal(err) + } + return bc +} + +func (bc *bundledServerConn) writeRequestHeaders(streamID uint32, endStream bool) { + bc.t.Helper() + bc.hbuf.Reset() + for _, kv := range [][2]string{ + {":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/"}, + } { + bc.encoder.WriteField(hpack.HeaderField{Name: kv[0], Value: kv[1]}) + } + if err := bc.fr.WriteHeaders(http2HeadersFrameParam{ + StreamID: streamID, + BlockFragment: bc.hbuf.Bytes(), + EndStream: endStream, + EndHeaders: true, + }); err != nil { + bc.t.Fatal(err) + } +} + +// A stream error must cancel the handler's request context +// (golang.org/issue/67036). This mirrors TestServerRequestCancelOnError +// in package http2. +func TestBundledServerRequestCancelOnError(t *testing.T) { + recvc := make(chan struct{}) // handler has started + donec := make(chan struct{}) // handler has finished + bc := newBundledServerConn(t, func(w ResponseWriter, r *Request) { + close(recvc) + <-r.Context().Done() + close(donec) + }) + + bc.writeRequestHeaders(1, true) + select { + case <-recvc: + case <-time.After(5 * time.Second): + t.Fatal("handler did not start") + } + + // A second set of request headers on the stream is a stream error. + bc.writeRequestHeaders(1, true) + select { + case <-donec: + case <-time.After(5 * time.Second): + t.Fatal("handler context was not canceled after the stream was reset") + } +} diff --git a/http2/server.go b/http2/server.go index 9173f1c4..81513b6f 100644 --- a/http2/server.go +++ b/http2/server.go @@ -1541,6 +1541,7 @@ func (sc *serverConn) closeStream(st *stream, err error) { p.CloseWithError(err) } + st.cancelCtx() st.cw.Close() // signals Handler's CloseNotifier, unblocks writes, etc sc.writeSched.CloseStream(st.id) } diff --git a/http2/server_test.go b/http2/server_test.go index 419892bd..d133444d 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -4327,3 +4327,39 @@ func TestServerWindowUpdateOnBodyClose(t *testing.T) { t.Error(err) } } + +// Issue 67036: A stream error should result in the handler's request context being canceled. +func TestServerRequestCancelOnError(t *testing.T) { + recvc := make(chan struct{}) // handler has started + donec := make(chan struct{}) // handler has finished + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + close(recvc) + <-r.Context().Done() + close(donec) + }) + defer st.Close() + + st.writePreface() + st.writeInitialSettings() + st.writeSettingsAck() + + // Client sends request headers, handler starts. + st.writeHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: st.encodeHeader(), + EndStream: true, + EndHeaders: true, + }) + <-recvc + + // Client sends an invalid second set of request headers. + // The stream is reset. + // The handler's context is canceled, and the handler exits. + st.writeHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: st.encodeHeader(), + EndStream: true, + EndHeaders: true, + }) + <-donec +}