trailers are not being passed correctly in the response

[rauanmayemir]

Trailers are being broken in the response when grpc is served behind vanguard:

I'm assuming that under the hood vanguard essentially wraps grpcServer.ServeHttp which leads to the same outcome when used as is without vanguard. There is a similar issue that was never really solved: grpc/grpc-go#2206.

[emcfarlane]

@rauanmayemir thanks for raising the issue. I'm going to add the conformance test suite to improve coverage which will hopefully highlight this issue and allow us to investigate solutions. Issues grpc/grpc-go#2206 and grpc/grpc-go#553 look like the cause so ideally we could upstream a fix if we are unable to workaround it.

[rauanmayemir]

Wondering if there's any chance to have this fixed soon.

I've been holding off on adopting vanguard due to this issue breaking my frontend that depends on gRPC-Web. To understand response state we need to parse grpc-status and grpc-status-details-bin headers which are not available especially when it's a non-OK response.

(I could take a stab and try fixing this, but I don't know where to start)

[jhump]

@rauanmayemir, I started looking into this and am trying to figure out what exactly the issue is. The screenshot above and the linked issue don't really describe an error. Is it that application-set trailers are missing from the response? Or are the actual status and details failing to come across (like an RPC that should fail is being incorrectly considered a success)?

The linked issue just describes that the handler never emits trailers-only responses. But such responses are not necessarily expected/required by the spec -- separating them into headers and trailers is acceptable (and, in fact, they must be separated if the application has set any custom headers since metadata in a trailers-only response are interpreted as trailers).

I'll keep playing with this to see if I can reproduce some sort of problem, but I think I'm uncertain from the above on exactly what the expected behavior is and what's going wrong.

[rauanmayemir]

Thank you, I’ll try to reproduce and find the issue as well. The issue here is that headers are being garbled like in the screenshot (it’s from grpcui, I’ll try to find out what’s actually happening).

[jhump]

@rauanmayemir, the screenshot does not seem to show garbled headers. What is it you are expecting them to look like? Did the application attempt to send custom header or trailer metadata that is missing?

[rauanmayemir]

@jhump It's the obvious part here:

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

What I should have seen is:

Grpc-Status: 13
Grpc-Message: Error message
Grpc-Message-Details-Bin: base64_error_details_payload

Also notice Accept-Encoding.

[jhump]

What I should have seen is:

@rauanmayemir, the Grpc-Status etc will be trailers, not headers.

So is the issue that the grpcui reported the RPC as successful, instead of failing with an "internal" error code?

Also notice Accept-Encoding.

That looks fine. Are the Vary headers posing an issue for you?

[rauanmayemir]

If I remember correctly, it will interpret rpc as failed.

I think I'm confusing grpcui's visual presentation with grpc-web/grpc-json transcoding of headers and trailers into http headers. But it still bothers me that some requests correctly show my trailers and headers separately (i.e: headers, body, trailers), but for unsuccessful requests with ServeHttp trailers and headers turn into a mess.

Let me get back to you after I try grpc-web/grpc-json requests instead of grpcui.

[jhump]

but for unsuccessful requests with ServeHttp trailers and headers turn into a mess.

Do you mind including a more full screenshot to demonstrate what you mean? The one above seems to be clipped and only shows the headers. There should be more, like an error and optional trailers, below that. This could also potentially be a bug/quirk with how grpcui renders these responses.

[rauanmayemir]

That’s what I meant, there is only what is displayed in the screenshot. Since this is an unsuccessful response, there is no body. I have no idea what is headers and what is trailers, all of it is in there together.

This could also potentially be a bug/quirk with how grpcui renders these responses.

Very possible, which is why I’m coming up with a repro so the metadata can be textual.

…

On 7 Jan 2025 at 22:32 +0500, Joshua Humphries ***@***.***>, wrote: > but for unsuccessful requests with ServeHttp trailers and headers turn into a mess. Do you mind including a more full screenshot to demonstrate what you mean? The one above seems to be clipped and only shows the headers. There should be more, like an error and optional trailers, below that. This could also potentially be a bug/quirk with how grpcui renders these responses. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[rauanmayemir]

There is indeed an issue with the trailers, they are being included to response headers in a weird way.

Here's an output from grpcui:

And here's a variant with grpcurl:

grpcurl -d '{}' -plaintext -vv 127.0.0.1:35071 my.package.ServiceName/MethodName

Resolved method descriptor: [skipped]

Response headers received:
content-type: application/grpc
trailer: Grpc-Status
trailer: Grpc-Message
trailer: Grpc-Status-Details-Bin

Response trailers received:
(empty)
Sent 1 request and received 0 responses
ERROR:
  Code: FailedPrecondition
  Message: Request validation error
  Details:
  1)	{"@error":"google.rpc.ErrorInfo is not recognized; see @value for raw binary message data","@type":"type.googleapis.com/google.rpc.ErrorInfo","@value":"payload"}
  2)	{"@error":"google.rpc.BadRequest is not recognized; see @value for raw binary message data","@type":"type.googleapis.com/google.rpc.BadRequest","@value":"payload"}

