From b1050542c1dcff9f5902ab2745aae3ccc8340c11 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 3 Nov 2015 12:04:20 -0800 Subject: [PATCH] net/http: don't panic after request if Handler sets Request.Body to nil The Server's server goroutine was panicing (but recovering) when cleaning up after handling a request. It was pretty harmless (it just closed that one connection and didn't kill the whole process) but it was distracting. Updates #13135 Change-Id: I2a0ce9e8b52c8d364e3f4ce245e05c6f8d62df14 Reviewed-on: https://go-review.googlesource.com/16572 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Andrew Gerrand --- src/net/http/serve_test.go | 25 +++++++++++++++++++++++++ src/net/http/server.go | 8 +++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 3bd3db4a69..f3454848b7 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -3381,6 +3381,31 @@ func TestHandlerFinishSkipBigContentLengthRead(t *testing.T) { } } +func TestHandlerSetsBodyNil(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + r.Body = nil + fmt.Fprintf(w, "%v", r.RemoteAddr) + })) + defer ts.Close() + get := func() string { + res, err := Get(ts.URL) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + slurp, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + return string(slurp) + } + a, b := get(), get() + if a != b { + t.Errorf("Failed to reuse connections between requests: %v vs %v", a, b) + } +} + func BenchmarkClientServer(b *testing.B) { b.ReportAllocs() b.StopTimer() diff --git a/src/net/http/server.go b/src/net/http/server.go index e8470efd6b..979b2eb1e5 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -317,8 +317,9 @@ func (cw *chunkWriter) close() { type response struct { conn *conn req *Request // request for this response - wroteHeader bool // reply header has been (logically) written - wroteContinue bool // 100 Continue response was written + reqBody io.ReadCloser + wroteHeader bool // reply header has been (logically) written + wroteContinue bool // 100 Continue response was written w *bufio.Writer // buffers output in chunks to chunkWriter cw chunkWriter @@ -658,6 +659,7 @@ func (c *conn) readRequest() (w *response, err error) { w = &response{ conn: c, req: req, + reqBody: req.Body, handlerHeader: make(Header), contentLength: -1, } @@ -1167,7 +1169,7 @@ func (w *response) finishRequest() { // Close the body (regardless of w.closeAfterReply) so we can // re-use its bufio.Reader later safely. - w.req.Body.Close() + w.reqBody.Close() if w.req.MultipartForm != nil { w.req.MultipartForm.RemoveAll() -- 2.48.1