]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: update bundled http2
authorBrad Fitzpatrick <bradfitz@golang.org>
Fri, 21 Oct 2016 20:55:25 +0000 (13:55 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Fri, 21 Oct 2016 21:19:16 +0000 (21:19 +0000)
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 <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/http/h2_bundle.go

index 814619d3a2f5f404deb7708950f4182920b0fe88..9d6d3caef69423af5c7ed7450adcd3ea07f7ddbc 100644 (file)
@@ -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