Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use response headers x-grpc-request-type and x-grpc-gateway-body to etermine when to close the request writer #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paskozdilar
Copy link

This is a preliminary PR suggesting a method of detecting which type of method (Unary, ServerStreaming, ClientStreaming, DuplexStreaming) grpc-gateway is serving, and whether or not it expects a body.


The grpc-gateway implementation was very liberal with the data it accepted.
In case of Unary and ClientStreaming messages:

  • it only read the first JSON object if body annotation is defined
  • it read nothing at all if body annotation is not defined

This caused a context leak when clients sent more data than was expected by the gateway.
Then grpc-gateway added a blocking connection drain for Unary method with no body annotation defined, which broke grpc-websocket-proxy - since it never closed the req.Body, the method never triggered at all.

There is work underway to do implement stricter request checking for Unary and ClientStreaming messages in general, which would break grpc-websocket-proxy as-is.
A possible solution would be to add the following metadata in response headers:

  • x-grpc-method-type (oneof: Unary, ServerStreaming, ClientStreaming, DuplexStreaming)
  • x-grpc-gateway-body (oneof: true, false)

Then, grpc-websocket-proxy could do the following:

  • on Unary and ServerStreaming methods:
    • if body is expected, wait for first message from websocket, then close the request body
    • if body is not expected, close the request body without sending anything
  • on Unary and ClientStreaming methods:
    • as soon as response is received, close the websocket

Before this is merged, this PR needs to be merged, and grpc-gateway version updated in go.mod:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant