http2: relax Trailer predeclaration requirement in Transport

Change-Id: I742866d94c6a9325a4970d528f1e46b9e2d7fdbc
Reviewed-on: https://go-review.googlesource.com/18472Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

http2/transport.go

@@ -188,8 +188,8 @@ type clientStream struct {
 	pastHeaders  bool // got HEADERS w/ END_HEADERS
 	pastTrailers bool // got second HEADERS frame w/ END_HEADERS
 
-	trailer    http.Header // accumulated trailers
-	resTrailer http.Header // client's Response.Trailer
+	trailer    http.Header  // accumulated trailers
+	resTrailer *http.Header // client's Response.Trailer
 }
 
 // awaitRequestCancel runs in its own goroutine and waits for the user
@@ -1166,7 +1166,7 @@ func (rl *clientConnReadLoop) processHeaderBlockFragment(frag []byte, streamID u
 		}
 	}
 
-	cs.resTrailer = res.Trailer
+	cs.resTrailer = &res.Trailer
 	rl.activeRes[cs.ID] = cs
 	cs.resc <- resAndError{res: res}
 	rl.nextRes = nil // unused now; will be reset next HEADERS frame
@@ -1296,7 +1296,11 @@ func (rl *clientConnReadLoop) endStream(cs *clientStream) {
 
 func (cs *clientStream) copyTrailers() {
 	for k, vv := range cs.trailer {
-		cs.resTrailer[k] = vv
+		t := cs.resTrailer
+		if *t == nil {
+			*t = make(http.Header)
+		}
+		(*t)[k] = vv
 	}
 }
 
@@ -1516,7 +1520,12 @@ func (rl *clientConnReadLoop) onNewTrailerField(cs *clientStream, f hpack.Header
 	}
 
 	key := http.CanonicalHeaderKey(f.Name)
-	if _, ok := cs.resTrailer[key]; ok {
+
+	// The spec says one must predeclare their trailers but in practice
+	// popular users (which is to say the only user we found) do not so we
+	// violate the spec and accept all of them.
+	const acceptAllTrailers = true
+	if _, ok := (*cs.resTrailer)[key]; ok || acceptAllTrailers {
 		if cs.trailer == nil {
 			cs.trailer = make(http.Header)
 		}

http2/transport_test.go

@@ -1003,6 +1003,70 @@ func testTransportResPattern(t *testing.T, expect100Continue, resHeader headerTy
 	ct.run()
 }
 
+func TestTransportReceiveUndeclaredTrailer(t *testing.T) {
+	ct := newClientTester(t)
+	ct.client = func() error {
+		req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+		res, err := ct.tr.RoundTrip(req)
+		if err != nil {
+			return fmt.Errorf("RoundTrip: %v", err)
+		}
+		defer res.Body.Close()
+		if res.StatusCode != 200 {
+			return fmt.Errorf("status code = %v; want 200", res.StatusCode)
+		}
+		slurp, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			return fmt.Errorf("res.Body ReadAll error = %q, %v; want %v", slurp, err, nil)
+		}
+		if len(slurp) > 0 {
+			return fmt.Errorf("body = %q; want nothing", slurp)
+		}
+		if _, ok := res.Trailer["Some-Trailer"]; !ok {
+			return fmt.Errorf("expected Some-Trailer")
+		}
+		return nil
+	}
+	ct.server = func() error {
+		ct.greet()
+
+		var n int
+		var hf *HeadersFrame
+		for hf == nil && n < 10 {
+			f, err := ct.fr.ReadFrame()
+			if err != nil {
+				return err
+			}
+			hf, _ = f.(*HeadersFrame)
+			n++
+		}
+
+		var buf bytes.Buffer
+		enc := hpack.NewEncoder(&buf)
+
+		// send headers without Trailer header
+		enc.WriteField(hpack.HeaderField{Name: ":status", Value: "200"})
+		ct.fr.WriteHeaders(HeadersFrameParam{
+			StreamID:      hf.StreamID,
+			EndHeaders:    true,
+			EndStream:     false,
+			BlockFragment: buf.Bytes(),
+		})
+
+		// send trailers
+		buf.Reset()
+		enc.WriteField(hpack.HeaderField{Name: "some-trailer", Value: "I'm an undeclared Trailer!"})
+		ct.fr.WriteHeaders(HeadersFrameParam{
+			StreamID:      hf.StreamID,
+			EndHeaders:    true,
+			EndStream:     true,
+			BlockFragment: buf.Bytes(),
+		})
+		return nil
+	}
+	ct.run()
+}
+
 func TestTransportInvalidTrailer_Pseudo1(t *testing.T) {
 	testTransportInvalidTrailer_Pseudo(t, oneHeader)
 }