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/31737Reviewed-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

@@ -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