fix(rpc): move CORS layer above AcceptHeaderLayer#1707
Open
WiktorStarczewski wants to merge 3 commits intomainfrom
Open
fix(rpc): move CORS layer above AcceptHeaderLayer#1707WiktorStarczewski wants to merge 3 commits intomainfrom
WiktorStarczewski wants to merge 3 commits intomainfrom
Conversation
When AcceptHeaderLayer rejects a request due to an unsupported SDK version, it short-circuits the response via futures::future::ready() which bypasses all downstream middleware. Since the CORS layer was below AcceptHeaderLayer, rejection responses had no CORS headers, causing browsers to block the error entirely.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reorders the tower middleware stack so that
cors_for_grpc_web_layer()is applied beforeAcceptHeaderLayer.Problem
When a browser-based gRPC-Web client (e.g. the Miden web SDK) connects to the RPC server with an SDK version the server doesn't recognize,
AcceptHeaderLayerrejects the request by returning atonic::Status::invalid_argumentresponse directly viafutures::future::ready(). This short-circuits all downstream middleware — including the CORS layer.Because the CORS layer sat below
AcceptHeaderLayerin the middleware stack, rejection responses were missingAccess-Control-Allow-Originand other CORS headers entirely. The browser then blocked the response at the network level, giving the user an opaquenet::ERR_FAILED/ "CORS policy" error instead of the actual gRPC error message ("server does not support any of the specified application/vnd.miden content types").This made it impossible to diagnose version mismatches from a browser environment — the real error was silently swallowed by the browser's CORS enforcement.
Root Cause
In tower's
.layer()builder, layers listed earlier are outer layers — they see responses last on the way out. The previous ordering was:When
AcceptHeaderLayerrejected a request, the response traveled back out throughHealthCheckLayerandTraceLayerbut never passed throughCorsLayer, since it was an inner layer that the request never reached.Fix
By moving the CORS layer above the accept header check, all responses — including version rejections — are wrapped with proper CORS headers. The browser can then read and surface the actual gRPC error to the application.
Verification
Tested with
curlagainst the two scenarios:Before (no CORS headers on rejection):
After (CORS headers present on rejection):
The same request would include
Access-Control-Allow-Origin,Access-Control-Allow-Credentials, andAccess-Control-Expose-Headers, allowing the browser to read the gRPC error.