From 9ad00fddf7ce599915102d3978ce3c53b87bb9b1 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 13:52:02 +0200 Subject: [PATCH] Keep a :version path capture when the API declares no version Grape::Request#make_params dropped :version from the routing args unconditionally, alongside :route_info. That is right when Grape put it there -- a path-versioned API captures the version as a segment and exposes it through env['api.version'] rather than params -- but it is not always Grape's. An API that declares no version can name a param :version: route_param :version do get { params[:version] } # => nil end The route matched and Mustermann captured the segment, but the value was filtered out before the endpoint saw it, so params[:version] came back nil and the key was absent from params entirely. Same for a bare get '/:version'. Silent loss of a segment on a route that had matched. A route reports a #version only when the API declared one, so use that to tell the two apart: drop the capture when the route carries a version, keep it otherwise. Path, header and param versioning are unaffected -- their routes all report a version, so :version stays out of params and env['api.version'] remains the way to read it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + lib/grape/request.rb | 28 ++++++++++++++++++++++++-- spec/grape/request_spec.rb | 22 +++++++++++++++++--- spec/integration/hashie/hashie_spec.rb | 2 +- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e54cdff8..6cb4abcb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ * [#2853](https://github.com/ruby-grape/grape/pull/2853): Restore, behind a deprecation warning, the trailing positional options Hash of `requires`, `optional` and `use`, which #2618 turned into a parameter name - [@ericproulx](https://github.com/ericproulx). * [#2856](https://github.com/ruby-grape/grape/pull/2856): Update simplecov - [@ericproulx](https://github.com/ericproulx). * [#2841](https://github.com/ruby-grape/grape/pull/2841): Stop `use`, `helpers`, `rescue_from` and other registrations declared below a route from reaching it when an earlier registration had seeded the same key (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). +* [#2846](https://github.com/ruby-grape/grape/pull/2846): Keep a `:version` path capture in `params` when the API declares no version, instead of always dropping it as Grape's own - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/request.rb b/lib/grape/request.rb index c56237e1d..e016169ed 100644 --- a/lib/grape/request.rb +++ b/lib/grape/request.rb @@ -168,8 +168,7 @@ def cookies? def make_params params = @params_builder.call(rack_params) - routing_args = env[Grape::Env::GRAPE_ROUTING_ARGS] - filtered = routing_args&.except(:version, :route_info) + filtered = routing_args_as_params(env[Grape::Env::GRAPE_ROUTING_ARGS]) return params if filtered.blank? params.deep_merge!(filtered) @@ -177,6 +176,31 @@ def make_params raise Grape::Exceptions::RequestError end + # The routing args carry two things that are not request params: + # +:route_info+, which is always Grape's own, and +:version+, which is only + # Grape's own when the API declared a version — that is captured as a path + # segment and exposed through +env['api.version']+ instead. + # + # An API that declares no version can legitimately name a param +:version+ + # (`route_param :version`, `get '/:version'`), and that capture belongs to + # the application. Dropping it unconditionally left `params[:version]` nil + # on a route that had matched, losing the segment silently. + def routing_args_as_params(routing_args) + return if routing_args.nil? + return routing_args.except(:version, :route_info) if grape_owns_version?(routing_args) + + routing_args.except(:route_info) + end + + # A route reports a +version+ only when the API declared one, which is the + # case where the captured segment is Grape's rather than the application's. + def grape_owns_version?(routing_args) + return false unless routing_args.key?(:version) + + route = routing_args[:route_info] + route.respond_to?(:version) && !route.version.nil? + end + # Uses a plain `each_header` block instead of `each_header.with_object`: # `with_object` can only pass the block one value plus the memo, so the # `k, v` pair would be boxed into a throwaway Array on every header. A diff --git a/spec/grape/request_spec.rb b/spec/grape/request_spec.rb index 19675178f..f1566960d 100644 --- a/spec/grape/request_spec.rb +++ b/spec/grape/request_spec.rb @@ -50,13 +50,29 @@ let(:routing_args) do { version: '123', - route_info: '456', + route_info: instance_double(Grape::Router::Route, version: route_version), c: 'ccc' } end - it 'cuts version and route_info' do - expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc')) + context 'when the route carries a version of its own' do + let(:route_version) { 'v1' } + + it 'cuts version and route_info' do + expect(request.params).to eq(ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc')) + end + end + + # Without a declared version the captured segment is the application's: + # `route_param :version` on an unversioned API has to reach the endpoint. + context 'when the route carries no version' do + let(:route_version) { nil } + + it 'cuts only route_info' do + expect(request.params).to eq( + ActiveSupport::HashWithIndifferentAccess.new(a: '123', b: 'xyz', c: 'ccc', version: '123') + ) + end end end diff --git a/spec/integration/hashie/hashie_spec.rb b/spec/integration/hashie/hashie_spec.rb index c4d373e73..2a839b8e8 100644 --- a/spec/integration/hashie/hashie_spec.rb +++ b/spec/integration/hashie/hashie_spec.rb @@ -152,7 +152,7 @@ let(:routing_args) do { version: '123', - route_info: '456', + route_info: instance_double(Grape::Router::Route, version: 'v1'), c: 'ccc' } end