@jhump Btw, it seems like vanguard is actually the one adding those headers here:

vanguard-go/protocol_grpc.go

Line 69 in 9c8fc53

headers.Add("Trailer", "Grpc-Status")

It is unclear to me why is it done.

[rauanmayemir]

Tried for `grpc-web:

curl -vv -X POST "http://127.0.0.1:35071/my.package.ServiceName/MethodName" -H 'x-grpc-web: 1' -H 'Content-Type: application/grpc-web+proto' -H 'Accept: application/grpc-web+proto' --data-binary @req.bin --output -
Note: Unnecessary use of -X or --request, POST is already inferred.
*   Trying 127.0.0.1:35071...
* Connected to 127.0.0.1 (127.0.0.1) port 35071
> POST /my.package.ServiceName/MethodName HTTP/1.1
> Host: 127.0.0.1:35071
> User-Agent: curl/8.7.1
> x-grpc-web: 1
> Content-Type: application/grpc-web+proto
> Accept: application/grpc-web+proto
> Content-Length: 11
> 
* upload completely sent off: 11 bytes
< HTTP/1.1 200 OK
< Content-Type: application/grpc-web+proto
< Transfer-Encoding: chunked
< 
p
%%%PAYLOAD%%% Grpc-Message: 
Grpc-Status: 0
* Connection #0 to host 127.0.0.1 left intact

I'm guessing p is misinterpreted 4 bytes that contain response data length.

As you can see here, Grpc-Message and Grpc-Status trailers aren't 'hoisted' as a response header. They should have been, envoy converts them to response headers.

[jhump]

@rauanmayemir, this all looks fine.

The first example isn't "hoisting" anything into headers. It is simply pre-declaring some of the trailer fields. This is an implementation detail of Go's "net/http" package: in order to make sure the package will send trailers separately from headers when there is no response body, one must predeclare a trailer.

That "trailer" response header is part of the HTTP standard: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Trailer

You will not see this with the grpc-web protocol because grpc-web does not actually use trailers: the trailer metadata and status are included in a special message at the end of the response body (because browsers APIs generally don't support trailers, so the only way for grpc-web browser JavaScript client code to observe the status is to put it in the body).

The reason you may not see this with other gRPC server implementations is that they don't use the "net/http" library, so don't have the same caveat imposed to make sure trailers are correctly sent. In some cases, the "net/http"-based implementation could send what's called a "trailers-only" response -- in this case, the response headers contain any trailing metadata and the status keys (designed to make the response more compact over HTTP/2 when there is no response body data and the application isn't trying to send custom headers separately from custom trailers). I have a branch where I was working on adding that to grpc-go, but we would likely need more work in vanguard-go, too.

Vanguard uses "net/http", and then requires the use of grpc.Server.ServeHTTP, which is an adapter for *grpc.Server instances to work with the "net/http" package, and that's why you see this behavior with Vanguard, but not when using the grpc protocol directly to the grpc.Server if using its normal Serve method.

@jhump Btw, it seems like vanguard is actually the one adding those headers here

Yes, Vanguard will do that sometimes. There is more work to do in vanguard-go to have it support trailers-only responses. But this is also done in the grpc-go server when using its ServerHTTP method:
https://github.com/grpc/grpc-go/blob/v1.69.2/internal/transport/handler_server.go#L304-L311

[rauanmayemir]

the trailer metadata and status are included in a special message at the end of the response body

Oh, you mean I was supposed to parse trailers from the response body in the first place and not rely on http headers.

I checked the protocol and you are right. I thought it was a bug in grpc-go when in fact it's a 'bug' in envoy: for trailer-only responses it hoists grpc-message, grpc-status, grpc-status-details-bin to headers and just returns empty response instead.

Should vanguard follow the protocol then? It shouldn't be very hard to parse trailers from the body and as a bonus, it will support larger values.

[rauanmayemir]

OTOH, browser features section for CORS mentions grpc-status and grpc-message headers explicitly.

[rauanmayemir]

I'm getting really confused. In case of non-OK responses vanguard DOES NOT fail, it will indeed add grpc-* headers and return empty body. (I can't remember why I couldn't reproduce this before)

The only instance where it differs from envoy is when grpc-status is 0. Envoy will still add it, but vanguard won't.

Should we close this issue? 😄