http2: improve error when server sends HTTP/1

If for some reason the server sends an HTTP/1.1 response response
starting with "HTTP/1.1 " (9 bytes), http2 transport interprets that as
a valid frame header for an expected payload of ~4MB, and fails with
non-descriptive messages like "unexpected EOF".

This could happen, for example, if ALPN is misconfigured on the server's
load balancer.

This change attempts to improve that feedback by noting in the error
messages whenever the frame header was exactly the "HTTP/1.1 bytes".

Signed-off-by: Oleg Zaytsev <[email protected]>

Author: colega

http2/frame.go

@@ -225,6 +225,16 @@ var fhBytes = sync.Pool{
 	},
 }
 
+var invalidHTTP1LookingFrameHeader = func() FrameHeader {
+	fh, err := readFrameHeader(make([]byte, frameHeaderLen), strings.NewReader("HTTP/1.1 "))
+	if err != nil {
+		panic(err)
+	}
+	return fh
+}()
+
+func (h FrameHeader) looksLikeHTTP1Header() bool { return h == invalidHTTP1LookingFrameHeader }
+
 // ReadFrameHeader reads 9 bytes from r and returns a FrameHeader.
 // Most users should use Framer.ReadFrame instead.
 func ReadFrameHeader(r io.Reader) (FrameHeader, error) {
@@ -503,10 +513,16 @@ func (fr *Framer) ReadFrame() (Frame, error) {
 		return nil, err
 	}
 	if fh.Length > fr.maxReadSize {
+		if fh.looksLikeHTTP1Header() {
+			return nil, fmt.Errorf("http2: failed reading the frame payload: %w, note that the frame header looked like an HTTP/1.1 header", err)
+		}
 		return nil, ErrFrameTooLarge
 	}
 	payload := fr.getReadBuf(fh.Length)
 	if _, err := io.ReadFull(fr.r, payload); err != nil {
+		if fh.looksLikeHTTP1Header() {
+			return nil, fmt.Errorf("http2: failed reading the frame payload: %w, note that the frame header looked like an HTTP/1.1 header", err)
+		}
 		return nil, err
 	}
 	f, err := typeFrameParser(fh.Type)(fr.frameCache, fh, fr.countError, payload)

http2/frame_test.go

@@ -626,6 +626,27 @@ func TestReadFrameHeader(t *testing.T) {
 	}
 }
 
+func TestReadFrameHeader_FromHTTP1Header(t *testing.T) {
+	tests := []struct {
+		in             string
+		looksLikeHTTP1 bool
+	}{
+		// Ignore high bit:
+		{in: "\xff\xff\xff" + "\xff" + "\xff" + "\xff\xff\xff\xff", looksLikeHTTP1: false},
+		{in: "HTTP/1.1 400 Bad Request\r\n", looksLikeHTTP1: true},
+	}
+	for i, tt := range tests {
+		got, err := readFrameHeader(make([]byte, 9), strings.NewReader(tt.in))
+		if err != nil {
+			t.Errorf("%d. readFrameHeader(%q) = %v", i, tt.in, err)
+			continue
+		}
+		if got.looksLikeHTTP1Header() != tt.looksLikeHTTP1 {
+			t.Errorf("%d. readFrameHeader(%q).looksLikeHTTP1Header = %v; want %v", i, tt.in, got.looksLikeHTTP1Header(), tt.looksLikeHTTP1)
+		}
+	}
+}
+
 func TestReadWriteFrameHeader(t *testing.T) {
 	tests := []struct {
 		len      uint32

http2/transport_test.go

@@ -272,6 +272,48 @@ func TestTransport(t *testing.T) {
 	}
 }
 
+func TestTransportFailureErrorForHTTP1Response(t *testing.T) {
+	const expectedHTTP1PayloadHint = "frame header looked like an HTTP/1.1 header"
+
+	ts := httptest.NewServer(http.NewServeMux())
+	t.Cleanup(ts.Close)
+
+	for _, tc := range []struct {
+		name            string
+		maxFrameSize    uint32
+		expectedErrorIs error
+	}{
+		{
+			name:         "with default max frame size",
+			maxFrameSize: 0,
+		},
+		{
+			name:         "with enough frame size to start reading",
+			maxFrameSize: invalidHTTP1LookingFrameHeader.Length + 1,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tr := &Transport{
+				DialTLSContext: func(ctx context.Context, network, addr string, cfg *tls.Config) (net.Conn, error) {
+					return net.Dial(network, addr)
+				},
+				MaxReadFrameSize: tc.maxFrameSize,
+				AllowHTTP:        true,
+			}
+
+			req, err := http.NewRequest("GET", ts.URL, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			_, err = tr.RoundTrip(req)
+			if !strings.Contains(err.Error(), expectedHTTP1PayloadHint) {
+				t.Errorf("expected error to contain %q, got %v", expectedHTTP1PayloadHint, err)
+			}
+		})
+	}
+}
+
 func testTransportReusesConns(t *testing.T, useClient, wantSame bool, modReq func(*http.Request)) {
 	ts := newTestServer(t, func(w http.ResponseWriter, r *http.Request) {
 		io.WriteString(w, r.RemoteAddr)