net/http/internal/http2: do not silently truncate large trailers

HTTP/2 transport currently applies Transport.MaxResponseHeaderBytes
limit to trailer headers. However, due to a missing check, when a server
sends trailers that exceeds the limit, it would silently truncate it
rather than giving an explicit error.

Change-Id: I749fa54447bb1eeb9c822cff6fb4c6416a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/796381
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>

src/net/http/internal/http2/transport.go

diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go
index beab55f..0b32ea7 100644
--- a/src/net/http/internal/http2/transport.go
+++ b/src/net/http/internal/http2/transport.go
@@ -2319,6 +2319,14 @@
 		// TODO: ConnectionError might be overly harsh? Check.
 		return ConnectionError(ErrCodeProtocol)
 	}
+	if f.Truncated {
+		rl.endStreamError(cs, StreamError{
+			StreamID: f.StreamID,
+			Code:     ErrCodeProtocol,
+			Cause:    errResponseHeaderListSize,
+		})
+		return nil
+	}
 
 	trailer := make(Header)
 	for _, hf := range f.RegularFields() {

src/net/http/internal/http2/transport_test.go

diff --git a/src/net/http/internal/http2/transport_test.go b/src/net/http/internal/http2/transport_test.go
index 3f45206..852a091 100644
--- a/src/net/http/internal/http2/transport_test.go
+++ b/src/net/http/internal/http2/transport_test.go
@@ -1360,8 +1360,14 @@
 }
 
 func TestTransportChecksResponseHeaderListSize(t *testing.T) {
-	synctest.Test(t, testTransportChecksResponseHeaderListSize)
+	t.Run("headers", func(t *testing.T) {
+		synctest.Test(t, testTransportChecksResponseHeaderListSize)
+	})
+	t.Run("trailers", func(t *testing.T) {
+		synctest.Test(t, testTransportChecksResponseTrailerHeaderListSize)
+	})
 }
+
 func testTransportChecksResponseHeaderListSize(t *testing.T) {
 	tc := newTestClientConn(t)
 	tc.greet()
@@ -1408,6 +1414,55 @@
 	}
 }
 
+func testTransportChecksResponseTrailerHeaderListSize(t *testing.T) {
+	tc := newTestClientConn(t)
+	tc.greet()
+
+	req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
+	rt := tc.roundTrip(req)
+
+	tc.wantFrameType(FrameHeaders)
+	tc.writeHeaders(HeadersFrameParam{
+		StreamID:   rt.streamID(),
+		EndHeaders: true,
+		EndStream:  false,
+		BlockFragment: tc.makeHeaderBlockFragment(
+			":status", "200",
+			"trailer", "x-trailer",
+		),
+	})
+	rt.wantStatus(200)
+
+	var hdr []string
+	large := strings.Repeat("a", 1<<10)
+	for range 5042 {
+		hdr = append(hdr, large, large)
+	}
+	hbf := tc.makeHeaderBlockFragment(hdr...)
+	// Note: this number might change if our hpack implementation changes.
+	if size, want := len(hbf), 6328; size != want {
+		t.Fatalf("encoding over 10MB of duplicate keypairs took %d bytes; expected %d", size, want)
+	}
+	tc.writeHeaders(HeadersFrameParam{
+		StreamID:      rt.streamID(),
+		EndHeaders:    true,
+		EndStream:     true,
+		BlockFragment: hbf,
+	})
+
+	_, err := rt.readBody()
+	if e, ok := err.(StreamError); ok {
+		err = e.Cause
+	}
+	if err != ErrResponseHeaderListSize {
+		t.Errorf("Read = %v, want %v", err, ErrResponseHeaderListSize)
+	}
+	// Verify that this is treated as a StreamError that does not close the
+	// whole connection down.
+	tc.wantFrameType(FrameRSTStream)
+	tc.wantIdle()
+}
+
 func TestTransportCookieHeaderSplit(t *testing.T) { synctest.Test(t, testTransportCookieHeaderSplit) }
 func testTransportCookieHeaderSplit(t *testing.T) {
 	tc := newTestClientConn(t)