Fix cascaded routes leaking routing args into the next matched route - #2824
Merged
Conversation
process_route seeded env['grape.routing_args'] once (||=) and merged each
attempt's path captures in place. When a matched route cascaded
(X-Cascade pass), the next candidate tried by rotation/transaction still
saw the previous attempt's :route_info (so the `route` helper returned
the wrong route) and its path captures leaked into params.
Build a fresh { route_info: }.merge(route_params) per attempt instead;
the router is the only writer of this env key.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ericproulx
force-pushed
the
fix-router-cascade-routing-args
branch
from
July 26, 2026 19:31
a55b683 to
658e0d7
Compare
Danger ReportNo issues found. |
ericproulx
added a commit
that referenced
this pull request
Jul 27, 2026
#2824 made process_route build a fresh routing-args Hash per attempt, so a candidate tried after a cascading match (X-Cascade pass) could not observe the previous attempt's :route_info or path captures. #2829 was squash-merged from a branch cut before #2824 landed, so its squashed diff replayed the stale router.rb over the fix and dropped the regression spec along with it. Git reported no conflict, and #2824's CHANGELOG entry survived, so the tree claims a fix it no longer has. Symptoms are back on master: with a cascading v2 and a serving v1, the `route` helper returns v2's route (origin "/:id" instead of "/:name") and v2's capture leaks into params as {"id"=>"123", "name"=>"123"} -- an endpoint sees a param it never declared. Restore the fix and its spec verbatim. The router is the only writer of env['grape.routing_args'], so the unconditional assignment is safe. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Problem
Grape::Router#process_routeseeds the routing args once and then merges every subsequent attempt's captures into the same Hash:When a matched route responds with
X-Cascade: pass(the canonical case: header/accept-version versioning withcascade: true),#rotation(and the'*'-method fallback in#transaction) tries the next candidate route — butprocess_routeruns again on an env whose routing args were already populated by the failed attempt. Because of the||=, two things go wrong for the route that ultimately serves the request::route_infostill points at the first (cascaded) route. Theroutehelper inside the endpoint returns the wrong route, andErrorFormatter::Base#presentresolveshttp_codesentity presenters from the wrong route's metadata.params. Any capture name the winning route doesn't declare survives the merge and shows up in the endpoint'sparams.Reproduction
GET /123withAccept: application/vnd.grape-v1+json— the v2 route matches first, its versioner cascades, and the v1 endpoint then responds with:{"origin":"/:id","params":{"id":"123","name":"123"}}route.originbelongs to the v2 route it never ran, andparams[:id]is a capture the v1 route never declared. Expected (and after this fix):{"origin":"/:name","params":{"name":"123"}}Fix
Build a fresh
{ route_info: route }.merge(route_params)on everyprocess_routecall. The router is the only writer ofenv['grape.routing_args'](readers:Grape::Request#make_params, theroutehelper, andErrorFormatter::Base), so resetting per attempt is safe — including for the greedy OPTIONS/405 helper routes, whoseparams_foryields no captures and which previously ran with an unset or unrelated args Hash anyway.This behavior predates the recent router refactors — the
||=/seed-once shape goes back through #2754, #2689 and #2636 to the originalmake_routing_args(default_args || { route_info: route }).🤖 Generated with Claude Code