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

Interceptor doesn't preserve HTTP method, causing CORS failures #911

Closed
jdahlq opened this issue Feb 12, 2022 · 3 comments
Closed

Interceptor doesn't preserve HTTP method, causing CORS failures #911

jdahlq opened this issue Feb 12, 2022 · 3 comments

Comments

@jdahlq
Copy link
Contributor

jdahlq commented Feb 12, 2022

Before the request is sent to an interceptor, the HTTP request is converted into a Tonic request and its body is removed. After the interceptor has transformed the Tonic request, the body is added back in, and it is converted back into an HTTP request. However, this last conversion assumes that the request will always be an HTTP2 POST.

Tonic-web handles preflight CORS requests (which have the HTTP OPTIONS method), but if there is an interceptor before the Tonic service, the request is changed from OPTIONS to POST before tonic-web sees it, and an incorrect response is sent, causing a CORS failure in the browser.

@LucioFranco
Copy link
Member

I will have to dig in but shouldn't tonic-web be handling this aspect?

@jdahlq
Copy link
Contributor Author

jdahlq commented Feb 16, 2022

Tonic-web does handle it, but only when the grpc service itself is called. Consider the following server definition.

let router = Server::builder()
    .layer(interceptor(my_interceptor_fn))
    .add_service(tonic_web::enable(my_svc));

tonic_web::enable wraps my_svc with a GrpcWeb service which handles CORS before calling my_svc. GrpcWeb relies on the headers, specifically the HTTP Method, to identify preflight CORS requests vs normal gRPC POST requests. So, the problem is that interceptor changes the HTTP method to POST (from OPTIONS, the one used by preflight CORS) before tonic-web even sees it.

An alternative to modifying interceptor to preserve HTTP method is to refactor Server and tonic-web to handle CORS before all layers, but I don't think that makes much sense. It would be a big refactor, and you wouldn't be able to apply different tonic-web settings for different services.

The input and output of interceptor is an HTTP request, not a gRPC request, so I think it makes sense to preserve all the HTTP attributes of the input request there.

@LucioFranco
Copy link
Member

I believe with #912 we can close this.

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

No branches or pull requests

2 participants