grpc_json_transcoder and http/1.1 doesn't properly handle trailers

[joshuarubin]

When a client connects to envoy using http/1.1 and the endpoint uses the grpc_json_transcoder, nothing is done with the response trailer headers. This is a violation of the http/1.1 spec so long as the transfer encoding isn't chunked (which it isn't in this case).

I've tried stacking the grpc_json_transcoder with the grpc_http1_bridge (both before and after the transcoder) but that doesn't seem to work either.

This is a significant issue with clients that use the Go http client as it returns an error (and the body is impossible to reach) if it sees the trailer headers over http/1.1.

[lizan]

Can you elaborate a bit more with examples? grpc_json_transcoder won't send anything for response trailers if your client connect to Envoy with HTTP/1.1.

You shouldn't be stacking the grpc_json_transcoder with the grpc_http1_bridge, those won't work together.

[joshuarubin]

Here is what I get with curl:

$ curl \
    --http1.1 \
    -v \
    -H'Content-Type: application/json' \
    -H'Authorization: Bearer <OAUTH2_TOKEN>' \
    --data '<JSON_DATA>' \
    https://api.integration.us-west-2.titan.zvelo.io/v1/query
*   Trying 35.155.178.206...
* TCP_NODELAY set
* Connected to api.integration.us-west-2.titan.zvelo.io (35.155.178.206) port 443 (#0)
* ALPN, offering http/1.1
* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Client hello (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES128-GCM-SHA256
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: CN=api.integration.us-west-2.titan.zvelo.io
*  start date: Jul 24 18:22:40 2018 GMT
*  expire date: Oct 22 18:22:40 2018 GMT
*  subjectAltName: host "api.integration.us-west-2.titan.zvelo.io" matched cert's "api.integration.us-west-2.titan.zvelo.io"
*  issuer: C=US; O=Let's Encrypt; CN=Let's Encrypt Authority X3
*  SSL certificate verify ok.
> POST /v1/query HTTP/1.1
> Host: api.integration.us-west-2.titan.zvelo.io
> User-Agent: curl/7.54.0
> Accept: */*
> Content-Type: application/json
> Authorization: Bearer <OAUTH2_TOKEN>
> Content-Length: 73
> 
* upload completely sent off: 73 out of 73 bytes
< HTTP/1.1 200 OK
< content-type: application/json
< trailer: Grpc-Status
< trailer: Grpc-Message
< trailer: Grpc-Status-Details-Bin
< x-envoy-upstream-service-time: 3
< strict-transport-security: max-age=31536000
< grpc-status: 0
< content-length: 55
< date: Mon, 27 Aug 2018 03:27:00 GMT
< server: zveloAPI
< 
* Connection #0 to host api.integration.us-west-2.titan.zvelo.io left intact
{"reply":[{"requestId":"19MWP4UDhLVAIwBauGSHUT0EeAU"}]}

My envoy http filters are set up like this:

                    http_filters:
                      - name: envoy.cors
                      - name: envoy.gzip
                      - name: envoy.grpc_json_transcoder
                        config:
                          proto_descriptor: /config/msg.protoset
                          services:
                            - grpc.health.v1.Health
                            - zvelo.msg.APIv1
                      - name: envoy.health_check
                        config:
                          headers:
                            name: :path
                            exact_match: /health
                          pass_through_mode: false
                          cluster_min_healthy_percentages:
                            zveloapi:
                              value: 100
                      - name: envoy.grpc_web
                      - name: envoy.router

[lizan]

@joshuarubin thanks for the example, the curl output matches my expectation. What does your go http client is complaining about? the trailer header?

[lizan]

< trailer: Grpc-Status
< trailer: Grpc-Message
< trailer: Grpc-Status-Details-Bin

Those should be sent by upstream gRPC server, but I think we can trim them in JSON transcoder filter.

[joshuarubin]

This is the result of a Go client we use:

> POST /v1/query HTTP/1.1
> Host: api.integration.us-west-2.titan.zvelo.io
> User-Agent: go-zapi v1
> Content-Length: 70
> Authorization: Bearer <OAUTH2_TOKEN>
> Content-Type: application/json
> Accept-Encoding: gzip
> 
> {"url":["https://www.example.com/foo12"],"dataset":["CATEGORIZATION"]}
query error: Post https://api.integration.us-west-2.titan.zvelo.io/v1/query: net/http: HTTP/1.x transport connection broken: trailer header without chunked transfer encoding

There is no way to catch or bypass this error in the Go client. Fortunately, we can fix the issue in this case by using the http/2 client. Still it is an issue that needs to be addressed.

[joshuarubin]

Anything I can do to speed this along? I'm happy to take a stab at a PR. This issue isn't affecting us in production yet, but very soon will be.

[lizan]

@joshuarubin If you would like to write a PR I'm happy to review. It would be simple to add some code here to do:
https://github.com/envoyproxy/envoy/blob/master/source/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter.cc#L401

1. check if downstream is HTTP1.1 callbacks_->requestInfo().protocol()
2. remove "trailer" header from response_headers_

Also add tests accordingly.

[joshuarubin]

the code itself is pretty simple, if I've done it properly (at the location you mentioned above):

  if (encoder_callbacks_->requestInfo().protocol() < Http::Protocol::Http2) {
    response_headers_->remove(Http::LowerCaseString("trailer"));
  }

However, I don't have a clue how to go about testing this. I'm able to run the tests with bazel, but don't know enough about the framework to be much help yet. Any advice?

[lizan]

Add trailer to response header here and test if it is removed? https://github.com/envoyproxy/envoy/blob/master/test/extensions/filters/http/grpc_json_transcoder/json_transcoder_filter_test.cc#L314