qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-26 09:28:07 -07:00
Code Releases Activity

net/http: close request body after recovering from a handler panic

Browse Source 
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 <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
Damien Neil 2021-06-21 20:23:36 -07:00
parent ead3fe0dba
commit 2a463a22ce
2 changed files with 39 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/net/http/server.go
View File

@ -1794,6 +1794,7 @@ func isCommonNetReadError(err error) bool {
func (c *conn) serve(ctx context.Context) { func (c *conn) serve(ctx context.Context) {
c.remoteAddr = c.rwc.RemoteAddr().String() c.remoteAddr = c.rwc.RemoteAddr().String()
ctx = context.WithValue(ctx, LocalAddrContextKey, c.rwc.LocalAddr()) ctx = context.WithValue(ctx, LocalAddrContextKey, c.rwc.LocalAddr())
var inFlightResponse *response
defer func() { defer func() {
if err := recover(); err != nil && err != ErrAbortHandler { if err := recover(); err != nil && err != ErrAbortHandler {
const size = 64 << 10 const size = 64 << 10
@ -1801,7 +1802,14 @@ func (c *conn) serve(ctx context.Context) {
buf = buf[:runtime.Stack(buf, false)] buf = buf[:runtime.Stack(buf, false)]
c.server.logf("http: panic serving %v: %v\n%s", c.remoteAddr, err, buf) c.server.logf("http: panic serving %v: %v\n%s", c.remoteAddr, err, buf)
} }
if inFlightResponse != nil {
inFlightResponse.cancelCtx()
}
if !c.hijacked() { if !c.hijacked() {
if inFlightResponse != nil {
inFlightResponse.conn.r.abortPendingRead()
inFlightResponse.reqBody.Close()
}
c.close() c.close()
c.setState(c.rwc, StateClosed, runHooks) c.setState(c.rwc, StateClosed, runHooks)
} }
@ -1926,7 +1934,9 @@ func (c *conn) serve(ctx context.Context) {
// in parallel even if their responses need to be serialized. // in parallel even if their responses need to be serialized.
// But we're not going to implement HTTP pipelining because it // But we're not going to implement HTTP pipelining because it
// was never deployed in the wild and the answer is HTTP/2. // was never deployed in the wild and the answer is HTTP/2.
inFlightResponse = w
serverHandler{c.server}.ServeHTTP(w, w.req) serverHandler{c.server}.ServeHTTP(w, w.req)
inFlightResponse = nil
w.cancelCtx() w.cancelCtx()
if c.hijacked() { if c.hijacked() {
return return

29
src/net/http/transport_test.go
View File

@ -6512,3 +6512,32 @@ func TestCancelRequestWhenSharingConnection(t *testing.T) {
close(r2c) close(r2c)
wg.Wait() wg.Wait()
} }
func TestHandlerAbortRacesBodyRead(t *testing.T) {
setParallel(t)
defer afterTest(t)
ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) {
go io.Copy(io.Discard, req.Body)
panic(ErrAbortHandler)
}))
defer ts.Close()
var wg sync.WaitGroup
for i := 0; i < 2; i++ {
wg.Add(1)
go func() {
defer wg.Done()
for j := 0; j < 10; j++ {
const reqLen = 6 * 1024 * 1024
req, _ := NewRequest("POST", ts.URL, &io.LimitedReader{R: neverEnding('x'), N: reqLen})
req.ContentLength = reqLen
resp, _ := ts.Client().Transport.RoundTrip(req)
if resp != nil {
resp.Body.Close()
}
}
}()
}
wg.Wait()
}