From 701d1ab638b23c24877fc41824add66232446676 Mon Sep 17 00:00:00 2001 From: Christopher Speller Date: Thu, 2 Feb 2017 09:32:00 -0500 Subject: Updating server dependancies (#5249) --- vendor/golang.org/x/net/http2/frame.go | 29 ++-- vendor/golang.org/x/net/http2/go18.go | 9 ++ vendor/golang.org/x/net/http2/h2i/h2i.go | 12 +- vendor/golang.org/x/net/http2/not_go18.go | 11 +- vendor/golang.org/x/net/http2/server.go | 131 ++++++++++----- vendor/golang.org/x/net/http2/server_push_test.go | 59 ++++++- vendor/golang.org/x/net/http2/server_test.go | 57 +++++-- vendor/golang.org/x/net/http2/transport.go | 69 +++++++- vendor/golang.org/x/net/http2/transport_test.go | 177 ++++++++++++++++++++- vendor/golang.org/x/net/http2/write.go | 9 +- vendor/golang.org/x/net/http2/writesched.go | 14 ++ .../x/net/http2/writesched_priority_test.go | 2 +- .../x/net/http2/writesched_random_test.go | 2 +- 13 files changed, 492 insertions(+), 89 deletions(-) (limited to 'vendor/golang.org/x/net/http2') diff --git a/vendor/golang.org/x/net/http2/frame.go b/vendor/golang.org/x/net/http2/frame.go index b0c79b01a..358833fed 100644 --- a/vendor/golang.org/x/net/http2/frame.go +++ b/vendor/golang.org/x/net/http2/frame.go @@ -317,10 +317,12 @@ type Framer struct { // non-Continuation or Continuation on a different stream is // attempted to be written. - logReads bool + logReads, logWrites bool - debugFramer *Framer // only use for logging written writes - debugFramerBuf *bytes.Buffer + debugFramer *Framer // only use for logging written writes + debugFramerBuf *bytes.Buffer + debugReadLoggerf func(string, ...interface{}) + debugWriteLoggerf func(string, ...interface{}) } func (fr *Framer) maxHeaderListSize() uint32 { @@ -355,7 +357,7 @@ func (f *Framer) endWrite() error { byte(length>>16), byte(length>>8), byte(length)) - if logFrameWrites { + if f.logWrites { f.logWrite() } @@ -378,10 +380,10 @@ func (f *Framer) logWrite() { f.debugFramerBuf.Write(f.wbuf) fr, err := f.debugFramer.ReadFrame() if err != nil { - log.Printf("http2: Framer %p: failed to decode just-written frame", f) + f.debugWriteLoggerf("http2: Framer %p: failed to decode just-written frame", f) return } - log.Printf("http2: Framer %p: wrote %v", f, summarizeFrame(fr)) + f.debugWriteLoggerf("http2: Framer %p: wrote %v", f, summarizeFrame(fr)) } func (f *Framer) writeByte(v byte) { f.wbuf = append(f.wbuf, v) } @@ -399,9 +401,12 @@ const ( // NewFramer returns a Framer that writes frames to w and reads them from r. func NewFramer(w io.Writer, r io.Reader) *Framer { fr := &Framer{ - w: w, - r: r, - logReads: logFrameReads, + w: w, + r: r, + logReads: logFrameReads, + logWrites: logFrameWrites, + debugReadLoggerf: log.Printf, + debugWriteLoggerf: log.Printf, } fr.getReadBuf = func(size uint32) []byte { if cap(fr.readBuf) >= int(size) { @@ -483,7 +488,7 @@ func (fr *Framer) ReadFrame() (Frame, error) { return nil, err } if fr.logReads { - log.Printf("http2: Framer %p: read %v", fr, summarizeFrame(f)) + fr.debugReadLoggerf("http2: Framer %p: read %v", fr, summarizeFrame(f)) } if fh.Type == FrameHeaders && fr.ReadMetaHeaders != nil { return fr.readMetaFrame(f.(*HeadersFrame)) @@ -1419,8 +1424,8 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (*MetaHeadersFrame, error) { hdec.SetEmitEnabled(true) hdec.SetMaxStringLength(fr.maxHeaderStringLen()) hdec.SetEmitFunc(func(hf hpack.HeaderField) { - if VerboseLogs && logFrameReads { - log.Printf("http2: decoded hpack field %+v", hf) + if VerboseLogs && fr.logReads { + fr.debugReadLoggerf("http2: decoded hpack field %+v", hf) } if !httplex.ValidHeaderFieldValue(hf.Value) { invalid = headerFieldValueError(hf.Value) diff --git a/vendor/golang.org/x/net/http2/go18.go b/vendor/golang.org/x/net/http2/go18.go index 8c0dd2508..633202c39 100644 --- a/vendor/golang.org/x/net/http2/go18.go +++ b/vendor/golang.org/x/net/http2/go18.go @@ -8,6 +8,7 @@ package http2 import ( "crypto/tls" + "io" "net/http" ) @@ -39,3 +40,11 @@ func configureServer18(h1 *http.Server, h2 *Server) error { func shouldLogPanic(panicValue interface{}) bool { return panicValue != nil && panicValue != http.ErrAbortHandler } + +func reqGetBody(req *http.Request) func() (io.ReadCloser, error) { + return req.GetBody +} + +func reqBodyIsNoBody(body io.ReadCloser) bool { + return body == http.NoBody +} diff --git a/vendor/golang.org/x/net/http2/h2i/h2i.go b/vendor/golang.org/x/net/http2/h2i/h2i.go index 228edf8a4..76c778711 100644 --- a/vendor/golang.org/x/net/http2/h2i/h2i.go +++ b/vendor/golang.org/x/net/http2/h2i/h2i.go @@ -88,6 +88,14 @@ func withPort(host string) string { return host } +// withoutPort strips the port from addr if present. +func withoutPort(addr string) string { + if h, _, err := net.SplitHostPort(addr); err == nil { + return h + } + return addr +} + // h2i is the app's state. type h2i struct { host string @@ -134,7 +142,7 @@ func main() { func (app *h2i) Main() error { cfg := &tls.Config{ - ServerName: app.host, + ServerName: withoutPort(app.host), NextProtos: strings.Split(*flagNextProto, ","), InsecureSkipVerify: *flagInsecure, } @@ -473,7 +481,7 @@ func (app *h2i) encodeHeaders(req *http.Request) []byte { host = req.URL.Host } - path := req.URL.Path + path := req.RequestURI if path == "" { path = "/" } diff --git a/vendor/golang.org/x/net/http2/not_go18.go b/vendor/golang.org/x/net/http2/not_go18.go index 2e600dc35..efbf83c32 100644 --- a/vendor/golang.org/x/net/http2/not_go18.go +++ b/vendor/golang.org/x/net/http2/not_go18.go @@ -6,7 +6,10 @@ package http2 -import "net/http" +import ( + "io" + "net/http" +) func configureServer18(h1 *http.Server, h2 *Server) error { // No IdleTimeout to sync prior to Go 1.8. @@ -16,3 +19,9 @@ func configureServer18(h1 *http.Server, h2 *Server) error { func shouldLogPanic(panicValue interface{}) bool { return panicValue != nil } + +func reqGetBody(req *http.Request) func() (io.ReadCloser, error) { + return nil +} + +func reqBodyIsNoBody(io.ReadCloser) bool { return false } diff --git a/vendor/golang.org/x/net/http2/server.go b/vendor/golang.org/x/net/http2/server.go index 0b6b4b08d..3c6b90ccd 100644 --- a/vendor/golang.org/x/net/http2/server.go +++ b/vendor/golang.org/x/net/http2/server.go @@ -278,6 +278,16 @@ func (s *Server) ServeConn(c net.Conn, opts *ServeConnOpts) { pushEnabled: true, } + // The net/http package sets the write deadline from the + // http.Server.WriteTimeout during the TLS handshake, but then + // passes the connection off to us with the deadline already + // set. Disarm it here so that it is not applied to additional + // streams opened on this connection. + // TODO: implement WriteTimeout fully. See Issue 18437. + if sc.hs.WriteTimeout != 0 { + sc.conn.SetWriteDeadline(time.Time{}) + } + if s.NewWriteScheduler != nil { sc.writeSched = s.NewWriteScheduler() } else { @@ -423,6 +433,11 @@ func (sc *serverConn) maxHeaderListSize() uint32 { return uint32(n + typicalHeaders*perFieldOverhead) } +func (sc *serverConn) curOpenStreams() uint32 { + sc.serveG.check() + return sc.curClientStreams + sc.curPushedStreams +} + // stream represents a stream. This is the minimal metadata needed by // the serve goroutine. Most of the actual stream state is owned by // the http.Handler's goroutine in the responseWriter. Because the @@ -448,8 +463,7 @@ type stream struct { numTrailerValues int64 weight uint8 state streamState - sentReset bool // only true once detached from streams map - gotReset bool // only true once detacted from streams map + resetQueued bool // RST_STREAM queued for write; set by sc.resetStream 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 @@ -753,7 +767,7 @@ func (sc *serverConn) serve() { fn(loopNum) } - if sc.inGoAway && sc.curClientStreams == 0 && !sc.needToSendGoAway && !sc.writingFrame { + if sc.inGoAway && sc.curOpenStreams() == 0 && !sc.needToSendGoAway && !sc.writingFrame { return } } @@ -869,8 +883,34 @@ func (sc *serverConn) writeFrameFromHandler(wr FrameWriteRequest) error { func (sc *serverConn) writeFrame(wr FrameWriteRequest) { sc.serveG.check() + // If true, wr will not be written and wr.done will not be signaled. var ignoreWrite bool + // We are not allowed to write frames on closed streams. RFC 7540 Section + // 5.1.1 says: "An endpoint MUST NOT send frames other than PRIORITY on + // a closed stream." Our server never sends PRIORITY, so that exception + // does not apply. + // + // The serverConn might close an open stream while the stream's handler + // is still running. For example, the server might close a stream when it + // receives bad data from the client. If this happens, the handler might + // attempt to write a frame after the stream has been closed (since the + // handler hasn't yet been notified of the close). In this case, we simply + // ignore the frame. The handler will notice that the stream is closed when + // it waits for the frame to be written. + // + // As an exception to this rule, we allow sending RST_STREAM after close. + // This allows us to immediately reject new streams without tracking any + // state for those streams (except for the queued RST_STREAM frame). This + // may result in duplicate RST_STREAMs in some cases, but the client should + // ignore those. + if wr.StreamID() != 0 { + _, isReset := wr.write.(StreamError) + if state, _ := sc.state(wr.StreamID()); state == stateClosed && !isReset { + ignoreWrite = true + } + } + // Don't send a 100-continue response if we've already sent headers. // See golang.org/issue/14030. switch wr.write.(type) { @@ -878,6 +918,11 @@ func (sc *serverConn) writeFrame(wr FrameWriteRequest) { wr.stream.wroteHeaders = true case write100ContinueHeadersFrame: if wr.stream.wroteHeaders { + // We do not need to notify wr.done because this frame is + // never written with wr.done != nil. + if wr.done != nil { + panic("wr.done != nil for write100ContinueHeadersFrame") + } ignoreWrite = true } } @@ -901,14 +946,15 @@ func (sc *serverConn) startFrameWrite(wr FrameWriteRequest) { if st != nil { switch st.state { case stateHalfClosedLocal: - panic("internal error: attempt to send frame on half-closed-local stream") - case stateClosed: - if st.sentReset || st.gotReset { - // Skip this frame. - sc.scheduleFrameWrite() - return + switch wr.write.(type) { + case StreamError, handlerPanicRST, writeWindowUpdate: + // RFC 7540 Section 5.1 allows sending RST_STREAM, PRIORITY, and WINDOW_UPDATE + // in this state. (We never send PRIORITY from the server, so that is not checked.) + default: + panic(fmt.Sprintf("internal error: attempt to send frame on a half-closed-local stream: %v", wr)) } - panic(fmt.Sprintf("internal error: attempt to send a write %v on a closed stream", wr)) + case stateClosed: + panic(fmt.Sprintf("internal error: attempt to send frame on a closed stream: %v", wr)) } } if wpp, ok := wr.write.(*writePushPromise); ok { @@ -916,9 +962,7 @@ func (sc *serverConn) startFrameWrite(wr FrameWriteRequest) { wpp.promisedID, err = wpp.allocatePromisedID() if err != nil { sc.writingFrameAsync = false - if wr.done != nil { - wr.done <- err - } + wr.replyToWriter(err) return } } @@ -951,25 +995,9 @@ func (sc *serverConn) wroteFrame(res frameWriteResult) { sc.writingFrameAsync = false wr := res.wr - st := wr.stream - - closeStream := endsStream(wr.write) - - if _, ok := wr.write.(handlerPanicRST); ok { - sc.closeStream(st, errHandlerPanicked) - } - - // Reply (if requested) to the blocked ServeHTTP goroutine. - if ch := wr.done; ch != nil { - select { - case ch <- res.err: - default: - panic(fmt.Sprintf("unbuffered done channel passed in for type %T", wr.write)) - } - } - wr.write = nil // prevent use (assume it's tainted after wr.done send) - if closeStream { + if writeEndsStream(wr.write) { + st := wr.stream if st == nil { panic("internal error: expecting non-nil stream") } @@ -982,15 +1010,29 @@ func (sc *serverConn) wroteFrame(res frameWriteResult) { // reading data (see possible TODO at top of // this file), we go into closed state here // anyway, after telling the peer we're - // hanging up on them. - st.state = stateHalfClosedLocal // won't last long, but necessary for closeStream via resetStream - errCancel := streamError(st.id, ErrCodeCancel) - sc.resetStream(errCancel) + // hanging up on them. We'll transition to + // stateClosed after the RST_STREAM frame is + // written. + st.state = stateHalfClosedLocal + sc.resetStream(streamError(st.id, ErrCodeCancel)) case stateHalfClosedRemote: sc.closeStream(st, errHandlerComplete) } + } else { + switch v := wr.write.(type) { + case StreamError: + // st may be unknown if the RST_STREAM was generated to reject bad input. + if st, ok := sc.streams[v.StreamID]; ok { + sc.closeStream(st, v) + } + case handlerPanicRST: + sc.closeStream(wr.stream, errHandlerPanicked) + } } + // Reply (if requested) to unblock the ServeHTTP goroutine. + wr.replyToWriter(res.err) + sc.scheduleFrameWrite() } @@ -1087,8 +1129,7 @@ func (sc *serverConn) resetStream(se StreamError) { sc.serveG.check() sc.writeFrame(FrameWriteRequest{write: se}) if st, ok := sc.streams[se.StreamID]; ok { - st.sentReset = true - sc.closeStream(st, se) + st.resetQueued = true } } @@ -1252,7 +1293,6 @@ func (sc *serverConn) processResetStream(f *RSTStreamFrame) error { return ConnectionError(ErrCodeProtocol) } if st != nil { - st.gotReset = true st.cancelCtx() sc.closeStream(st, streamError(f.StreamID, f.ErrCode)) } @@ -1391,7 +1431,7 @@ func (sc *serverConn) processData(f *DataFrame) error { // type PROTOCOL_ERROR." return ConnectionError(ErrCodeProtocol) } - if st == nil || state != stateOpen || st.gotTrailerHeader { + if st == nil || state != stateOpen || st.gotTrailerHeader || st.resetQueued { // This includes sending a RST_STREAM if the stream is // in stateHalfClosedLocal (which currently means that // the http.Handler returned, so it's done reading & @@ -1411,6 +1451,10 @@ func (sc *serverConn) processData(f *DataFrame) error { sc.inflow.take(int32(f.Length)) sc.sendWindowUpdate(nil, int(f.Length)) // conn-level + if st != nil && st.resetQueued { + // Already have a stream error in flight. Don't send another. + return nil + } return streamError(id, ErrCodeStreamClosed) } if st.body == nil { @@ -1519,6 +1563,11 @@ func (sc *serverConn) processHeaders(f *MetaHeadersFrame) error { // open, let it process its own HEADERS frame (trailers at this // point, if it's valid). if st := sc.streams[f.StreamID]; st != nil { + if st.resetQueued { + // We're sending RST_STREAM to close the stream, so don't bother + // processing this frame. + return nil + } return st.processTrailerHeaders(f) } @@ -1681,7 +1730,7 @@ func (sc *serverConn) newStream(id, pusherID uint32, state streamState) *stream } else { sc.curClientStreams++ } - if sc.curClientStreams+sc.curPushedStreams == 1 { + if sc.curOpenStreams() == 1 { sc.setConnState(http.StateActive) } @@ -2556,7 +2605,7 @@ func (sc *serverConn) startPush(msg startPushRequest) { scheme: msg.url.Scheme, authority: msg.url.Host, path: msg.url.RequestURI(), - header: msg.header, + header: cloneHeader(msg.header), // clone since handler runs concurrently with writing the PUSH_PROMISE }) if err != nil { // Should not happen, since we've already validated msg.url. diff --git a/vendor/golang.org/x/net/http2/server_push_test.go b/vendor/golang.org/x/net/http2/server_push_test.go index 3fea20870..f70edd3c7 100644 --- a/vendor/golang.org/x/net/http2/server_push_test.go +++ b/vendor/golang.org/x/net/http2/server_push_test.go @@ -244,6 +244,50 @@ func TestServer_Push_Success(t *testing.T) { } } +func TestServer_Push_SuccessNoRace(t *testing.T) { + // Regression test for issue #18326. Ensure the request handler can mutate + // pushed request headers without racing with the PUSH_PROMISE write. + errc := make(chan error, 2) + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.RequestURI() { + case "/": + opt := &http.PushOptions{ + Header: http.Header{"User-Agent": {"testagent"}}, + } + if err := w.(http.Pusher).Push("/pushed", opt); err != nil { + errc <- fmt.Errorf("error pushing: %v", err) + return + } + w.WriteHeader(200) + errc <- nil + + case "/pushed": + // Update request header, ensure there is no race. + r.Header.Set("User-Agent", "newagent") + r.Header.Set("Cookie", "cookie") + w.WriteHeader(200) + errc <- nil + + default: + errc <- fmt.Errorf("unknown RequestURL %q", r.URL.RequestURI()) + } + }) + + // Send one request, which should push one response. + st.greet() + getSlash(st) + for k := 0; k < 2; k++ { + select { + case <-time.After(2 * time.Second): + t.Errorf("timeout waiting for handler %d to finish", k) + case err := <-errc: + if err != nil { + t.Fatal(err) + } + } + } +} + func TestServer_Push_RejectRecursivePush(t *testing.T) { // Expect two requests, but might get three if there's a bug and the second push succeeds. errc := make(chan error, 3) @@ -386,18 +430,20 @@ func TestServer_Push_RejectForbiddenHeader(t *testing.T) { func TestServer_Push_StateTransitions(t *testing.T) { const body = "foo" - startedPromise := make(chan bool) + gotPromise := make(chan bool) finishedPush := make(chan bool) + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.RequestURI() { case "/": if err := w.(http.Pusher).Push("/pushed", nil); err != nil { t.Errorf("Push error: %v", err) } - close(startedPromise) // Don't finish this request until the push finishes so we don't // nondeterministically interleave output frames with the push. <-finishedPush + case "/pushed": + <-gotPromise } w.Header().Set("Content-Type", "text/html") w.Header().Set("Content-Length", strconv.Itoa(len(body))) @@ -414,11 +460,16 @@ func TestServer_Push_StateTransitions(t *testing.T) { t.Fatalf("streamState(2)=%v, want %v", got, want) } getSlash(st) - <-startedPromise + // After the PUSH_PROMISE is sent, the stream should be stateHalfClosedRemote. + st.wantPushPromise() if got, want := st.streamState(2), stateHalfClosedRemote; got != want { t.Fatalf("streamState(2)=%v, want %v", got, want) } - st.wantPushPromise() + // We stall the HTTP handler for "/pushed" until the above check. If we don't + // stall the handler, then the handler might write HEADERS and DATA and finish + // the stream before we check st.streamState(2) -- should that happen, we'll + // see stateClosed and fail the above check. + close(gotPromise) st.wantHeaders() if df := st.wantData(); !df.StreamEnded() { t.Fatal("expected END_STREAM flag on DATA") diff --git a/vendor/golang.org/x/net/http2/server_test.go b/vendor/golang.org/x/net/http2/server_test.go index 2e6146b67..dfa4cff2e 100644 --- a/vendor/golang.org/x/net/http2/server_test.go +++ b/vendor/golang.org/x/net/http2/server_test.go @@ -45,13 +45,22 @@ type serverTester struct { t testing.TB ts *httptest.Server fr *Framer - logBuf *bytes.Buffer - logFilter []string // substrings to filter out - scMu sync.Mutex // guards sc + serverLogBuf bytes.Buffer // logger for httptest.Server + logFilter []string // substrings to filter out + scMu sync.Mutex // guards sc sc *serverConn hpackDec *hpack.Decoder decodedHeaders [][2]string + // If http2debug!=2, then we capture Frame debug logs that will be written + // to t.Log after a test fails. The read and write logs use separate locks + // and buffers so we don't accidentally introduce synchronization between + // the read and write goroutines, which may hide data races. + frameReadLogMu sync.Mutex + frameReadLogBuf bytes.Buffer + frameWriteLogMu sync.Mutex + frameWriteLogBuf bytes.Buffer + // writing headers: headerBuf bytes.Buffer hpackEnc *hpack.Encoder @@ -75,7 +84,6 @@ var optQuiet = serverTesterOpt("quiet_logging") func newServerTester(t testing.TB, handler http.HandlerFunc, opts ...interface{}) *serverTester { resetHooks() - logBuf := new(bytes.Buffer) ts := httptest.NewUnstartedServer(handler) tlsConfig := &tls.Config{ @@ -110,9 +118,8 @@ func newServerTester(t testing.TB, handler http.HandlerFunc, opts ...interface{} ConfigureServer(ts.Config, h2server) st := &serverTester{ - t: t, - ts: ts, - logBuf: logBuf, + t: t, + ts: ts, } st.hpackEnc = hpack.NewEncoder(&st.headerBuf) st.hpackDec = hpack.NewDecoder(initialHeaderTableSize, st.onHeaderField) @@ -121,7 +128,7 @@ func newServerTester(t testing.TB, handler http.HandlerFunc, opts ...interface{} if quiet { ts.Config.ErrorLog = log.New(ioutil.Discard, "", 0) } else { - ts.Config.ErrorLog = log.New(io.MultiWriter(stderrv(), twriter{t: t, st: st}, logBuf), "", log.LstdFlags) + ts.Config.ErrorLog = log.New(io.MultiWriter(stderrv(), twriter{t: t, st: st}, &st.serverLogBuf), "", log.LstdFlags) } ts.StartTLS() @@ -142,6 +149,22 @@ func newServerTester(t testing.TB, handler http.HandlerFunc, opts ...interface{} } st.cc = cc st.fr = NewFramer(cc, cc) + if !logFrameReads && !logFrameWrites { + st.fr.debugReadLoggerf = func(m string, v ...interface{}) { + m = time.Now().Format("2006-01-02 15:04:05.999999999 ") + strings.TrimPrefix(m, "http2: ") + "\n" + st.frameReadLogMu.Lock() + fmt.Fprintf(&st.frameReadLogBuf, m, v...) + st.frameReadLogMu.Unlock() + } + st.fr.debugWriteLoggerf = func(m string, v ...interface{}) { + m = time.Now().Format("2006-01-02 15:04:05.999999999 ") + strings.TrimPrefix(m, "http2: ") + "\n" + st.frameWriteLogMu.Lock() + fmt.Fprintf(&st.frameWriteLogBuf, m, v...) + st.frameWriteLogMu.Unlock() + } + st.fr.logReads = true + st.fr.logWrites = true + } } return st } @@ -201,6 +224,18 @@ func (st *serverTester) awaitIdle() { func (st *serverTester) Close() { if st.t.Failed() { + st.frameReadLogMu.Lock() + if st.frameReadLogBuf.Len() > 0 { + st.t.Logf("Framer read log:\n%s", st.frameReadLogBuf.String()) + } + st.frameReadLogMu.Unlock() + + st.frameWriteLogMu.Lock() + if st.frameWriteLogBuf.Len() > 0 { + st.t.Logf("Framer write log:\n%s", st.frameWriteLogBuf.String()) + } + st.frameWriteLogMu.Unlock() + // If we failed already (and are likely in a Fatal, // unwindowing), force close the connection, so the // httptest.Server doesn't wait forever for the conn @@ -1100,10 +1135,10 @@ func TestServer_RejectsLargeFrames(t *testing.T) { if gf.ErrCode != ErrCodeFrameSize { t.Errorf("GOAWAY err = %v; want %v", gf.ErrCode, ErrCodeFrameSize) } - if st.logBuf.Len() != 0 { + if st.serverLogBuf.Len() != 0 { // Previously we spun here for a bit until the GOAWAY disconnect // timer fired, logging while we fired. - t.Errorf("unexpected server output: %.500s\n", st.logBuf.Bytes()) + t.Errorf("unexpected server output: %.500s\n", st.serverLogBuf.Bytes()) } } @@ -1227,6 +1262,7 @@ func testServerPostUnblock(t *testing.T, inHandler <- true errc <- handler(w, r) }) + defer st.Close() st.greet() st.writeHeaders(HeadersFrameParam{ StreamID: 1, @@ -1244,7 +1280,6 @@ func testServerPostUnblock(t *testing.T, case <-time.After(5 * time.Second): t.Fatal("timeout waiting for Handler to return") } - st.Close() } func TestServer_RSTStream_Unblocks_Read(t *testing.T) { diff --git a/vendor/golang.org/x/net/http2/transport.go b/vendor/golang.org/x/net/http2/transport.go index 8f5f84412..0c7e859db 100644 --- a/vendor/golang.org/x/net/http2/transport.go +++ b/vendor/golang.org/x/net/http2/transport.go @@ -191,6 +191,7 @@ type clientStream struct { ID uint32 resc chan resAndError bufPipe pipe // buffered pipe with the flow-controlled response payload + startedWrite bool // started request body write; guarded by cc.mu requestedGzip bool on100 func() // optional code to run if get a 100 continue response @@ -314,6 +315,10 @@ func authorityAddr(scheme string, authority string) (addr string) { if a, err := idna.ToASCII(host); err == nil { host = a } + // IPv6 address literal, without a port: + if strings.HasPrefix(host, "[") && strings.HasSuffix(host, "]") { + return host + ":" + port + } return net.JoinHostPort(host, port) } @@ -332,8 +337,10 @@ func (t *Transport) RoundTripOpt(req *http.Request, opt RoundTripOpt) (*http.Res } traceGotConn(req, cc) res, err := cc.RoundTrip(req) - if shouldRetryRequest(req, err) { - continue + if err != nil { + if req, err = shouldRetryRequest(req, err); err == nil { + continue + } } if err != nil { t.vlogf("RoundTrip failure: %v", err) @@ -355,12 +362,41 @@ func (t *Transport) CloseIdleConnections() { var ( errClientConnClosed = errors.New("http2: client conn is closed") errClientConnUnusable = errors.New("http2: client conn not usable") + + errClientConnGotGoAway = errors.New("http2: Transport received Server's graceful shutdown GOAWAY") + errClientConnGotGoAwayAfterSomeReqBody = errors.New("http2: Transport received Server's graceful shutdown GOAWAY; some request body already written") ) -func shouldRetryRequest(req *http.Request, err error) bool { - // TODO: retry GET requests (no bodies) more aggressively, if shutdown - // before response. - return err == errClientConnUnusable +// 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 +// modified clone), or an error if the request can't be replayed. +func shouldRetryRequest(req *http.Request, err error) (*http.Request, error) { + switch err { + default: + return nil, err + case errClientConnUnusable, errClientConnGotGoAway: + return req, nil + case errClientConnGotGoAwayAfterSomeReqBody: + // If the Body is nil (or http.NoBody), it's safe to reuse + // this request and its Body. + if req.Body == nil || reqBodyIsNoBody(req.Body) { + return req, nil + } + // Otherwise we depend on the Request having its GetBody + // func defined. + getBody := reqGetBody(req) // Go 1.8: getBody = req.GetBody + if getBody == nil { + return nil, errors.New("http2: Transport: peer server initiated graceful shutdown after some of Request.Body was written; define Request.GetBody to avoid this error") + } + body, err := getBody() + if err != nil { + return nil, err + } + newReq := *req + newReq.Body = body + return &newReq, nil + } } func (t *Transport) dialClientConn(addr string, singleUse bool) (*ClientConn, error) { @@ -513,6 +549,15 @@ func (cc *ClientConn) setGoAway(f *GoAwayFrame) { if old != nil && old.ErrCode != ErrCodeNo { cc.goAway.ErrCode = old.ErrCode } + last := f.LastStreamID + for streamID, cs := range cc.streams { + if streamID > last { + select { + case cs.resc <- resAndError{err: errClientConnGotGoAway}: + default: + } + } + } } func (cc *ClientConn) CanTakeNewRequest() bool { @@ -613,8 +658,6 @@ func commaSeparatedTrailers(req *http.Request) (string, error) { } if len(keys) > 0 { sort.Strings(keys) - // TODO: could do better allocation-wise here, but trailers are rare, - // so being lazy for now. return strings.Join(keys, ","), nil } return "", nil @@ -773,6 +816,13 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cs.abortRequestBodyWrite(errStopReqBodyWrite) } if re.err != nil { + if re.err == errClientConnGotGoAway { + cc.mu.Lock() + if cs.startedWrite { + re.err = errClientConnGotGoAwayAfterSomeReqBody + } + cc.mu.Unlock() + } cc.forgetStreamID(cs.ID) return nil, re.err } @@ -2013,6 +2063,9 @@ func (t *Transport) getBodyWriterState(cs *clientStream, body io.Reader) (s body resc := make(chan error, 1) s.resc = resc s.fn = func() { + cs.cc.mu.Lock() + cs.startedWrite = true + cs.cc.mu.Unlock() resc <- cs.writeRequestBody(body, cs.req.Body) } s.delay = t.expectContinueTimeout() diff --git a/vendor/golang.org/x/net/http2/transport_test.go b/vendor/golang.org/x/net/http2/transport_test.go index f9287e575..8ef4f3388 100644 --- a/vendor/golang.org/x/net/http2/transport_test.go +++ b/vendor/golang.org/x/net/http2/transport_test.go @@ -2073,10 +2073,11 @@ func TestTransportHandlerBodyClose(t *testing.T) { // https://golang.org/issue/15930 func TestTransportFlowControl(t *testing.T) { - const ( - total = 100 << 20 // 100MB - bufLen = 1 << 16 - ) + const bufLen = 64 << 10 + var total int64 = 100 << 20 // 100MB + if testing.Short() { + total = 10 << 20 + } var wrote int64 // updated atomically st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) { @@ -2745,3 +2746,171 @@ func TestTransportCancelDataResponseRace(t *testing.T) { t.Errorf("Got = %q; want %q", slurp, msg) } } + +func TestTransportRetryAfterGOAWAY(t *testing.T) { + var dialer struct { + sync.Mutex + count int + } + ct1 := make(chan *clientTester) + ct2 := make(chan *clientTester) + + ln := newLocalListener(t) + defer ln.Close() + + tr := &Transport{ + TLSClientConfig: tlsConfigInsecure, + } + tr.DialTLS = func(network, addr string, cfg *tls.Config) (net.Conn, error) { + dialer.Lock() + defer dialer.Unlock() + dialer.count++ + if dialer.count == 3 { + return nil, errors.New("unexpected number of dials") + } + cc, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + return nil, fmt.Errorf("dial error: %v", err) + } + sc, err := ln.Accept() + if err != nil { + return nil, fmt.Errorf("accept error: %v", err) + } + ct := &clientTester{ + t: t, + tr: tr, + cc: cc, + sc: sc, + fr: NewFramer(sc, sc), + } + switch dialer.count { + case 1: + ct1 <- ct + case 2: + ct2 <- ct + } + return cc, nil + } + + errs := make(chan error, 3) + done := make(chan struct{}) + defer close(done) + + // Client. + go func() { + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + res, err := tr.RoundTrip(req) + if res != nil { + res.Body.Close() + if got := res.Header.Get("Foo"); got != "bar" { + err = fmt.Errorf("foo header = %q; want bar", got) + } + } + if err != nil { + err = fmt.Errorf("RoundTrip: %v", err) + } + errs <- err + }() + + connToClose := make(chan io.Closer, 2) + + // Server for the first request. + go func() { + var ct *clientTester + select { + case ct = <-ct1: + case <-done: + return + } + + connToClose <- ct.cc + ct.greet() + hf, err := ct.firstHeaders() + if err != nil { + errs <- fmt.Errorf("server1 failed reading HEADERS: %v", err) + return + } + t.Logf("server1 got %v", hf) + if err := ct.fr.WriteGoAway(0 /*max id*/, ErrCodeNo, nil); err != nil { + errs <- fmt.Errorf("server1 failed writing GOAWAY: %v", err) + return + } + errs <- nil + }() + + // Server for the second request. + go func() { + var ct *clientTester + select { + case ct = <-ct2: + case <-done: + return + } + + connToClose <- ct.cc + ct.greet() + hf, err := ct.firstHeaders() + if err != nil { + errs <- fmt.Errorf("server2 failed reading HEADERS: %v", err) + return + } + t.Logf("server2 got %v", hf) + + var buf bytes.Buffer + enc := hpack.NewEncoder(&buf) + enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"}) + enc.WriteField(hpack.HeaderField{Name: "foo", Value: "bar"}) + err = ct.fr.WriteHeaders(HeadersFrameParam{ + StreamID: hf.StreamID, + EndHeaders: true, + EndStream: false, + BlockFragment: buf.Bytes(), + }) + if err != nil { + errs <- fmt.Errorf("server2 failed writing response HEADERS: %v", err) + } else { + errs <- nil + } + }() + + for k := 0; k < 3; k++ { + select { + case err := <-errs: + if err != nil { + t.Error(err) + } + case <-time.After(1 * time.Second): + t.Errorf("timed out") + } + } + + for { + select { + case c := <-connToClose: + c.Close() + default: + return + } + } +} + +func TestAuthorityAddr(t *testing.T) { + tests := []struct { + scheme, authority string + want string + }{ + {"http", "foo.com", "foo.com:80"}, + {"https", "foo.com", "foo.com:443"}, + {"https", "foo.com:1234", "foo.com:1234"}, + {"https", "1.2.3.4:1234", "1.2.3.4:1234"}, + {"https", "1.2.3.4", "1.2.3.4:443"}, + {"https", "[::1]:1234", "[::1]:1234"}, + {"https", "[::1]", "[::1]:443"}, + } + for _, tt := range tests { + got := authorityAddr(tt.scheme, tt.authority) + if got != tt.want { + t.Errorf("authorityAddr(%q, %q) = %q; want %q", tt.scheme, tt.authority, got, tt.want) + } + } +} diff --git a/vendor/golang.org/x/net/http2/write.go b/vendor/golang.org/x/net/http2/write.go index 1c135fdf7..6b0dfae31 100644 --- a/vendor/golang.org/x/net/http2/write.go +++ b/vendor/golang.org/x/net/http2/write.go @@ -45,9 +45,10 @@ type writeContext interface { HeaderEncoder() (*hpack.Encoder, *bytes.Buffer) } -// endsStream reports whether the given frame writer w will locally -// close the stream. -func endsStream(w writeFramer) bool { +// writeEndsStream reports whether w writes a frame that will transition +// the stream to a half-closed local state. This returns false for RST_STREAM, +// which closes the entire stream (not just the local half). +func writeEndsStream(w writeFramer) bool { switch v := w.(type) { case *writeData: return v.endStream @@ -57,7 +58,7 @@ func endsStream(w writeFramer) bool { // This can only happen if the caller reuses w after it's // been intentionally nil'ed out to prevent use. Keep this // here to catch future refactoring breaking it. - panic("endsStream called on nil writeFramer") + panic("writeEndsStream called on nil writeFramer") } return false } diff --git a/vendor/golang.org/x/net/http2/writesched.go b/vendor/golang.org/x/net/http2/writesched.go index caa77c7cb..4fe307307 100644 --- a/vendor/golang.org/x/net/http2/writesched.go +++ b/vendor/golang.org/x/net/http2/writesched.go @@ -160,6 +160,20 @@ func (wr FrameWriteRequest) String() string { return fmt.Sprintf("[FrameWriteRequest stream=%d, ch=%v, writer=%v]", wr.StreamID(), wr.done != nil, des) } +// replyToWriter sends err to wr.done and panics if the send must block +// This does nothing if wr.done is nil. +func (wr *FrameWriteRequest) replyToWriter(err error) { + if wr.done == nil { + return + } + select { + case wr.done <- err: + default: + panic(fmt.Sprintf("unbuffered done channel passed in for type %T", wr.write)) + } + wr.write = nil // prevent use (assume it's tainted after wr.done send) +} + // writeQueue is used by implementations of WriteScheduler. type writeQueue struct { s []FrameWriteRequest diff --git a/vendor/golang.org/x/net/http2/writesched_priority_test.go b/vendor/golang.org/x/net/http2/writesched_priority_test.go index 2b232043c..f2b535a2c 100644 --- a/vendor/golang.org/x/net/http2/writesched_priority_test.go +++ b/vendor/golang.org/x/net/http2/writesched_priority_test.go @@ -434,7 +434,7 @@ func TestPriorityFlowControl(t *testing.T) { t.Fatalf("Pop(%d)=false, want true", i) } if got, want := wr.DataSize(), 8; got != want { - t.Fatalf("Pop(%d)=%d bytes, want %d bytes", got, want) + t.Fatalf("Pop(%d)=%d bytes, want %d bytes", i, got, want) } } } diff --git a/vendor/golang.org/x/net/http2/writesched_random_test.go b/vendor/golang.org/x/net/http2/writesched_random_test.go index 97b0bcdbf..3bf4aa36a 100644 --- a/vendor/golang.org/x/net/http2/writesched_random_test.go +++ b/vendor/golang.org/x/net/http2/writesched_random_test.go @@ -30,7 +30,7 @@ func TestRandomScheduler(t *testing.T) { t.Fatalf("got %d frames, expected 6", len(order)) } if order[0].StreamID() != 0 || order[1].StreamID() != 0 { - t.Fatalf("expected non-stream frames first", order[0], order[1]) + t.Fatal("expected non-stream frames first", order[0], order[1]) } got := make(map[uint32]bool) for _, wr := range order[2:] { -- cgit v1.2.3-1-g7c22