Skip to content

Restore the per-attempt routing args in Grape::Router - #2834

Merged
ericproulx merged 1 commit into
masterfrom
restore-routing-args-per-attempt
Jul 27, 2026
Merged

Restore the per-attempt routing args in Grape::Router#2834
ericproulx merged 1 commit into
masterfrom
restore-routing-args-per-attempt

Conversation

@ericproulx

Copy link
Copy Markdown
Contributor

Summary

#2824's fix is no longer in master. This restores it, with its regression spec.

lib/grape/router.rb#process_route is back to seeding env['grape.routing_args'] once with ||= and merging each attempt's captures in place:

env[Grape::Env::GRAPE_ROUTING_ARGS] ||= { route_info: route }
env[Grape::Env::GRAPE_ROUTING_ARGS].merge!(route_params) if route_params.present?

How it was lost

#2824 landed as a merge commit (0a08e97d). #2829 was squash-merged (59188e2a) from a branch cut before #2824, so its squashed diff replayed the stale router.rb over the fix and removed the regression spec from router_spec.rb in the same stroke. Git reported no conflict — a squash replays the branch diff onto the tip.

658e0d7f is still an ancestor of HEAD and #2824's CHANGELOG entry survived, so nothing about the history or the changelog suggests the fix is missing. With the spec gone too, the suite stayed green.

Impact (verified on current master)

Restoring the spec against master fails:

origin  = "/:id"                          expected "/:name"
params  = {"id"=>"123", "name"=>"123"}    expected {"name"=>"123"}

When a route cascades (X-Cascade: pass) and a later candidate answers:

  • the route helper returns the cascaded route, not the serving one;
  • the cascaded attempt's path captures leak into params — the endpoint sees id, which it never declared. This is the more consequential half, since it reaches user code and declared.

Notes

🤖 Generated with Claude Code

#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>
@github-actions

Copy link
Copy Markdown

Danger Report

No issues found.

View run

@ericproulx
ericproulx merged commit f7175ca into master Jul 27, 2026
69 checks passed
@ericproulx
ericproulx deleted the restore-routing-args-per-attempt branch July 27, 2026 21:14
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