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

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
ericproulx added a commit that referenced this pull request Sep 7, 2026
Auditing the 4.0.0 CHANGELOG against every commit since v3.3.5 turned up
three defects.

#2824 and #2834 were both listed under 4.0.0, and #2834 was listed a
second time under 3.3.5. The three entries describe one net change: #2824
fixed cascaded routes leaking route_info and path captures, #2829's
squash silently replayed a stale router.rb over it, and #2834 restored
it. The fix shipped in 3.3.5, so 4.0.0 keeps only #2829, whose
"hand over to every remaining route" fix is genuinely new here.

#2870 had no entry at all. It is the last of the #2864-#2867 params-DSL
stack and the only merged lib-affecting PR in the range without one.

The `http_digest` removal note sat under "Upgrading to >= 3.3", but #2849
is not in v3.3.5 and the section's own text says the method now raises
NoMethodError -- which is only true from 4.0.0. Moved into the 4.0.0
block, next to the auth/http_basic positional-Hash entry. While there,
closed a ```ruby fence opened at the ValidationErrors keyword-rename
entry and never terminated, which was swallowing the heading and prose
of the section that follows it.

The version benchmark now benches the latest patch of each major/minor
(3.0.1, 3.1.1, 3.2.1) rather than the .0 releases, with RESULTS.md
regenerated on Ruby 4.0.6; README.md and app.rb follow the new list.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericproulx added a commit that referenced this pull request Sep 7, 2026
)

Auditing the 4.0.0 CHANGELOG against every commit since v3.3.5 turned up
three defects.

#2824 and #2834 were both listed under 4.0.0, and #2834 was listed a
second time under 3.3.5. The three entries describe one net change: #2824
fixed cascaded routes leaking route_info and path captures, #2829's
squash silently replayed a stale router.rb over it, and #2834 restored
it. The fix shipped in 3.3.5, so 4.0.0 keeps only #2829, whose
"hand over to every remaining route" fix is genuinely new here.

#2870 had no entry at all. It is the last of the #2864-#2867 params-DSL
stack and the only merged lib-affecting PR in the range without one.

The `http_digest` removal note sat under "Upgrading to >= 3.3", but #2849
is not in v3.3.5 and the section's own text says the method now raises
NoMethodError -- which is only true from 4.0.0. Moved into the 4.0.0
block, next to the auth/http_basic positional-Hash entry. While there,
closed a ```ruby fence opened at the ValidationErrors keyword-rename
entry and never terminated, which was swallowing the heading and prose
of the section that follows it.

The version benchmark now benches the latest patch of each major/minor
(3.0.1, 3.1.1, 3.2.1) rather than the .0 releases, with RESULTS.md
regenerated on Ruby 4.0.6; README.md and app.rb follow the new list.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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