mirror of
https://github.com/golang/go
synced 2024-11-17 16:04:47 -07:00
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 <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:
parent
ead3fe0dba
commit
2a463a22ce
@ -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
|
||||||
|
@ -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()
|
||||||
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user