envoyproxy / envoyproxy/envoy

grpc_http1_bridge sets HTTP status incorrectly for Trailers-Only responses

Open
#14,872 7 comments 0 reactions 2 assignees Claimed by @samrabelachew View on GitHub
area/grpc bug help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
428

Description

*Title*: grpc_http1_bridge sets HTTP status incorrectly for Trailers-Only responses

*Description*:

gRPC responses may optimize and put would-be-trailers in the headers frame if there are no data frames. The gRPC spec refers to this a a "Trailers-Only" response.

That is, the following two gRPC responses should be considered equivalent:

```
# normal
HEADERS
- END_STREAM
+ END_HEADERS
:status: 200
content-type: application/grpc
trailer: Grpc-Status
trailer: Grpc-Message
trailer: Grpc-Status-Details-Bin

HEADERS
+ END_STREAM
+ END_HEADERS
grpc-message: requested-error
grpc-status: 1
```
```
# Trailers-Only
HEADERS
+ END_STREAM
+ END_HEADERS
:status: 200
content-type: application/grpc
grpc-status: 1
grpc-message: requested-error
```

However, the `grpc_http1_bridge` does not treat them as equivalent. The bridge attempts to set the HTTP status code based on the `grpc-status` code; however, when doing so it it only looks for `grpc-status` in trailer frames; so it doesn't find it for a Trailers-Only response. And so, for the "normal" response, the bridge correctly sets the downstream HTTP status to 503, while for the "Trailers-Only" response, it erroneously sets the downstream HTTP status to 200.

*Repro steps*:

Put the `grpc_http1_bridge` in front of a gRPC server that performs the Trailers-Only optimization (in Go, `google.golang.org/grpc.Server.Serve` does this optimization, but `google.golang.org/grpc.Server.ServeHTTP` does not). Arrange for that server to respond with a non-zero `grpc-status`. Send an HTTP/1 request through the bridge. Observe that the HTTP status code is 200.

*Notes*:

The bug is plainly visible in `source/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter.cc`. For reference the `grpc_json_transcoder` has a reasonably elegant solution to this problem.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.