Skip to content

Commit 2a463a2

Browse files
committed
net/http: close request body after recovering from a handler panic
When recovering from a panic in a HTTP handler, close the request body before closing the *conn, ensuring that the *conn's bufio.Reader is safe to recycle. Fixes #46866. Change-Id: I3fe304592e3b423a0970727d68bc1229c3752939 Reviewed-on: https://go-review.googlesource.com/c/go/+/329922 Trust: Damien Neil <[email protected]> Run-TryBot: Damien Neil <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]>
1 parent ead3fe0 commit 2a463a2

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

src/net/http/server.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1794,14 +1794,22 @@ func isCommonNetReadError(err error) bool {
17941794
func (c *conn) serve(ctx context.Context) {
17951795
c.remoteAddr = c.rwc.RemoteAddr().String()
17961796
ctx = context.WithValue(ctx, LocalAddrContextKey, c.rwc.LocalAddr())
1797+
var inFlightResponse *response
17971798
defer func() {
17981799
if err := recover(); err != nil && err != ErrAbortHandler {
17991800
const size = 64 << 10
18001801
buf := make([]byte, size)
18011802
buf = buf[:runtime.Stack(buf, false)]
18021803
c.server.logf("http: panic serving %v: %v\n%s", c.remoteAddr, err, buf)
18031804
}
1805+
if inFlightResponse != nil {
1806+
inFlightResponse.cancelCtx()
1807+
}
18041808
if !c.hijacked() {
1809+
if inFlightResponse != nil {
1810+
inFlightResponse.conn.r.abortPendingRead()
1811+
inFlightResponse.reqBody.Close()
1812+
}
18051813
c.close()
18061814
c.setState(c.rwc, StateClosed, runHooks)
18071815
}
@@ -1926,7 +1934,9 @@ func (c *conn) serve(ctx context.Context) {
19261934
// in parallel even if their responses need to be serialized.
19271935
// But we're not going to implement HTTP pipelining because it
19281936
// was never deployed in the wild and the answer is HTTP/2.
1937+
inFlightResponse = w
19291938
serverHandler{c.server}.ServeHTTP(w, w.req)
1939+
inFlightResponse = nil
19301940
w.cancelCtx()
19311941
if c.hijacked() {
19321942
return

src/net/http/transport_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6512,3 +6512,32 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) {
65126512
close(r2c)
65136513
wg.Wait()
65146514
}
6515+
6516+
func TestHandlerAbortRacesBodyRead(t *testing.T) {
6517+
setParallel(t)
6518+
defer afterTest(t)
6519+
6520+
ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) {
6521+
go io.Copy(io.Discard, req.Body)
6522+
panic(ErrAbortHandler)
6523+
}))
6524+
defer ts.Close()
6525+
6526+
var wg sync.WaitGroup
6527+
for i := 0; i < 2; i++ {
6528+
wg.Add(1)
6529+
go func() {
6530+
defer wg.Done()
6531+
for j := 0; j < 10; j++ {
6532+
const reqLen = 6 * 1024 * 1024
6533+
req, _ := NewRequest("POST", ts.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen})
6534+
req.ContentLength = reqLen
6535+
resp, _ := ts.Client().Transport.RoundTrip(req)
6536+
if resp != nil {
6537+
resp.Body.Close()
6538+
}
6539+
}
6540+
}()
6541+
}
6542+
wg.Wait()
6543+
}

0 commit comments

Comments
 (0)