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 <bradfitz@golang.org>
Reviewed-by: Blake Mizerany <blake.mizerany@gmail.com>

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.