Skip to content

Commit 7087664

Browse files
committed
addressing review comments for #8486
1 parent b4cce55 commit 7087664

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

internal/transport/http2_client.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,8 +1512,8 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
15121512
}
15131513
}
15141514

1515-
//if not a gRPC response - evaluate entire http status and process close stream / response
1516-
//for 200 -> Unknown, else decode the error
1515+
// If a non gRPC response is received, then evaluate entire http status and process close stream / response.
1516+
// In case http status doesn't provide any error information (status : 200), evalute response code to be Unknown.
15171517
if !isGRPC {
15181518
var grpcErrorCode = codes.Internal // when header does not include HTTP status, return INTERNAL
15191519
var errs []string
@@ -1533,8 +1533,12 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
15331533
}
15341534
statusCode := int(c)
15351535
if statusCode >= 100 && statusCode < 200 {
1536+
if endStream {
1537+
se := status.New(codes.Internal, fmt.Sprintf(
1538+
"protocol error: informational header with status code %d must not have END_STREAM set", statusCode))
1539+
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
1540+
}
15361541
//In case of informational headers return
1537-
//For trailers, since we are already in gRPC mode, we will ignore all http statuses and not enter this block
15381542
return
15391543
}
15401544
httpStatusErr = fmt.Sprintf(
@@ -1557,7 +1561,7 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
15571561
if contentTypeErr != "" {
15581562
errs = append(errs, contentTypeErr)
15591563
}
1560-
// Verify the HTTP response is a 200.
1564+
15611565
se := status.New(grpcErrorCode, strings.Join(errs, "; "))
15621566
t.closeStream(s, se.Err(), true, http2.ErrCodeProtocol, se, nil, endStream)
15631567
return

internal/transport/transport_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2807,12 +2807,8 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
28072807
}
28082808

28092809
s.operateHeaders(test.metaHeaderFrame)
2810-
28112810
got := ts.status
2812-
want := test.wantStatus
2813-
if test.wantStatusEndStream != nil {
2814-
want = test.wantStatusEndStream
2815-
}
2811+
want := test.wantStatusEndStream
28162812
if got.Code() != want.Code() || got.Message() != want.Message() {
28172813
t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
28182814
}
@@ -3217,14 +3213,17 @@ func (s) TestClientTransport_Handle1xxHeaders(t *testing.T) {
32173213
wantStatus *status.Status
32183214
}{
32193215
{
3220-
name: "1xx with END_STREAM will be ignored",
3216+
name: "1xx with END_STREAM is error",
32213217
metaHeaderFrame: &http2.MetaHeadersFrame{
32223218
Fields: []hpack.HeaderField{
32233219
{Name: ":status", Value: "100"},
32243220
},
32253221
},
3226-
httpFlags: http2.FlagHeadersEndStream,
3227-
wantStatus: nil,
3222+
httpFlags: http2.FlagHeadersEndStream,
3223+
wantStatus: status.New(
3224+
codes.Internal,
3225+
"protocol error: informational header with status code 100 must not have END_STREAM set",
3226+
),
32283227
},
32293228
{
32303229
name: "1xx without END_STREAM is ignored",

0 commit comments

Comments
 (0)