transport: validate http 200 status for responses

internal/transport/http2_client.go

@@ -1266,7 +1266,7 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 		// that the peer is speaking gRPC and we are in gRPC mode.
 		isGRPC         = !initialHeader
 		mdata          = make(map[string][]string)
-		contentTypeErr string
+		contentTypeErr = "missing content-type"
 		grpcMessage    string
 		statusGen      *status.Status
 
@@ -1283,6 +1283,7 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 				contentTypeErr = fmt.Sprintf("transport: received the unexpected content-type %q", hf.Value)
 				break
 			}
+			contentTypeErr = ""
 			mdata[hf.Name] = append(mdata[hf.Name], hf.Value)
 			isGRPC = true
 		case "grpc-encoding":
@@ -1313,10 +1314,10 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 		}
 	}
 
-	if !isGRPC {
+	if !isGRPC || (httpStatus != "200" && !endStream) {
 		var (
 			code           = codes.Internal // when header does not include HTTP status, return INTERNAL
-			httpStatusCode int
+			httpStatusCode *int
 		)
 
 		if httpStatus != "" {
@@ -1326,17 +1327,18 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
 				t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 				return
 			}
-			httpStatusCode = int(c)
+			statusCode := int(c)
+			httpStatusCode = &statusCode
 
 			var ok bool
-			code, ok = HTTPStatusConvTab[httpStatusCode]
+			code, ok = HTTPStatusConvTab[statusCode]
 			if !ok {
 				code = codes.Unknown
 			}
 		}
 
 		// Verify the HTTP response is a 200.
-		se := status.New(code, constructHTTPErrMsg(&httpStatusCode, contentTypeErr))
+		se := status.New(code, constructHTTPErrMsg(httpStatusCode, contentTypeErr))
 		t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
 		return
 	}

internal/transport/http_util.go

@@ -177,16 +177,17 @@ func decodeGRPCStatusDetails(rawDetails string) (*status.Status, error) {
 // Format: HTTP status code and its corresponding message + content-type error message.
 func constructHTTPErrMsg(httpStatus *int, contentTypeErr string) string {
 	var errMsgs []string
-
 	if httpStatus == nil {
 		errMsgs = append(errMsgs, "malformed header: missing HTTP status")
 	} else {
-		errMsgs = append(errMsgs, fmt.Sprintf("%s: HTTP status code %d", http.StatusText(*(httpStatus)), *httpStatus))
+		errMsgs = append(errMsgs, fmt.Sprintf(
+			"unexpected HTTP status code received from server: %d (%s)",
+			*httpStatus,
+			http.StatusText(*(httpStatus)),
+		))
 	}
 
-	if contentTypeErr == "" {
-		errMsgs = append(errMsgs, "transport: missing content-type field")
-	} else {
+	if contentTypeErr != "" {
 		errMsgs = append(errMsgs, contentTypeErr)
 	}
 

internal/transport/transport_test.go

@@ -1991,17 +1991,33 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
 					{Name: "grpc-status", Value: "0"},
+					{Name: ":status", Value: "200"},
 				},
 			},
 			// no error
 			wantStatus: status.New(codes.OK, ""),
 		},
+		{
+			name: "missing content-type header",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "grpc-status", Value: "0"},
+					{Name: ":status", Value: "200"},
+				},
+			},
+			// no error
+			wantStatus: status.New(
+				codes.Unknown,
+				"unexpected HTTP status code received from server: 200 (OK); missing content-type",
+			),
+		},
 		{
 			name: "invalid grpc status header field",
 			metaHeaderFrame: &http2.MetaHeadersFrame{
 				Fields: []hpack.HeaderField{
 					{Name: "content-type", Value: "application/grpc"},
 					{Name: "grpc-status", Value: "xxxx"},
+					{Name: ":status", Value: "200"},
 				},
 			},
 			wantStatus: status.New(
@@ -2018,7 +2034,7 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 			},
 			wantStatus: status.New(
 				codes.Internal,
-				": HTTP status code 0; transport: received the unexpected content-type \"application/json\"",
+				"malformed header: missing HTTP status; transport: received the unexpected content-type \"application/json\"",
 			),
 		},
 		{
@@ -2045,6 +2061,25 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 				"peer header list size exceeded limit",
 			),
 		},
+		{
+			name: "bad status in grpc mode",
+			metaHeaderFrame: &http2.MetaHeadersFrame{
+				Fields: []hpack.HeaderField{
+					{Name: "content-type", Value: "application/grpc"},
+					{Name: "grpc-status", Value: "0"},
+					{Name: ":status", Value: "504"},
+				},
+				HeadersFrame: &http2.HeadersFrame{
+					FrameHeader: http2.FrameHeader{
+						StreamID: 0,
+					},
+				},
+			},
+			wantStatus: status.New(
+				codes.Unavailable,
+				"unexpected HTTP status code received from server: 504 (Gateway Timeout)",
+			),
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			ts := &Stream{
@@ -2066,11 +2101,13 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
 					list: &itemList{},
 				},
 			}
-			test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
-				FrameHeader: http2.FrameHeader{
-					StreamID: 0,
-					Flags:    http2.FlagHeadersEndStream,
-				},
+			if test.metaHeaderFrame.HeadersFrame == nil {
+				test.metaHeaderFrame.HeadersFrame = &http2.HeadersFrame{
+					FrameHeader: http2.FrameHeader{
+						StreamID: 0,
+						Flags:    http2.FlagHeadersEndStream,
+					},
+				}
 			}
 
 			s.operateHeaders(test.metaHeaderFrame)

test/end2end_test.go

@@ -7289,7 +7289,7 @@ func (s) TestHTTPHeaderFrameErrorHandlingInitialHeader(t *testing.T) {
 	}
 }
 
-// Testing non-Trailers-only Trailers (delievered in second HEADERS frame)
+// Testing non-Trailers-only Trailers (delivered in second HEADERS frame)
 func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
 	for _, test := range []struct {
 		responseHeader []string
@@ -7317,6 +7317,18 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
 				"grpc-status", "0",
 				"grpc-status-details-bin", "????",
 			},
+			errCode: codes.Unimplemented,
+		},
+		{
+			responseHeader: []string{
+				":status", "200",
+				"content-type", "application/grpc",
+			},
+			trailer: []string{
+				// malformed grpc-status-details-bin field
+				"grpc-status", "0",
+				"grpc-status-details-bin", "????",
+			},
 			errCode: codes.Internal,
 		},
 	} {