Skip to content

Commit

Permalink
net/http: don't panic after request if Handler sets Request.Body to nil
Browse files Browse the repository at this point in the history
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 golang#13135

Change-Id: I2a0ce9e8b52c8d364e3f4ce245e05c6f8d62df14
Reviewed-on: https://go-review.googlesource.com/16572
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Andrew Gerrand <[email protected]>
  • Loading branch information
bradfitz committed Nov 4, 2015
1 parent 9179c9c commit b105054
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
25 changes: 25 additions & 0 deletions src/net/http/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
8 changes: 5 additions & 3 deletions src/net/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit b105054

Please sign in to comment.