From 468851f1d57eb5cd3ec0ec6d3ce306ea5749090b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 3 Apr 2013 10:31:12 -0700 Subject: [PATCH] net/http: don't allocate 0-byte io.LimitedReaders for GET requests Save an allocation per GET request and don't call io.LimitedReader(r, 0) just to read 0 bytes. There's already an eofReader global variable for when we just want a non-nil io.Reader to immediately EOF. (Sorry, I know Rob told me to stop, but I was bored on the plane and wrote this before I received the recent "please, really stop" email.) benchmark old ns/op new ns/op delta BenchmarkServerHandlerTypeLen 13888 13279 -4.39% BenchmarkServerHandlerNoLen 12912 12229 -5.29% BenchmarkServerHandlerNoType 13348 12632 -5.36% BenchmarkServerHandlerNoHeader 10911 10261 -5.96% benchmark old allocs new allocs delta BenchmarkServerHandlerTypeLen 20 19 -5.00% BenchmarkServerHandlerNoLen 18 17 -5.56% BenchmarkServerHandlerNoType 18 17 -5.56% BenchmarkServerHandlerNoHeader 13 12 -7.69% benchmark old bytes new bytes delta BenchmarkServerHandlerTypeLen 1913 1878 -1.83% BenchmarkServerHandlerNoLen 1878 1843 -1.86% BenchmarkServerHandlerNoType 1878 1844 -1.81% BenchmarkServerHandlerNoHeader 1085 1051 -3.13% Fixes #5188 R=golang-dev, adg, r CC=golang-dev https://golang.org/cl/8297044 --- src/pkg/net/http/transfer.go | 47 +++++++++++++++++------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/pkg/net/http/transfer.go b/src/pkg/net/http/transfer.go index 43c6023a3a..53569bcc2f 100644 --- a/src/pkg/net/http/transfer.go +++ b/src/pkg/net/http/transfer.go @@ -328,12 +328,13 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { switch { case chunked(t.TransferEncoding): if noBodyExpected(t.RequestMethod) { - t.Body = &body{Reader: io.LimitReader(r, 0), closing: t.Close} + t.Body = &body{Reader: eofReader, closing: t.Close} } else { t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing: t.Close} } - case realLength >= 0: - // TODO: limit the Content-Length. This is an easy DoS vector. + case realLength == 0: + t.Body = &body{Reader: eofReader, closing: t.Close} + case realLength > 0: t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close} default: // realLength < 0, i.e. "Content-Length" not mentioned in header @@ -342,7 +343,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { t.Body = &body{Reader: r, closing: t.Close} } else { // Persistent connection (i.e. HTTP/1.1) - t.Body = &body{Reader: io.LimitReader(r, 0), closing: t.Close} + t.Body = &body{Reader: eofReader, closing: t.Close} } } @@ -612,30 +613,26 @@ func (b *body) Close() error { if b.closed { return nil } - defer func() { - b.closed = true - }() - if b.hdr == nil && b.closing { + var err error + switch { + case b.hdr == nil && b.closing: // no trailer and closing the connection next. // no point in reading to EOF. - return nil - } - - // In a server request, don't continue reading from the client - // if we've already hit the maximum body size set by the - // handler. If this is set, that also means the TCP connection - // is about to be closed, so getting to the next HTTP request - // in the stream is not necessary. - if b.res != nil && b.res.requestBodyLimitHit { - return nil - } - - // Fully consume the body, which will also lead to us reading - // the trailer headers after the body, if present. - if _, err := io.Copy(ioutil.Discard, b); err != nil { - return err + case b.res != nil && b.res.requestBodyLimitHit: + // In a server request, don't continue reading from the client + // if we've already hit the maximum body size set by the + // handler. If this is set, that also means the TCP connection + // is about to be closed, so getting to the next HTTP request + // in the stream is not necessary. + case b.Reader == eofReader: + // Nothing to read. No need to io.Copy from it. + default: + // Fully consume the body, which will also lead to us reading + // the trailer headers after the body, if present. + _, err = io.Copy(ioutil.Discard, b) } - return nil + b.closed = true + return err } // parseContentLength trims whitespace from s and returns -1 if no value -- 2.50.0