Skip to content

Commit 927feec

Browse files
ericproulxclaude
andauthored
Fix middleware build crash when a top-level ::Options is in scope (#2773)
Grape::Middleware::Base#initialize used self.class.const_defined?(:Options), whose default inherit: true reaches top-level constants on Object. When any loaded gem defines a global ::Options (e.g. the `options` gem, a transitive dependency of progress_bar), the guard returned true for every middleware subclass, but `self.class::Options` then raised NameError because the `::` resolution operator does not fall back to Object. This took down the whole middleware stack for third-party middleware that does not declare its own Options (e.g. grape_logging's RequestLogger). Resolve the constant through self.class::Options directly: `::` already honours middleware inheritance (e.g. Versioner::Path -> Versioner::Base) and never falls back to Object, so a global ::Options is no longer matched and an absent one cleanly yields the legacy DEFAULT_OPTIONS path. Apply the same treatment to the DEFAULT_OPTIONS lookup, which had the identical defect. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fc6d8c1 commit 927feec

3 files changed

Lines changed: 89 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#### Fixes
99

1010
* [#2767](https://github.com/ruby-grape/grape/pull/2767): Update rubocop to 1.88.0 and rubocop-rspec to 3.10.2 - [@ericproulx](https://github.com/ericproulx).
11+
* [#2773](https://github.com/ruby-grape/grape/pull/2773): Fix middleware build crash when a top-level `::Options` constant is in scope - [@ericproulx](https://github.com/ericproulx).
1112
* Your contribution here.
1213

1314
### 3.3.1 (2026-06-28)

lib/grape/middleware/base.rb

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@ class Base
1616
# subclass's `DEFAULT_OPTIONS` Hash (legacy path) and frozen.
1717
def initialize(app, **options)
1818
@app = app
19-
if self.class.const_defined?(:Options)
20-
# Search ancestors so subclasses (e.g. Versioner::Path → Versioner::Base)
21-
# inherit their parent's Options Data class without redeclaring it.
22-
@config = self.class::Options.new(**options)
19+
if (config_class = options_data_class)
20+
@config = config_class.new(**options)
2321
@options = @config.to_h.freeze
2422
else
2523
@options = merge_default_options(options).freeze
@@ -92,9 +90,25 @@ def merge_headers(response)
9290

9391
def merge_default_options(options)
9492
return default_options.deep_merge(options) if respond_to?(:default_options)
95-
return self.class::DEFAULT_OPTIONS.deep_merge(options) if self.class.const_defined?(:DEFAULT_OPTIONS)
9693

97-
options
94+
default_options_constant&.deep_merge(options) || options
95+
end
96+
97+
# self.class::Options honours middleware inheritance (e.g. Versioner::Path
98+
# → Versioner::Base) and, unlike const_defined?, never resolves a
99+
# top-level ::Options on Object. Absent an own/inherited Options class,
100+
# the lookup raises NameError and we fall back to the legacy path.
101+
def options_data_class
102+
self.class::Options
103+
rescue NameError
104+
nil
105+
end
106+
107+
# Same idea as {#options_data_class} for the legacy DEFAULT_OPTIONS Hash.
108+
def default_options_constant
109+
self.class::DEFAULT_OPTIONS
110+
rescue NameError
111+
nil
98112
end
99113

100114
def try_scrub(obj)

spec/grape/middleware/base_spec.rb

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,74 @@
153153
expect(example_ware.new(blank_app, monkey: false).options[:monkey]).to be false
154154
end
155155
end
156+
157+
context 'when an unrelated top-level ::Options constant is in scope' do
158+
# The `options` gem (a transitive dep of e.g. progress_bar) defines a
159+
# global ::Options. const_defined?(:Options) reaches it through Object,
160+
# but `self.class::Options` does not, so the naive guard raised NameError.
161+
before { stub_const('Options', Module.new) }
162+
163+
let(:example_ware) do
164+
Class.new(Grape::Middleware::Base) do
165+
const_set(:DEFAULT_OPTIONS, { monkey: true }.freeze)
166+
end
167+
end
168+
169+
it 'builds without resolving the global ::Options' do
170+
expect { example_ware.new(blank_app) }.not_to raise_error
171+
end
172+
173+
it 'falls back to the legacy DEFAULT_OPTIONS path' do
174+
expect(example_ware.new(blank_app).options[:monkey]).to be true
175+
end
176+
177+
it 'does not expose a config object' do
178+
expect(example_ware.new(blank_app).config).to be_nil
179+
end
180+
end
181+
182+
context 'when an unrelated top-level ::DEFAULT_OPTIONS constant is in scope' do
183+
before { stub_const('DEFAULT_OPTIONS', { monkey: true }.freeze) }
184+
185+
let(:example_ware) { Class.new(described_class) }
186+
187+
it 'ignores the global ::DEFAULT_OPTIONS' do
188+
expect(example_ware.new(blank_app).options).not_to have_key(:monkey)
189+
end
190+
end
191+
192+
context 'when a middleware declares its own Options Data class' do
193+
let(:example_ware) do
194+
Class.new(Grape::Middleware::Base) do
195+
self::Options = Data.define(:monkey)
196+
end
197+
end
198+
199+
it 'routes options through the Options value object' do
200+
instance = example_ware.new(blank_app, monkey: true)
201+
expect(instance.config.monkey).to be true
202+
expect(instance.options[:monkey]).to be true
203+
end
204+
205+
it 'still resolves its own Options when a global ::Options is in scope' do
206+
stub_const('Options', Module.new)
207+
expect(example_ware.new(blank_app, monkey: true).config.monkey).to be true
208+
end
209+
end
210+
211+
context 'when a middleware inherits its parent Options Data class' do
212+
let(:parent_ware) do
213+
Class.new(Grape::Middleware::Base) do
214+
self::Options = Data.define(:monkey)
215+
end
216+
end
217+
218+
let(:child_ware) { Class.new(parent_ware) }
219+
220+
it 'inherits the ancestor Options without redeclaring it' do
221+
expect(child_ware.new(blank_app, monkey: true).config.monkey).to be true
222+
end
223+
end
156224
end
157225

158226
context 'header' do

0 commit comments

Comments
 (0)