Grpc-web: Use lower case header keys in in-body trailers as mandated by gRPC-Web spec.

[ktr0731]

I found some concerns when I have tested a gRPC Web server related to trailers.

here are the response trailers.

Content-Type: application/grpc+proto
Grpc-Status: 0

1. Why aren't trailer names using lower-case? The gRPC Web spec says, "use lower-case header/trailer names".
2. Why is Grpc-Status at the end? In order to the ABNF of gRPC response, Status must be next to Length-Prefixed-Message.

ref.
https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-WEB.md
https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md

[johanbrandhorst]

This is a great spot @ktr0731, and at least the first suggestion is an obviously incorrect implementation of the spec. I think this is simply a matter of using the default Go header formatting without thinking about it (https://golang.org/pkg/net/textproto/#CanonicalMIMEHeaderKey).

The second issue is a bit more unclear. Do you mean to suggest that status should come before the content type trailer? The gRPC HTTP2 spec says that Trailers are _after_ the Length-Prefixed-Message. I don't see how this contradicts that?

[ktr0731]

Thank you @johanbrandhorst! I want to fix the first matter if I can afford to do.

About the second issue, the spec says:

Response → (Response-Headers *Length-Prefixed-Message Trailers) / Trailers-Only
Response-Headers → HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata
Trailers → Status [Status-Message] *Custom-Metadata

When a response has some Length-Prefixed-Message, Response is represented as Response → Response-Headers *Length-Prefixed-Message Trailers.
Response-Headers is represented as Response-Headers → HTTP-Status [Message-Encoding] [Message-Accept-Encoding] Content-Type *Custom-Metadata. Therefore, Content-Type is a part of Response-Headers, not Trailers.

[johanbrandhorst]

So, if I understand correctly, you are simply saying that the content type shouldn't be in the in-body trailers? Do you know what the behaviour of the Envoy proxy is in this case?

[ktr0731]

No, I don't. Can you tell me about the behaviour of the Envoy proxy?

[johanbrandhorst]

I haven't had a chance to try it myself but it's the alternative implementation of the gRPC web spec recommended by gRPC/gRPC-Web. Worth investigating.

[johanbrandhorst]

@ktr0731 I've created a repo that should help test this behaviour: https://github.com/johanbrandhorst/grpc-web-compatibility-test. If you're interested in testing this more, maybe you could contribute to the repo?

[johanbrandhorst]

btw @MarcusLongmuir @jonny-improbable I think this issue is important to solve, it is a violation of the spec and I don't think it should take too much effort to fix. It's breaking the grpc/grpc-web client.

[johanbrandhorst]

This is breaking errors returned in server-streaming requests.

[bayesianmind]

This is also breaking us because the client expects lowercase trailer names so error handling doesn't work in grpc-web when used with this proxy.

[ktr0731]

@johanbrandhorst
That's awesome repository, thank you!
Yes, I'm interested in this. I'll take an effort to fix the problem.

[johanbrandhorst]

Should be fixed with #271

[johanbrandhorst]

I'm my testing of 0.6.4 in https://github.com/johanbrandhorst/grpc-web-compatibility-test this is now working as expected :tada:. Huge thanks to @ktr0731 for getting the ball rolling on this!

[johanbrandhorst]

This is _now_ working as expected (curse you fat fingers).