From 1119af89767dc4086cba336e732afcea084c8c34 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 18 May 2016 04:42:37 +0000 Subject: [PATCH] net/http: update bundled x/net/http2 for httptrace changes Updates x/net/http2 to 3b99394 for golang.org/cl/23205 And associated tests. Fixes #12580 Change-Id: I1f4b59267b453d241f2afaa315b7fe10d477e52d Reviewed-on: https://go-review.googlesource.com/23206 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/clientserver_test.go | 7 ++++ src/net/http/h2_bundle.go | 58 +++++++++++++++++++++++++++++++ src/net/http/httptrace/trace.go | 1 + src/net/http/transport_test.go | 38 +++++++++++--------- 4 files changed, 87 insertions(+), 17 deletions(-) diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index 39c1eaa04afad..b1b7d137d93c5 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -44,6 +44,13 @@ func (t *clientServerTest) close() { t.ts.Close() } +func (t *clientServerTest) scheme() string { + if t.h2 { + return "https" + } + return "http" +} + const ( h1Mode = false h2Mode = true diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index c2a2d37f6dfaa..21b10355a9d5b 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -30,6 +30,7 @@ import ( "io/ioutil" "log" "net" + "net/http/httptrace" "net/textproto" "net/url" "os" @@ -1973,10 +1974,52 @@ func http2summarizeFrame(f http2Frame) string { return buf.String() } +type http2clientTrace httptrace.ClientTrace + func http2reqContext(r *Request) context.Context { return r.Context() } func http2setResponseUncompressed(res *Response) { res.Uncompressed = true } +func http2traceGotConn(req *Request, cc *http2ClientConn) { + trace := httptrace.ContextClientTrace(req.Context()) + if trace == nil || trace.GotConn == nil { + return + } + ci := httptrace.GotConnInfo{Conn: cc.tconn} + cc.mu.Lock() + ci.Reused = cc.nextStreamID > 1 + ci.WasIdle = len(cc.streams) == 0 + if ci.WasIdle { + ci.IdleTime = time.Now().Sub(cc.lastActive) + } + cc.mu.Unlock() + + trace.GotConn(ci) +} + +func http2traceWroteHeaders(trace *http2clientTrace) { + if trace != nil && trace.WroteHeaders != nil { + trace.WroteHeaders() + } +} + +func http2traceWroteRequest(trace *http2clientTrace, err error) { + if trace != nil && trace.WroteRequest != nil { + trace.WroteRequest(httptrace.WroteRequestInfo{Err: err}) + } +} + +func http2traceFirstResponseByte(trace *http2clientTrace) { + if trace != nil && trace.GotFirstResponseByte != nil { + trace.GotFirstResponseByte() + } +} + +func http2requestTrace(req *Request) *http2clientTrace { + trace := httptrace.ContextClientTrace(req.Context()) + return (*http2clientTrace)(trace) +} + var http2DebugGoroutines = os.Getenv("DEBUG_HTTP2_GOROUTINES") == "1" type http2goroutineLock uint64 @@ -4879,6 +4922,8 @@ type http2ClientConn struct { bw *bufio.Writer br *bufio.Reader fr *http2Framer + lastActive time.Time + // Settings from peer: maxFrameSize uint32 maxConcurrentStreams uint32 @@ -4896,6 +4941,7 @@ type http2ClientConn struct { type http2clientStream struct { cc *http2ClientConn req *Request + trace *http2clientTrace // or nil ID uint32 resc chan http2resAndError bufPipe http2pipe // buffered pipe with the flow-controlled response payload @@ -5014,6 +5060,7 @@ func (t *http2Transport) RoundTripOpt(req *Request, opt http2RoundTripOpt) (*Res t.vlogf("http2: Transport failed to get client conn for %s: %v", addr, err) return nil, err } + http2traceGotConn(req, cc) res, err := cc.RoundTrip(req) if http2shouldRetryRequest(req, err) { continue @@ -5335,6 +5382,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { } cc.mu.Lock() + cc.lastActive = time.Now() if cc.closed || !cc.canTakeNewRequestLocked() { cc.mu.Unlock() return nil, http2errClientConnUnusable @@ -5342,6 +5390,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { cs := cc.newStream() cs.req = req + cs.trace = http2requestTrace(req) hasBody := body != nil if !cc.t.disableCompression() && @@ -5357,6 +5406,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { endStream := !hasBody && !hasTrailers werr := cc.writeHeaders(cs.ID, endStream, hdrs) cc.wmu.Unlock() + http2traceWroteHeaders(cs.trace) cc.mu.Unlock() if werr != nil { @@ -5365,6 +5415,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { } cc.forgetStreamID(cs.ID) + http2traceWroteRequest(cs.trace, werr) return nil, werr } @@ -5376,6 +5427,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { bodyCopyErrc <- cs.writeRequestBody(body, req.Body) }() } else { + http2traceWroteRequest(cs.trace, nil) if d := cc.responseHeaderTimeout(); d != 0 { timer := time.NewTimer(d) defer timer.Stop() @@ -5430,6 +5482,7 @@ func (cc *http2ClientConn) RoundTrip(req *Request) (*Response, error) { return nil, cs.resetErr case err := <-bodyCopyErrc: + http2traceWroteRequest(cs.trace, err) if err != nil { return nil, err } @@ -5729,6 +5782,7 @@ func (cc *http2ClientConn) streamByID(id uint32, andRemove bool) *http2clientStr defer cc.mu.Unlock() cs := cc.streams[id] if andRemove && cs != nil && !cc.closed { + cc.lastActive = time.Now() delete(cc.streams, id) close(cs.done) } @@ -5852,6 +5906,10 @@ func (rl *http2clientConnReadLoop) processHeaders(f *http2MetaHeadersFrame) erro } else { return rl.processTrailers(cs, f) } + if cs.trace != nil { + + http2traceFirstResponseByte(cs.trace) + } res, err := rl.handleResponse(cs, f) if err != nil { diff --git a/src/net/http/httptrace/trace.go b/src/net/http/httptrace/trace.go index 5d2c548b3cc31..6f187a7b694a8 100644 --- a/src/net/http/httptrace/trace.go +++ b/src/net/http/httptrace/trace.go @@ -90,6 +90,7 @@ type ClientTrace struct { // connection reuse is disabled via Transport.DisableKeepAlives. // PutIdleConn is called before the caller's Response.Body.Close // call returns. + // For HTTP/2, this hook is not currently used. PutIdleConn func(err error) // GotFirstResponseByte is called when the first byte of the response diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 328fd5727b9e6..48b1b309d330b 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -3193,26 +3193,26 @@ func TestTransportResponseHeaderLength(t *testing.T) { } } -func TestTransportEventTrace(t *testing.T) { testTransportEventTrace(t, false) } +func TestTransportEventTrace(t *testing.T) { testTransportEventTrace(t, h1Mode, false) } +func TestTransportEventTrace_h2(t *testing.T) { testTransportEventTrace(t, h2Mode, false) } // test a non-nil httptrace.ClientTrace but with all hooks set to zero. -func TestTransportEventTrace_NoHooks(t *testing.T) { testTransportEventTrace(t, true) } +func TestTransportEventTrace_NoHooks(t *testing.T) { testTransportEventTrace(t, h1Mode, true) } +func TestTransportEventTrace_NoHooks_h2(t *testing.T) { testTransportEventTrace(t, h2Mode, true) } -func testTransportEventTrace(t *testing.T, noHooks bool) { +func testTransportEventTrace(t *testing.T, h2 bool, noHooks bool) { defer afterTest(t) const resBody = "some body" - ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) { if _, err := ioutil.ReadAll(r.Body); err != nil { t.Error(err) } io.WriteString(w, resBody) })) - defer ts.Close() - tr := &Transport{ - ExpectContinueTimeout: 1 * time.Second, + defer cst.close() + if !h2 { + cst.tr.ExpectContinueTimeout = 1 * time.Second } - defer tr.CloseIdleConnections() - c := &Client{Transport: tr} var mu sync.Mutex var buf bytes.Buffer @@ -3223,7 +3223,8 @@ func testTransportEventTrace(t *testing.T, noHooks bool) { buf.WriteByte('\n') } - ip, port, err := net.SplitHostPort(ts.Listener.Addr().String()) + addrStr := cst.ts.Listener.Addr().String() + ip, port, err := net.SplitHostPort(addrStr) if err != nil { t.Fatal(err) } @@ -3237,7 +3238,7 @@ func testTransportEventTrace(t *testing.T, noHooks bool) { return []net.IPAddr{{IP: net.ParseIP(ip)}}, nil }) - req, _ := NewRequest("POST", "http://dns-is-faked.golang:"+port, strings.NewReader("some body")) + req, _ := NewRequest("POST", cst.scheme()+"://dns-is-faked.golang:"+port, strings.NewReader("some body")) trace := &httptrace.ClientTrace{ GetConn: func(hostPort string) { logf("Getting conn for %v ...", hostPort) }, GotConn: func(ci httptrace.GotConnInfo) { logf("got conn: %+v", ci) }, @@ -3263,7 +3264,7 @@ func testTransportEventTrace(t *testing.T, noHooks bool) { req = req.WithContext(httptrace.WithClientTrace(ctx, trace)) req.Header.Set("Expect", "100-continue") - res, err := c.Do(req) + res, err := cst.c.Do(req) if err != nil { t.Fatal(err) } @@ -3292,14 +3293,17 @@ func testTransportEventTrace(t *testing.T, noHooks bool) { wantSub("Getting conn for dns-is-faked.golang:" + port) wantSub("DNS start: {Host:dns-is-faked.golang}") wantSub("DNS done: {Addrs:[{IP:" + ip + " Zone:}] Err: Coalesced:false}") - wantSub("Connecting to tcp " + ts.Listener.Addr().String()) - wantSub("connected to tcp " + ts.Listener.Addr().String() + " = ") + wantSub("Connecting to tcp " + addrStr) + wantSub("connected to tcp " + addrStr + " = ") wantSub("Reused:false WasIdle:false IdleTime:0s") wantSub("first response byte") - wantSub("PutIdleConn = ") + if !h2 { + wantSub("PutIdleConn = ") + // TODO: implement these next two for Issue 13851 + wantSub("Wait100Continue") + wantSub("Got100Continue") + } wantSub("WroteRequest: {Err:}") - wantSub("Wait100Continue") - wantSub("Got100Continue") if strings.Contains(got, " to udp ") { t.Errorf("should not see UDP (DNS) connections") }