Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
* [#2827](https://github.com/ruby-grape/grape/pull/2827): Make the `cascade` DSL getter return the configured value (`cascade false` read back as `true`) - [@ericproulx](https://github.com/ericproulx).
* [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx).
* [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@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).
* Your contribution here.

### 3.3.5 (2026-07-30)
Expand Down
35 changes: 35 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,41 @@ Upgrading Grape

### Upgrading to >= 4.0.0

#### `use`, `helpers`, `rescue_from` and other registrations no longer reach routes defined above them

A route captures the middleware, helpers, callbacks and rescue handlers registered above it. That was already true most of the time, but not always: `Grape::Util::InheritableSetting#point_in_time_copy` copied a scope's stackable store and its rescue-handler maps shallowly, so the nested Arrays and Hashes stayed shared with the scope. A registration added *after* an endpoint was defined therefore still reached that endpoint — but only when the key already held at least one registration when the endpoint was defined, since otherwise the scope allocated a fresh store only for itself.

The outcome depended on something the API never expressed:

```ruby
class A < Grape::API
use Middleware1
get('/x') { } # endpoint defined here
use Middleware2 # applied to GET /x
end

class B < Grape::API
get('/x') { } # endpoint defined here
use Middleware2 # NOT applied to GET /x
end
```

`A` and `B` state the same thing and behaved differently. Both now behave like `B`. The same held for `rescue_from` declared below a route.

**What can break.** An API that declares `use` (or `helpers`, a filter such as `before`, or `rescue_from`) below its routes and relies on it applying to them. That arrangement only ever worked when an earlier registration for the same key happened to seed the stack, so it was never dependable, but code written against it will now see the middleware or helper silently not run.

**The fix is to move the registration above the routes it should cover**, which is where Grape's documentation has always placed it:

```ruby
class A < Grape::API
use Middleware1
use Middleware2
get('/x') { }
end
```

Nothing changes for the ordinary arrangement — registrations declared before a route, or inherited from an enclosing namespace or a mounting API, still apply exactly as before, including values an enclosing scope gains after the nested scope was created.

#### `Array`/`Set` of an unsupported type is rejected when the API is defined

Declaring a collection whose element type Grape cannot coerce — `type: Array[Foo]` or `type: Set[Foo]` where `Foo` is neither a primitive, a structure, nor a valid custom type — now raises as soon as the `params` block is evaluated, i.e. while the API class is being loaded:
Expand Down
13 changes: 8 additions & 5 deletions lib/grape/util/inheritable_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -778,11 +778,14 @@ def merged_rescue_handlers(key)
def copy_state_from(source)
@namespace = source.namespace.dup
@namespace_inheritable = source.namespace_inheritable&.dup
# Shallow, matching the store this replaced: the per-key Arrays stay
# shared with the source, so a registration made on the source after
# the copy was taken is still visible through it.
@stackable_values = source.stackable_values&.dup
@rescue_handler_maps = source.rescue_handler_maps&.dup
# The nested stores are duped too, not just the Hash holding them. A
# copy is a point in time: what the source registers afterwards must not
# reach it. Sharing them made that depend on whether the key already
# held a registration — `use` below a route applied to it when some
# earlier `use` had seeded the Array, and did nothing otherwise; the
# same went for `rescue_from` below a route.
@stackable_values = source.stackable_values&.transform_values(&:dup)
@rescue_handler_maps = source.rescue_handler_maps&.transform_values(&:dup)
@route = source.route.clone
end

Expand Down
42 changes: 42 additions & 0 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,48 @@ def call(env)
expect(last_response.body).to eql 'hello'
end

# A route captures the middleware registered above it. Whether some
# earlier `use` had already seeded the scope's stack must not change that.
context 'when declared below a route' do
it 'does not apply to that route' do
subject.get('/') { env['phony.args'].inspect }
subject.use phony_middleware, 'too-late'

get '/'
expect(last_response.body).to eql 'nil'
end

it 'does not apply to that route when an earlier use seeded the stack' do
subject.use phony_middleware, 'in-time'
subject.get('/') { env['phony.args'].flatten.inspect }
subject.use phony_middleware, 'too-late'

get '/'
expect(last_response.body).to eql ['in-time'].inspect
end
end
end

describe '.rescue_from declared below a route' do
let(:boom) { Class.new(StandardError) }

it 'does not apply to that route' do
error_class = boom
subject.get('/') { raise error_class }
subject.rescue_from(error_class) { error!('too late', 480) }

expect { get '/' }.to raise_error(error_class)
end

it 'does not apply to that route when an earlier rescue_from seeded the map' do
error_class = boom
subject.rescue_from(ArgumentError) { error!('in time', 481) }
subject.get('/') { raise error_class }
subject.rescue_from(error_class) { error!('too late', 480) }

expect { get '/' }.to raise_error(error_class)
end

it 'adds a block if one is given' do
block = -> {}
subject.use phony_middleware, &block
Expand Down
62 changes: 62 additions & 0 deletions spec/grape/util/inheritable_setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,68 @@
expect(cloned_obj.helpers).to eq [:namespace_stackable_foo_bar]
end

# The case above registers only on the parent, so the copy never shares an
# Array with `subject` and passes even when the per-key Arrays are shared.
# Here the key already holds one of `subject`'s own registrations when the
# copy is taken, which is what made the later one leak into it.
context 'when the scope already registered the key itself' do
subject(:setting) do
described_class.new.tap do |settings|
settings.inherit_from parent
settings.add_helper(:own_before_copy)
end
end

let!(:cloned_obj) { setting.point_in_time_copy }

it 'does not leak a later registration into the copy' do
setting.add_helper(:own_after_copy)

expect(setting.helpers).to eq %i[namespace_stackable_foo_bar own_before_copy own_after_copy]
expect(cloned_obj.helpers).to eq %i[namespace_stackable_foo_bar own_before_copy]
end

it 'does not leak the copy’s own registration back to the source' do
cloned_obj.add_helper(:only_on_copy)

expect(setting.helpers).to eq %i[namespace_stackable_foo_bar own_before_copy]
expect(cloned_obj.helpers).to eq %i[namespace_stackable_foo_bar own_before_copy only_on_copy]
end

it 'keeps sibling copies independent' do
sibling = setting.point_in_time_copy
cloned_obj.add_helper(:only_on_first)

expect(sibling.helpers).to eq %i[namespace_stackable_foo_bar own_before_copy]
end
end

# Same shape as the stackable case: the per-kind Hashes inside
# @rescue_handler_maps have to be duped, not just the Hash holding them.
context 'when the scope already registered a rescue handler' do
subject(:setting) do
described_class.new.tap do |settings|
settings.add_rescue_handlers({ ArgumentError => :before_copy }, subclasses: true)
end
end

let!(:cloned_obj) { setting.point_in_time_copy }

it 'does not leak a later handler into the copy' do
setting.add_rescue_handlers({ TypeError => :after_copy }, subclasses: true)

expect(setting.rescue_handlers).to eq(ArgumentError => :before_copy, TypeError => :after_copy)
expect(cloned_obj.rescue_handlers).to eq(ArgumentError => :before_copy)
end

it 'does not leak a later base-only handler into the copy' do
setting.add_rescue_handlers({ TypeError => :after_copy }, subclasses: false)

expect(setting.base_only_rescue_handlers).to eq(TypeError => :after_copy)
expect(cloned_obj.base_only_rescue_handlers).to be_blank
end
end

it 'decouples route values' do
expect(cloned_obj.route[:route_thing]).to eq :route_foo_bar

Expand Down
Loading