Skip to content

Commit 50b5337

Browse files
mbissambissa
committed
ignoring http status when content type is application/grpc grpc#8486
Fixes grpc#8486 When a gRPC response is received with content type application/grpc, we then do not expect any information in the http status and the status information needs to be conveyed by gRPC status only. In case of missing gRPC status, we will throw an Internal error instead of Unknown in accordance with https://grpc.io/docs/guides/status-codes/ Changes : - Ignore http status in case of content type application/grpc - Change the default rawStatusCode to return Internal for missing grpc status RELEASE NOTES: * client : Ignore http status for gRPC mode and return Internal error for missing grpc status
1 parent 3074bcd commit 50b5337

3 files changed

Lines changed: 51 additions & 28 deletions

File tree

‎internal/transport/http2_client.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,15 +1467,14 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
14671467
recvCompress string
14681468
httpStatusCode *int
14691469
httpStatusErr string
1470-
rawStatusCode = codes.Unknown
1470+
rawStatusCode = codes.Internal
14711471
// headerError is set if an error is encountered while parsing the headers
14721472
headerError string
14731473
)
14741474

14751475
if initialHeader {
14761476
httpStatusErr = "malformed header: missing HTTP status"
14771477
}
1478-
14791478
for _, hf := range frame.Fields {
14801479
switch hf.Name {
14811480
case "content-type":
@@ -1534,7 +1533,8 @@ func (t *http2Client) operateHeaders(frame *http2.MetaHeadersFrame) {
15341533
}
15351534
}
15361535

1537-
if !isGRPC || httpStatusErr != "" {
1536+
//if response is not grpc or endstream doesn't receive a grpc status, fall back to http error code
1537+
if !isGRPC {
15381538
var code = codes.Internal // when header does not include HTTP status, return INTERNAL
15391539

15401540
if httpStatusCode != nil {

‎internal/transport/transport_test.go‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2618,6 +2618,8 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
26182618
metaHeaderFrame *http2.MetaHeadersFrame
26192619
// output
26202620
wantStatus *status.Status
2621+
// end stream output
2622+
wantStatusEndStream *status.Status
26212623
}{
26222624
{
26232625
name: "valid header",
@@ -2695,34 +2697,55 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
26952697
),
26962698
},
26972699
{
2698-
name: "bad status in grpc mode",
2700+
name: "ignoring bad http status in grpc mode",
26992701
metaHeaderFrame: &http2.MetaHeadersFrame{
27002702
Fields: []hpack.HeaderField{
27012703
{Name: "content-type", Value: "application/grpc"},
27022704
{Name: "grpc-status", Value: "0"},
27032705
{Name: ":status", Value: "504"},
27042706
},
27052707
},
2706-
wantStatus: status.New(
2707-
codes.Unavailable,
2708-
"unexpected HTTP status code received from server: 504 (Gateway Timeout)",
2709-
),
2708+
wantStatus: status.New(codes.OK, ""),
27102709
},
27112710
{
2712-
name: "missing http status",
2711+
name: "missing http status and grpc status",
27132712
metaHeaderFrame: &http2.MetaHeadersFrame{
27142713
Fields: []hpack.HeaderField{
27152714
{Name: "content-type", Value: "application/grpc"},
27162715
},
27172716
},
2718-
wantStatus: status.New(
2719-
codes.Internal,
2720-
"malformed header: missing HTTP status",
2721-
),
2717+
wantStatus: status.New(codes.OK, ""),
2718+
wantStatusEndStream: status.New(codes.Internal, ""),
2719+
},
2720+
{
2721+
name: "ignore http status and fail for grpc status missing in trailer",
2722+
metaHeaderFrame: &http2.MetaHeadersFrame{
2723+
Fields: []hpack.HeaderField{
2724+
{Name: "content-type", Value: "application/grpc"},
2725+
{Name: ":status", Value: "504"},
2726+
},
2727+
},
2728+
wantStatus: status.New(codes.OK, ""),
2729+
wantStatusEndStream: status.New(codes.Internal, ""),
2730+
},
2731+
{
2732+
name: "trailer only grpc timeout ignores http status",
2733+
metaHeaderFrame: &http2.MetaHeadersFrame{
2734+
Fields: []hpack.HeaderField{
2735+
{Name: "content-type", Value: "application/grpc"},
2736+
{Name: "grpc-status", Value: "4"},
2737+
{Name: "grpc-message", Value: "Request timed out: Internal error"},
2738+
{Name: ":status", Value: "200"},
2739+
},
2740+
},
2741+
wantStatusEndStream: status.New(codes.DeadlineExceeded, "Request timed out: Internal error"),
27222742
},
27232743
} {
27242744

27252745
t.Run(test.name, func(t *testing.T) {
2746+
if test.wantStatus == nil {
2747+
t.Skip()
2748+
}
27262749
ts := testStream()
27272750
s := testClient(ts)
27282751

@@ -2733,7 +2756,6 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
27332756
}
27342757

27352758
s.operateHeaders(test.metaHeaderFrame)
2736-
27372759
got := ts.status
27382760
want := test.wantStatus
27392761
if got.Code() != want.Code() || got.Message() != want.Message() {
@@ -2755,6 +2777,9 @@ func (s) TestClientDecodeHeaderStatusErr(t *testing.T) {
27552777

27562778
got := ts.status
27572779
want := test.wantStatus
2780+
if test.wantStatusEndStream != nil {
2781+
want = test.wantStatusEndStream
2782+
}
27582783
if got.Code() != want.Code() || got.Message() != want.Message() {
27592784
t.Fatalf("operateHeaders(%v); status = \ngot: %s\nwant: %s", test.metaHeaderFrame, got, want)
27602785
}

‎test/http_header_end2end_test.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,9 @@ func (s) TestHTTPHeaderFrameErrorHandlingInitialHeader(t *testing.T) {
103103
errCode codes.Code
104104
}{
105105
{
106-
name: "missing gRPC status",
106+
name: "missing gRPC content type",
107107
header: []string{
108108
":status", "403",
109-
"content-type", "application/grpc",
110109
},
111110
errCode: codes.PermissionDenied,
112111
},
@@ -120,23 +119,23 @@ func (s) TestHTTPHeaderFrameErrorHandlingInitialHeader(t *testing.T) {
120119
errCode: codes.Internal,
121120
},
122121
{
123-
name: "Malformed grpc-tags-bin field",
122+
name: "Malformed grpc-tags-bin field ignores http status",
124123
header: []string{
125124
":status", "502",
126125
"content-type", "application/grpc",
127126
"grpc-status", "0",
128127
"grpc-tags-bin", "???",
129128
},
130-
errCode: codes.Unavailable,
129+
errCode: codes.Internal,
131130
},
132131
{
133-
name: "gRPC status error",
132+
name: "gRPC status error ignoring http status",
134133
header: []string{
135134
":status", "502",
136135
"content-type", "application/grpc",
137136
"grpc-status", "3",
138137
},
139-
errCode: codes.Unavailable,
138+
errCode: codes.InvalidArgument,
140139
},
141140
} {
142141
t.Run(test.name, func(t *testing.T) {
@@ -161,7 +160,7 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
161160
errCode codes.Code
162161
}{
163162
{
164-
name: "trailer missing grpc-status",
163+
name: "trailer missing grpc-status to ignore http status",
165164
responseHeader: []string{
166165
":status", "200",
167166
"content-type", "application/grpc",
@@ -170,10 +169,10 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
170169
// trailer missing grpc-status
171170
":status", "502",
172171
},
173-
errCode: codes.Unavailable,
172+
errCode: codes.Internal,
174173
},
175174
{
176-
name: "malformed grpc-status-details-bin field with status 404",
175+
name: "malformed grpc-status-details-bin field with status 404 to be ignored due to content type",
177176
responseHeader: []string{
178177
":status", "404",
179178
"content-type", "application/grpc",
@@ -183,20 +182,19 @@ func (s) TestHTTPHeaderFrameErrorHandlingNormalTrailer(t *testing.T) {
183182
"grpc-status", "0",
184183
"grpc-status-details-bin", "????",
185184
},
186-
errCode: codes.Unimplemented,
185+
errCode: codes.Internal,
187186
},
188187
{
189-
name: "malformed grpc-status-details-bin field with status 200",
188+
name: "malformed grpc-status-details-bin field with status 404 and no content type",
190189
responseHeader: []string{
191-
":status", "200",
192-
"content-type", "application/grpc",
190+
":status", "404",
193191
},
194192
trailer: []string{
195193
// malformed grpc-status-details-bin field
196194
"grpc-status", "0",
197195
"grpc-status-details-bin", "????",
198196
},
199-
errCode: codes.Internal,
197+
errCode: codes.Unimplemented,
200198
},
201199
}
202200
for _, test := range tests {

0 commit comments

Comments
 (0)