From 691e63b7fe69102c4db83a96cea0e19a4c345841 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 17 Dec 2015 00:31:56 +0000 Subject: [PATCH] net/http: update bundled copy of http2, enable TestTrailersServerToClient tests This CL updates the bundled copy of x/net/http2 to include https://golang.org/cl/17930 and enables the previously-skipped tests TestTrailersServerToClient_h2 and TestTrailersServerToClient_Flush_h2. It also updates the docs on http.Response.Trailer to describe how to use it. No change in rules. Just documenting the old unwritten rules. (there were tests locking in the behavior, and misc docs and examples scattered about, but not on http.Response.Trailer itself) Updates #13557 Change-Id: I6261d439f6c0d17654a1a7928790e8ffed16df6c Reviewed-on: https://go-review.googlesource.com/17931 Run-TryBot: Brad Fitzpatrick Reviewed-by: Blake Mizerany --- src/net/http/clientserver_test.go | 35 +++-- src/net/http/h2_bundle.go | 208 +++++++++++++++++++++++++----- src/net/http/response.go | 6 + 3 files changed, 203 insertions(+), 46 deletions(-) diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index 09dbceb99d..00d1c58cf0 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -526,16 +526,10 @@ func testTrailersClientToServer(t *testing.T, h2 bool) { } // Tests that servers send trailers to a client and that the client can read them. -func TestTrailersServerToClient_h1(t *testing.T) { testTrailersServerToClient(t, h1Mode, false) } -func TestTrailersServerToClient_h2(t *testing.T) { - t.Skip("skipping in http2 mode; golang.org/issue/13557") - testTrailersServerToClient(t, h2Mode, false) -} +func TestTrailersServerToClient_h1(t *testing.T) { testTrailersServerToClient(t, h1Mode, false) } +func TestTrailersServerToClient_h2(t *testing.T) { testTrailersServerToClient(t, h2Mode, false) } func TestTrailersServerToClient_Flush_h1(t *testing.T) { testTrailersServerToClient(t, h1Mode, true) } -func TestTrailersServerToClient_Flush_h2(t *testing.T) { - t.Skip("skipping in http2 mode; golang.org/issue/13557") - testTrailersServerToClient(t, h2Mode, true) -} +func TestTrailersServerToClient_Flush_h2(t *testing.T) { testTrailersServerToClient(t, h2Mode, true) } func testTrailersServerToClient(t *testing.T, h2, flush bool) { defer afterTest(t) @@ -564,11 +558,26 @@ func testTrailersServerToClient(t *testing.T, h2, flush bool) { t.Fatal(err) } - delete(res.Header, "Date") // irrelevant for test - if got, want := res.Header, (Header{ + wantHeader := Header{ "Content-Type": {"text/plain; charset=utf-8"}, - }); !reflect.DeepEqual(got, want) { - t.Errorf("Header = %v; want %v", got, want) + } + wantLen := -1 + if h2 && !flush { + // In HTTP/1.1, any use of trailers forces HTTP/1.1 + // chunking and a flush at the first write. That's + // unnecessary with HTTP/2's framing, so the server + // is able to calculate the length while still sending + // trailers afterwards. + wantLen = len(body) + wantHeader["Content-Length"] = []string{fmt.Sprint(wantLen)} + } + if res.ContentLength != int64(wantLen) { + t.Errorf("ContentLength = %v; want %v", res.ContentLength, wantLen) + } + + delete(res.Header, "Date") // irrelevant for test + if !reflect.DeepEqual(res.Header, wantHeader) { + t.Errorf("Header = %v; want %v", res.Header, wantHeader) } if got, want := res.Trailer, (Header{ diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index 89a8fd094a..b793f18416 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -28,6 +28,7 @@ import ( "io/ioutil" "log" "net" + "net/textproto" "net/url" "os" "runtime" @@ -3706,12 +3707,13 @@ type http2responseWriterState struct { bw *bufio.Writer // writing to a chunkWriter{this *responseWriterState} // mutated by http.Handler goroutine: - handlerHeader Header // nil until called - snapHeader Header // snapshot of handlerHeader at WriteHeader time - status int // status code passed to WriteHeader - wroteHeader bool // WriteHeader called (explicitly or implicitly). Not necessarily sent to user yet. - sentHeader bool // have we sent the header frame? - handlerDone bool // handler has finished + handlerHeader Header // nil until called + snapHeader Header // snapshot of handlerHeader at WriteHeader time + trailers []string // set in writeChunk + status int // status code passed to WriteHeader + wroteHeader bool // WriteHeader called (explicitly or implicitly). Not necessarily sent to user yet. + sentHeader bool // have we sent the header frame? + handlerDone bool // handler has finished sentContentLen int64 // non-zero if handler set a Content-Length header wroteBytes int64 @@ -3724,6 +3726,21 @@ type http2chunkWriter struct{ rws *http2responseWriterState } func (cw http2chunkWriter) Write(p []byte) (n int, err error) { return cw.rws.writeChunk(p) } +func (rws *http2responseWriterState) hasTrailers() bool { return len(rws.trailers) != 0 } + +// declareTrailer is called for each Trailer header when the +// response header is written. It notes that a header will need to be +// written in the trailers at the end of the response. +func (rws *http2responseWriterState) declareTrailer(k string) { + k = CanonicalHeaderKey(k) + switch k { + case "Transfer-Encoding", "Content-Length", "Trailer": + + return + } + rws.trailers = append(rws.trailers, k) +} + // writeChunk writes chunks from the bufio.Writer. But because // bufio.Writer may bypass its chunking, sometimes p may be // arbitrarily large. @@ -3734,6 +3751,7 @@ func (rws *http2responseWriterState) writeChunk(p []byte) (n int, err error) { if !rws.wroteHeader { rws.writeHeader(200) } + isHeadResp := rws.req.Method == "HEAD" if !rws.sentHeader { rws.sentHeader = true @@ -3759,7 +3777,12 @@ func (rws *http2responseWriterState) writeChunk(p []byte) (n int, err error) { date = time.Now().UTC().Format(TimeFormat) } - endStream := (rws.handlerDone && len(p) == 0) || isHeadResp + + for _, v := range rws.snapHeader["Trailer"] { + http2foreachHeaderElement(v, rws.declareTrailer) + } + + endStream := (rws.handlerDone && !rws.hasTrailers() && len(p) == 0) || isHeadResp err = rws.conn.writeHeaders(rws.stream, &http2writeResHeaders{ streamID: rws.stream.id, httpResCode: rws.status, @@ -3783,8 +3806,22 @@ func (rws *http2responseWriterState) writeChunk(p []byte) (n int, err error) { return 0, nil } - if err := rws.conn.writeDataFromHandler(rws.stream, p, rws.handlerDone); err != nil { - return 0, err + endStream := rws.handlerDone && !rws.hasTrailers() + if len(p) > 0 || endStream { + + if err := rws.conn.writeDataFromHandler(rws.stream, p, endStream); err != nil { + return 0, err + } + } + + if rws.handlerDone && rws.hasTrailers() { + err = rws.conn.writeHeaders(rws.stream, &http2writeResHeaders{ + streamID: rws.stream.id, + h: rws.handlerHeader, + trailers: rws.trailers, + endStream: true, + }) + return len(p), err } return len(p), nil } @@ -3912,6 +3949,24 @@ func (w *http2responseWriter) handlerDone() { http2responseWriterStatePool.Put(rws) } +// foreachHeaderElement splits v according to the "#rule" construction +// in RFC 2616 section 2.1 and calls fn for each non-empty element. +func http2foreachHeaderElement(v string, fn func(string)) { + v = textproto.TrimString(v) + if v == "" { + return + } + if !strings.Contains(v, ",") { + fn(v) + return + } + for _, f := range strings.Split(v, ",") { + if f = textproto.TrimString(f); f != "" { + fn(f) + } + } +} + const ( // transportDefaultConnFlow is how many connection-level flow control // tokens we give the server at start-up, past the default 64k. @@ -4045,6 +4100,13 @@ type http2clientStream struct { peerReset chan struct{} // closed on peer reset resetErr error // populated before peerReset is closed + + // owned by clientConnReadLoop: + headersDone bool // got HEADERS w/ END_HEADERS + trailersDone bool // got second HEADERS frame w/ END_HEADERS + + trailer Header // accumulated trailers + resTrailer Header // client's Response.Trailer } // awaitRequestCancel runs in its own goroutine and waits for the user's @@ -4753,9 +4815,14 @@ func (rl *http2clientConnReadLoop) processHeaderBlockFragment(frag []byte, strea return nil } + if cs.headersDone { + rl.hdec.SetEmitFunc(cs.onNewTrailerField) + } else { + rl.hdec.SetEmitFunc(rl.onNewHeaderField) + } _, err := rl.hdec.Write(frag) if err != nil { - return err + return http2ConnectionError(http2ErrCodeCompression) } if !headersEnded { rl.continueStreamID = cs.ID @@ -4764,6 +4831,23 @@ func (rl *http2clientConnReadLoop) processHeaderBlockFragment(frag []byte, strea rl.continueStreamID = 0 + if !cs.headersDone { + cs.headersDone = true + } else { + + if cs.trailersDone { + + return http2ConnectionError(http2ErrCodeProtocol) + } + cs.trailersDone = true + if !streamEnded { + + return http2ConnectionError(http2ErrCodeProtocol) + } + rl.endStream(cs) + return nil + } + if rl.reqMalformed != nil { cs.resc <- http2resAndError{err: rl.reqMalformed} rl.cc.writeStreamReset(cs.ID, http2ErrCodeProtocol, rl.reqMalformed) @@ -4802,6 +4886,7 @@ func (rl *http2clientConnReadLoop) processHeaderBlockFragment(frag []byte, strea } } + cs.resTrailer = res.Trailer rl.activeRes[cs.ID] = cs cs.resc <- http2resAndError{res: res} rl.nextRes = nil @@ -4907,12 +4992,23 @@ func (rl *http2clientConnReadLoop) processData(f *http2DataFrame) error { } if f.StreamEnded() { - cs.bufPipe.CloseWithError(io.EOF) - delete(rl.activeRes, cs.ID) + rl.endStream(cs) } return nil } +func (rl *http2clientConnReadLoop) endStream(cs *http2clientStream) { + + cs.bufPipe.closeWithErrorAndCode(io.EOF, cs.copyTrailers) + delete(rl.activeRes, cs.ID) +} + +func (cs *http2clientStream) copyTrailers() { + for k, vv := range cs.trailer { + cs.resTrailer[k] = vv + } +} + func (rl *http2clientConnReadLoop) processGoAway(f *http2GoAwayFrame) error { cc := rl.cc cc.t.connPool().MarkDead(cc) @@ -5019,6 +5115,7 @@ func (rl *http2clientConnReadLoop) onNewHeaderField(f hpack.HeaderField) { if http2VerboseLogs { cc.logf("Header field: %+v", f) } + isPseudo := strings.HasPrefix(f.Name, ":") if isPseudo { if rl.sawRegHeader { @@ -5040,7 +5137,37 @@ func (rl *http2clientConnReadLoop) onNewHeaderField(f hpack.HeaderField) { } } else { rl.sawRegHeader = true - rl.nextRes.Header.Add(CanonicalHeaderKey(f.Name), f.Value) + key := CanonicalHeaderKey(f.Name) + if key == "Trailer" { + t := rl.nextRes.Trailer + if t == nil { + t = make(Header) + rl.nextRes.Trailer = t + } + http2foreachHeaderElement(f.Value, func(v string) { + t[CanonicalHeaderKey(v)] = nil + }) + } else { + rl.nextRes.Header.Add(key, f.Value) + } + } +} + +func (cs *http2clientStream) onNewTrailerField(f hpack.HeaderField) { + isPseudo := strings.HasPrefix(f.Name, ":") + if isPseudo { + + return + } + key := CanonicalHeaderKey(f.Name) + if _, ok := cs.resTrailer[key]; ok { + if cs.trailer == nil { + cs.trailer = make(Header) + } + const tooBig = 1000 // TODO: arbitrary; use max header list size limits + if cur := cs.trailer[key]; len(cur) < tooBig { + cs.trailer[key] = append(cur, f.Value) + } } } @@ -5205,11 +5332,12 @@ func (http2writeSettingsAck) writeFrame(ctx http2writeContext) error { } // writeResHeaders is a request to write a HEADERS and 0+ CONTINUATION frames -// for HTTP response headers from a server handler. +// for HTTP response headers or trailers from a server handler. type http2writeResHeaders struct { streamID uint32 - httpResCode int - h Header // may be nil + httpResCode int // 0 means no ":status" line + h Header // may be nil + trailers []string // if non-nil, which keys of h to write. nil means all. endStream bool date string @@ -5220,25 +5348,16 @@ type http2writeResHeaders struct { func (w *http2writeResHeaders) writeFrame(ctx http2writeContext) error { enc, buf := ctx.HeaderEncoder() buf.Reset() - enc.WriteField(hpack.HeaderField{Name: ":status", Value: http2httpCodeString(w.httpResCode)}) - keys := make([]string, 0, len(w.h)) - for k := range w.h { - keys = append(keys, k) + if w.httpResCode != 0 { + enc.WriteField(hpack.HeaderField{ + Name: ":status", + Value: http2httpCodeString(w.httpResCode), + }) } - sort.Strings(keys) - for _, k := range keys { - vv := w.h[k] - k = http2lowerHeader(k) - isTE := k == "transfer-encoding" - for _, v := range vv { - if isTE && v != "trailers" { - continue - } - enc.WriteField(hpack.HeaderField{Name: k, Value: v}) - } - } + http2encodeHeaders(enc, w.h, w.trailers) + if w.contentType != "" { enc.WriteField(hpack.HeaderField{Name: "content-type", Value: w.contentType}) } @@ -5250,7 +5369,7 @@ func (w *http2writeResHeaders) writeFrame(ctx http2writeContext) error { } headerBlock := buf.Bytes() - if len(headerBlock) == 0 { + if len(headerBlock) == 0 && w.trailers == nil { panic("unexpected empty hpack") } @@ -5314,6 +5433,29 @@ func (wu http2writeWindowUpdate) writeFrame(ctx http2writeContext) error { return ctx.Framer().WriteWindowUpdate(wu.streamID, wu.n) } +func http2encodeHeaders(enc *hpack.Encoder, h Header, keys []string) { + + if keys == nil { + keys = make([]string, 0, len(h)) + for k := range h { + keys = append(keys, k) + } + sort.Strings(keys) + } + for _, k := range keys { + vv := h[k] + k = http2lowerHeader(k) + isTE := k == "transfer-encoding" + for _, v := range vv { + + if isTE && v != "trailers" { + continue + } + enc.WriteField(hpack.HeaderField{Name: k, Value: v}) + } + } +} + // frameWriteMsg is a request to write a frame. type http2frameWriteMsg struct { // write is the interface value that does the writing, once the diff --git a/src/net/http/response.go b/src/net/http/response.go index 76b8538524..0e39ed3a3a 100644 --- a/src/net/http/response.go +++ b/src/net/http/response.go @@ -74,6 +74,12 @@ type Response struct { // Trailer maps trailer keys to values, in the same // format as the header. + // + // The Trailer initially contains only the server's + // pre-declared trailer keys, but with nil values. Trailer + // must not be access concurrently with Read calls on the + // Body. After Body.Read has returned io.EOF, Trailer can be read + // again and will contain any values sent by the server. Trailer Header // The Request that was sent to obtain this Response.