From a50fbcd331c57b885c13a6c0c2502202417ce312 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 21 Oct 2016 13:55:25 -0700 Subject: [PATCH] net/http: update bundled http2 Updates http2 to x/net/http2 git rev 40a0a18 for: http2: fix Server race with concurrent Read/Close http2: make Server reuse 64k request body buffer between requests http2: never Read from Request.Body in Transport to determine ContentLength Fixes #17480 Updates #17071 Change-Id: If142925764a2e148f95957f559637cfc1785ad21 Reviewed-on: https://go-review.googlesource.com/31737 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/net/http/h2_bundle.go | 123 +++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 61 deletions(-) diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 814619d3a2..9d6d3caef6 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -2384,6 +2384,7 @@ var ( http2VerboseLogs bool http2logFrameWrites bool http2logFrameReads bool + http2inTests bool ) func init() { @@ -3153,7 +3154,6 @@ type http2serverConn struct { goAwayCode http2ErrCode shutdownTimerCh <-chan time.Time // nil until used shutdownTimer *time.Timer // nil until used - freeRequestBodyBuf []byte // if non-nil, a free initialWindowSize buffer for getRequestBodyBuf // Owned by the writeFrameAsync goroutine: headerWriteBuf bytes.Buffer @@ -3197,11 +3197,11 @@ type http2stream struct { numTrailerValues int64 weight uint8 state http2streamState - sentReset bool // only true once detached from streams map - gotReset bool // only true once detacted from streams map - gotTrailerHeader bool // HEADER frame for trailers was seen - wroteHeaders bool // whether we wrote headers (not status 100) - reqBuf []byte + sentReset bool // only true once detached from streams map + gotReset bool // only true once detacted from streams map + gotTrailerHeader bool // HEADER frame for trailers was seen + wroteHeaders bool // whether we wrote headers (not status 100) + reqBuf []byte // if non-nil, body pipe buffer to return later at EOF trailer Header // accumulated trailers reqTrailer Header // handler's Request.Trailer @@ -3890,10 +3890,6 @@ func (sc *http2serverConn) closeStream(st *http2stream, err error) { } st.cw.Close() sc.writeSched.forgetStream(st.id) - if st.reqBuf != nil { - - sc.freeRequestBodyBuf = st.reqBuf - } } func (sc *http2serverConn) processSettings(f *http2SettingsFrame) error { @@ -4287,11 +4283,9 @@ func (sc *http2serverConn) newWriterAndRequest(st *http2stream, f *http2MetaHead } req = http2requestWithContext(req, st.ctx) if bodyOpen { - - buf := make([]byte, http2initialWindowSize) - + st.reqBuf = http2getRequestBodyBuf() body.pipe = &http2pipe{ - b: &http2fixedBuffer{buf: buf}, + b: &http2fixedBuffer{buf: st.reqBuf}, } if vv, ok := header["Content-Length"]; ok { @@ -4315,13 +4309,22 @@ func (sc *http2serverConn) newWriterAndRequest(st *http2stream, f *http2MetaHead return rw, req, nil } -func (sc *http2serverConn) getRequestBodyBuf() []byte { - sc.serveG.check() - if buf := sc.freeRequestBodyBuf; buf != nil { - sc.freeRequestBodyBuf = nil - return buf +var http2reqBodyCache = make(chan []byte, 8) + +func http2getRequestBodyBuf() []byte { + select { + case b := <-http2reqBodyCache: + return b + default: + return make([]byte, http2initialWindowSize) + } +} + +func http2putRequestBodyBuf(b []byte) { + select { + case http2reqBodyCache <- b: + default: } - return make([]byte, http2initialWindowSize) } // Run on its own goroutine. @@ -4406,11 +4409,19 @@ type http2bodyReadMsg struct { // called from handler goroutines. // Notes that the handler for the given stream ID read n bytes of its body // and schedules flow control tokens to be sent. -func (sc *http2serverConn) noteBodyReadFromHandler(st *http2stream, n int) { +func (sc *http2serverConn) noteBodyReadFromHandler(st *http2stream, n int, err error) { sc.serveG.checkNotOn() - select { - case sc.bodyReadCh <- http2bodyReadMsg{st, n}: - case <-sc.doneServing: + if n > 0 { + select { + case sc.bodyReadCh <- http2bodyReadMsg{st, n}: + case <-sc.doneServing: + } + } + if err == io.EOF { + if buf := st.reqBuf; buf != nil { + st.reqBuf = nil + http2putRequestBodyBuf(buf) + } } } @@ -4467,16 +4478,19 @@ func (sc *http2serverConn) sendWindowUpdate32(st *http2stream, n int32) { } } +// requestBody is the Handler's Request.Body type. +// Read and Close may be called concurrently. type http2requestBody struct { stream *http2stream conn *http2serverConn - closed bool + closed bool // for use by Close only + sawEOF bool // for use by Read only pipe *http2pipe // non-nil if we have a HTTP entity message body needsContinue bool // need to send a 100-continue } func (b *http2requestBody) Close() error { - if b.pipe != nil { + if b.pipe != nil && !b.closed { b.pipe.BreakWithError(http2errClosedBody) } b.closed = true @@ -4488,13 +4502,17 @@ func (b *http2requestBody) Read(p []byte) (n int, err error) { b.needsContinue = false b.conn.write100ContinueHeaders(b.stream) } - if b.pipe == nil { + if b.pipe == nil || b.sawEOF { return 0, io.EOF } n, err = b.pipe.Read(p) - if n > 0 { - b.conn.noteBodyReadFromHandler(b.stream, n) + if err == io.EOF { + b.sawEOF = true + } + if b.conn == nil && http2inTests { + return } + b.conn.noteBodyReadFromHandler(b.stream, n, err) return } @@ -5493,46 +5511,28 @@ func (cc *http2ClientConn) responseHeaderTimeout() time.Duration { // Certain headers are special-cased as okay but not transmitted later. func http2checkConnHeaders(req *Request) error { if v := req.Header.Get("Upgrade"); v != "" { - return errors.New("http2: invalid Upgrade request header") + return fmt.Errorf("http2: invalid Upgrade request header: %q", req.Header["Upgrade"]) } - if v := req.Header.Get("Transfer-Encoding"); (v != "" && v != "chunked") || len(req.Header["Transfer-Encoding"]) > 1 { - return errors.New("http2: invalid Transfer-Encoding request header") + if vv := req.Header["Transfer-Encoding"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && vv[0] != "chunked") { + return fmt.Errorf("http2: invalid Transfer-Encoding request header: %q", vv) } - if v := req.Header.Get("Connection"); (v != "" && v != "close" && v != "keep-alive") || len(req.Header["Connection"]) > 1 { - return errors.New("http2: invalid Connection request header") + if vv := req.Header["Connection"]; len(vv) > 0 && (len(vv) > 1 || vv[0] != "" && vv[0] != "close" && vv[0] != "keep-alive") { + return fmt.Errorf("http2: invalid Connection request header: %q", vv) } return nil } -func http2bodyAndLength(req *Request) (body io.Reader, contentLen int64) { - body = req.Body - if body == nil { - return nil, 0 +// actualContentLength returns a sanitized version of +// req.ContentLength, where 0 actually means zero (not unknown) and -1 +// means unknown. +func http2actualContentLength(req *Request) int64 { + if req.Body == nil { + return 0 } if req.ContentLength != 0 { - return req.Body, req.ContentLength - } - - if req.Header.Get("Expect") == "100-continue" { - return req.Body, -1 - } - - // We have a body but a zero content length. Test to see if - // it's actually zero or just unset. - var buf [1]byte - n, rerr := body.Read(buf[:]) - if rerr != nil && rerr != io.EOF { - return http2errorReader{rerr}, -1 + return req.ContentLength } - if n == 1 { - - if rerr == io.EOF { - return bytes.NewReader(buf[:]), 1 - } - return io.MultiReader(bytes.NewReader(buf[:]), body), -1 - } - - return nil, 0 + return -1 } func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { @@ -5556,8 +5556,9 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { return nil, http2errClientConnUnusable } - body, contentLen := http2bodyAndLength(req) + body := req.Body hasBody := body != nil + contentLen := http2actualContentLength(req) // TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere? var requestedGzip bool -- 2.48.1