From 1329aa51b605cb54ba9aae3a82a0a87b881fb7b3 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Mon, 13 Nov 2017 09:09:58 -0800 Subject: Updating server dependancies. (#7816) --- vendor/golang.org/x/net/http2/server.go | 53 +++++++----- vendor/golang.org/x/net/http2/server_test.go | 9 +- vendor/golang.org/x/net/http2/transport.go | 87 ++++++++++--------- vendor/golang.org/x/net/http2/transport_test.go | 110 ++++++++++++++++++++++++ vendor/golang.org/x/net/http2/write.go | 7 +- 5 files changed, 198 insertions(+), 68 deletions(-) (limited to 'vendor/golang.org/x/net/http2') diff --git a/vendor/golang.org/x/net/http2/server.go b/vendor/golang.org/x/net/http2/server.go index eae143ddf..3e705a01c 100644 --- a/vendor/golang.org/x/net/http2/server.go +++ b/vendor/golang.org/x/net/http2/server.go @@ -220,12 +220,15 @@ func ConfigureServer(s *http.Server, conf *Server) error { } else if s.TLSConfig.CipherSuites != nil { // If they already provided a CipherSuite list, return // an error if it has a bad order or is missing - // ECDHE_RSA_WITH_AES_128_GCM_SHA256. - const requiredCipher = tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 + // ECDHE_RSA_WITH_AES_128_GCM_SHA256 or ECDHE_ECDSA_WITH_AES_128_GCM_SHA256. haveRequired := false sawBad := false for i, cs := range s.TLSConfig.CipherSuites { - if cs == requiredCipher { + switch cs { + case tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + // Alternative MTI cipher to not discourage ECDSA-only servers. + // See http://golang.org/cl/30721 for further information. + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256: haveRequired = true } if isBadCipher(cs) { @@ -235,7 +238,7 @@ func ConfigureServer(s *http.Server, conf *Server) error { } } if !haveRequired { - return fmt.Errorf("http2: TLSConfig.CipherSuites is missing HTTP/2-required TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256") + return fmt.Errorf("http2: TLSConfig.CipherSuites is missing an HTTP/2-required AES_128_GCM_SHA256 cipher.") } } @@ -853,8 +856,13 @@ func (sc *serverConn) serve() { } } - if sc.inGoAway && sc.curOpenStreams() == 0 && !sc.needToSendGoAway && !sc.writingFrame { - return + // Start the shutdown timer after sending a GOAWAY. When sending GOAWAY + // with no error code (graceful shutdown), don't start the timer until + // all open streams have been completed. + sentGoAway := sc.inGoAway && !sc.needToSendGoAway && !sc.writingFrame + gracefulShutdownComplete := sc.goAwayCode == ErrCodeNo && sc.curOpenStreams() == 0 + if sentGoAway && sc.shutdownTimer == nil && (sc.goAwayCode != ErrCodeNo || gracefulShutdownComplete) { + sc.shutDownIn(goAwayTimeout) } } } @@ -1218,30 +1226,31 @@ func (sc *serverConn) startGracefulShutdown() { sc.shutdownOnce.Do(func() { sc.sendServeMsg(gracefulShutdownMsg) }) } +// After sending GOAWAY, the connection will close after goAwayTimeout. +// If we close the connection immediately after sending GOAWAY, there may +// be unsent data in our kernel receive buffer, which will cause the kernel +// to send a TCP RST on close() instead of a FIN. This RST will abort the +// connection immediately, whether or not the client had received the GOAWAY. +// +// Ideally we should delay for at least 1 RTT + epsilon so the client has +// a chance to read the GOAWAY and stop sending messages. Measuring RTT +// is hard, so we approximate with 1 second. See golang.org/issue/18701. +// +// This is a var so it can be shorter in tests, where all requests uses the +// loopback interface making the expected RTT very small. +// +// TODO: configurable? +var goAwayTimeout = 1 * time.Second + func (sc *serverConn) startGracefulShutdownInternal() { - sc.goAwayIn(ErrCodeNo, 0) + sc.goAway(ErrCodeNo) } func (sc *serverConn) goAway(code ErrCode) { - sc.serveG.check() - var forceCloseIn time.Duration - if code != ErrCodeNo { - forceCloseIn = 250 * time.Millisecond - } else { - // TODO: configurable - forceCloseIn = 1 * time.Second - } - sc.goAwayIn(code, forceCloseIn) -} - -func (sc *serverConn) goAwayIn(code ErrCode, forceCloseIn time.Duration) { sc.serveG.check() if sc.inGoAway { return } - if forceCloseIn != 0 { - sc.shutDownIn(forceCloseIn) - } sc.inGoAway = true sc.needToSendGoAway = true sc.goAwayCode = code diff --git a/vendor/golang.org/x/net/http2/server_test.go b/vendor/golang.org/x/net/http2/server_test.go index b4e832894..91db6a2c5 100644 --- a/vendor/golang.org/x/net/http2/server_test.go +++ b/vendor/golang.org/x/net/http2/server_test.go @@ -68,6 +68,7 @@ type serverTester struct { func init() { testHookOnPanicMu = new(sync.Mutex) + goAwayTimeout = 25 * time.Millisecond } func resetHooks() { @@ -3188,12 +3189,18 @@ func TestConfigureServer(t *testing.T) { CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}, }, }, + { + name: "just the alternative required cipher suite", + tlsConfig: &tls.Config{ + CipherSuites: []uint16{tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, + }, + }, { name: "missing required cipher suite", tlsConfig: &tls.Config{ CipherSuites: []uint16{tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384}, }, - wantErr: "is missing HTTP/2-required TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + wantErr: "is missing an HTTP/2-required AES_128_GCM_SHA256 cipher.", }, { name: "required after bad", diff --git a/vendor/golang.org/x/net/http2/transport.go b/vendor/golang.org/x/net/http2/transport.go index adb77ffab..4392a09f8 100644 --- a/vendor/golang.org/x/net/http2/transport.go +++ b/vendor/golang.org/x/net/http2/transport.go @@ -274,6 +274,13 @@ func (cs *clientStream) checkResetOrDone() error { } } +func (cs *clientStream) getStartedWrite() bool { + cc := cs.cc + cc.mu.Lock() + defer cc.mu.Unlock() + return cs.startedWrite +} + func (cs *clientStream) abortRequestBodyWrite(err error) { if err == nil { panic("nil error") @@ -349,14 +356,9 @@ func (t *Transport) RoundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Res return nil, err } traceGotConn(req, cc) - res, err := cc.RoundTrip(req) + res, gotErrAfterReqBodyWrite, err := cc.roundTrip(req) if err != nil && retry <= 6 { - afterBodyWrite := false - if e, ok := err.(afterReqBodyWriteError); ok { - err = e - afterBodyWrite = true - } - if req, err = shouldRetryRequest(req, err, afterBodyWrite); err == nil { + if req, err = shouldRetryRequest(req, err, gotErrAfterReqBodyWrite); err == nil { // After the first retry, do exponential backoff with 10% jitter. if retry == 0 { continue @@ -394,16 +396,6 @@ var ( errClientConnGotGoAway = errors.New("http2: Transport received Server's graceful shutdown GOAWAY") ) -// afterReqBodyWriteError is a wrapper around errors returned by ClientConn.RoundTrip. -// It is used to signal that err happened after part of Request.Body was sent to the server. -type afterReqBodyWriteError struct { - err error -} - -func (e afterReqBodyWriteError) Error() string { - return e.err.Error() + "; some request body already written" -} - // shouldRetryRequest is called by RoundTrip when a request fails to get // response headers. It is always called with a non-nil error. // It returns either a request to retry (either the same request, or a @@ -752,8 +744,13 @@ func actualContentLength(req *http.Request) int64 { } func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { + resp, _, err := cc.roundTrip(req) + return resp, err +} + +func (cc *ClientConn) roundTrip(req *http.Request) (res *http.Response, gotErrAfterReqBodyWrite bool, err error) { if err := checkConnHeaders(req); err != nil { - return nil, err + return nil, false, err } if cc.idleTimer != nil { cc.idleTimer.Stop() @@ -761,14 +758,14 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { trailers, err := commaSeparatedTrailers(req) if err != nil { - return nil, err + return nil, false, err } hasTrailers := trailers != "" cc.mu.Lock() if err := cc.awaitOpenSlotForRequest(req); err != nil { cc.mu.Unlock() - return nil, err + return nil, false, err } body := req.Body @@ -802,7 +799,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { hdrs, err := cc.encodeHeaders(req, requestedGzip, trailers, contentLen) if err != nil { cc.mu.Unlock() - return nil, err + return nil, false, err } cs := cc.newStream() @@ -828,7 +825,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { // Don't bother sending a RST_STREAM (our write already failed; // no need to keep writing) traceWroteRequest(cs.trace, werr) - return nil, werr + return nil, false, werr } var respHeaderTimer <-chan time.Time @@ -847,7 +844,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { bodyWritten := false ctx := reqContext(req) - handleReadLoopResponse := func(re resAndError) (*http.Response, error) { + handleReadLoopResponse := func(re resAndError) (*http.Response, bool, error) { res := re.res if re.err != nil || res.StatusCode > 299 { // On error or status code 3xx, 4xx, 5xx, etc abort any @@ -863,18 +860,12 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cs.abortRequestBodyWrite(errStopReqBodyWrite) } if re.err != nil { - cc.mu.Lock() - afterBodyWrite := cs.startedWrite - cc.mu.Unlock() cc.forgetStreamID(cs.ID) - if afterBodyWrite { - return nil, afterReqBodyWriteError{re.err} - } - return nil, re.err + return nil, cs.getStartedWrite(), re.err } res.Request = req res.TLS = cc.tlsState - return res, nil + return res, false, nil } for { @@ -889,7 +880,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel) } cc.forgetStreamID(cs.ID) - return nil, errTimeout + return nil, cs.getStartedWrite(), errTimeout case <-ctx.Done(): if !hasBody || bodyWritten { cc.writeStreamReset(cs.ID, ErrCodeCancel, nil) @@ -898,7 +889,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel) } cc.forgetStreamID(cs.ID) - return nil, ctx.Err() + return nil, cs.getStartedWrite(), ctx.Err() case <-req.Cancel: if !hasBody || bodyWritten { cc.writeStreamReset(cs.ID, ErrCodeCancel, nil) @@ -907,12 +898,12 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cs.abortRequestBodyWrite(errStopReqBodyWriteAndCancel) } cc.forgetStreamID(cs.ID) - return nil, errRequestCanceled + return nil, cs.getStartedWrite(), errRequestCanceled case <-cs.peerReset: // processResetStream already removed the // stream from the streams map; no need for // forgetStreamID. - return nil, cs.resetErr + return nil, cs.getStartedWrite(), cs.resetErr case err := <-bodyWriter.resc: // Prefer the read loop's response, if available. Issue 16102. select { @@ -921,7 +912,7 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { default: } if err != nil { - return nil, err + return nil, cs.getStartedWrite(), err } bodyWritten = true if d := cc.responseHeaderTimeout(); d != 0 { @@ -1536,7 +1527,17 @@ func (rl *clientConnReadLoop) run() error { func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error { cc := rl.cc - cs := cc.streamByID(f.StreamID, f.StreamEnded()) + if f.StreamEnded() { + // Issue 20521: If the stream has ended, streamByID() causes + // clientStream.done to be closed, which causes the request's bodyWriter + // to be closed with an errStreamClosed, which may be received by + // clientConn.RoundTrip before the result of processing these headers. + // Deferring stream closure allows the header processing to occur first. + // clientConn.RoundTrip may still receive the bodyWriter error first, but + // the fix for issue 16102 prioritises any response. + defer cc.streamByID(f.StreamID, true) + } + cs := cc.streamByID(f.StreamID, false) if cs == nil { // We'd get here if we canceled a request while the // server had its response still in flight. So if this @@ -1841,6 +1842,14 @@ func (rl *clientConnReadLoop) processData(f *DataFrame) error { return nil } if f.Length > 0 { + if cs.req.Method == "HEAD" && len(data) > 0 { + cc.logf("protocol error: received DATA on a HEAD request") + rl.endStreamError(cs, StreamError{ + StreamID: f.StreamID, + Code: ErrCodeProtocol, + }) + return nil + } // Check connection-level flow control. cc.mu.Lock() if cs.inflow.available() >= int32(f.Length) { @@ -1902,11 +1911,11 @@ func (rl *clientConnReadLoop) endStreamError(cs *clientStream, err error) { err = io.EOF code = cs.copyTrailers } - cs.bufPipe.closeWithErrorAndCode(err, code) - delete(rl.activeRes, cs.ID) if isConnectionCloseRequest(cs.req) { rl.closeWhenIdle = true } + cs.bufPipe.closeWithErrorAndCode(err, code) + delete(rl.activeRes, cs.ID) select { case cs.resc <- resAndError{err: err}: diff --git a/vendor/golang.org/x/net/http2/transport_test.go b/vendor/golang.org/x/net/http2/transport_test.go index 0126ff483..30d7b5de0 100644 --- a/vendor/golang.org/x/net/http2/transport_test.go +++ b/vendor/golang.org/x/net/http2/transport_test.go @@ -2290,6 +2290,60 @@ func TestTransportReadHeadResponse(t *testing.T) { ct.run() } +func TestTransportReadHeadResponseWithBody(t *testing.T) { + response := "redirecting to /elsewhere" + ct := newClientTester(t) + clientDone := make(chan struct{}) + ct.client = func() error { + defer close(clientDone) + req, _ := http.NewRequest("HEAD", "https://dummy.tld/", nil) + res, err := ct.tr.RoundTrip(req) + if err != nil { + return err + } + if res.ContentLength != int64(len(response)) { + return fmt.Errorf("Content-Length = %d; want %d", res.ContentLength, len(response)) + } + slurp, err := ioutil.ReadAll(res.Body) + if err != nil { + return fmt.Errorf("ReadAll: %v", err) + } + if len(slurp) > 0 { + return fmt.Errorf("Unexpected non-empty ReadAll body: %q", slurp) + } + return nil + } + ct.server = func() error { + ct.greet() + for { + f, err := ct.fr.ReadFrame() + if err != nil { + t.Logf("ReadFrame: %v", err) + return nil + } + hf, ok := f.(*HeadersFrame) + if !ok { + continue + } + var buf bytes.Buffer + enc := hpack.NewEncoder(&buf) + enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"}) + enc.WriteField(hpack.HeaderField{Name: "content-length", Value: strconv.Itoa(len(response))}) + ct.fr.WriteHeaders(HeadersFrameParam{ + StreamID: hf.StreamID, + EndHeaders: true, + EndStream: false, + BlockFragment: buf.Bytes(), + }) + ct.fr.WriteData(hf.StreamID, true, []byte(response)) + + <-clientDone + return nil + } + } + ct.run() +} + type neverEnding byte func (b neverEnding) Read(p []byte) (int, error) { @@ -3043,6 +3097,34 @@ func TestTransportCancelDataResponseRace(t *testing.T) { } } +// Issue 21316: It should be safe to reuse an http.Request after the +// request has completed. +func TestTransportNoRaceOnRequestObjectAfterRequestComplete(t *testing.T) { + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + io.WriteString(w, "body") + }, optOnlyServer) + defer st.Close() + + tr := &Transport{TLSClientConfig: tlsConfigInsecure} + defer tr.CloseIdleConnections() + + req, _ := http.NewRequest("GET", st.ts.URL, nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + if _, err = io.Copy(ioutil.Discard, resp.Body); err != nil { + t.Fatalf("error reading response body: %v", err) + } + if err := resp.Body.Close(); err != nil { + t.Fatalf("error closing response body: %v", err) + } + + // This access of req.Header should not race with code in the transport. + req.Header = http.Header{} +} + func TestTransportRetryAfterGOAWAY(t *testing.T) { var dialer struct { sync.Mutex @@ -3678,6 +3760,34 @@ func benchSimpleRoundTrip(b *testing.B, nHeaders int) { } } +type infiniteReader struct{} + +func (r infiniteReader) Read(b []byte) (int, error) { + return len(b), nil +} + +// Issue 20521: it is not an error to receive a response and end stream +// from the server without the body being consumed. +func TestTransportResponseAndResetWithoutConsumingBodyRace(t *testing.T) { + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }, optOnlyServer) + defer st.Close() + + tr := &Transport{TLSClientConfig: tlsConfigInsecure} + defer tr.CloseIdleConnections() + + // The request body needs to be big enough to trigger flow control. + req, _ := http.NewRequest("PUT", st.ts.URL, infiniteReader{}) + res, err := tr.RoundTrip(req) + if err != nil { + t.Fatal(err) + } + if res.StatusCode != http.StatusOK { + t.Fatalf("Response code = %v; want %v", res.StatusCode, http.StatusOK) + } +} + func BenchmarkClientRequestHeaders(b *testing.B) { b.Run(" 0 Headers", func(b *testing.B) { benchSimpleRoundTrip(b, 0) }) b.Run(" 10 Headers", func(b *testing.B) { benchSimpleRoundTrip(b, 10) }) diff --git a/vendor/golang.org/x/net/http2/write.go b/vendor/golang.org/x/net/http2/write.go index 6b0dfae31..54ab4a88e 100644 --- a/vendor/golang.org/x/net/http2/write.go +++ b/vendor/golang.org/x/net/http2/write.go @@ -10,7 +10,6 @@ import ( "log" "net/http" "net/url" - "time" "golang.org/x/net/http2/hpack" "golang.org/x/net/lex/httplex" @@ -90,11 +89,7 @@ type writeGoAway struct { func (p *writeGoAway) writeFrame(ctx writeContext) error { err := ctx.Framer().WriteGoAway(p.maxStreamID, p.code, nil) - if p.code != 0 { - ctx.Flush() // ignore error: we're hanging up on them anyway - time.Sleep(50 * time.Millisecond) - ctx.CloseConn() - } + ctx.Flush() // ignore error: we're hanging up on them anyway return err } -- cgit v1.2.3-1-g7c22