From 1a362f12b6a8ce0211818a08413154da34609c6b Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 29 Jul 2026 22:52:26 +0200 Subject: [PATCH 01/11] Reject params nested deeper than the array scope declares `AttributesIterator#do_each` recursed into any element that was an Array. That recursion is required when the declaration itself nests array scopes -- `map_params` adds one level of nesting per element-iterating scope on the chain, so the params for the inner scope really are an array of arrays -- but nothing compared the incoming depth against the declared one. A request could therefore wrap its elements in extra arrays and have them silently unwrapped: params do requires :lines, type: Array do requires :book_id, type: String requires :qty, type: Integer end end `{"lines":[[{"book_id":"x","qty":1}]]}` passed validation, and `params[:lines]` / `declared` then handed the endpoint `[[{...}]]`. Any ordinary body assuming the declared shape (`params[:lines].sum { |l| l[:qty] }`) died with a `TypeError`, i.e. a 500 on malformed input. `[[]]` passed the same way. Each scope now records, at definition time, how many element-iterating scopes sit on its chain; the iterator descends that many levels less the one `Array.wrap` already consumes, and yields anything deeper as-is. The attribute validators then see a non-hash and fail it exactly as they do for any other unexpected element type, so these requests get the 400 they always should have. "Element-iterating" is a scope predicate rather than an `== Array` test, because `type: Array[JSON]` iterates elements just as much but evaluates to the Array *instance* `[JSON]` rather than the Array class. Excluding it also cost `Array[JSON]` its error indices: every element reported under the same bracket-less name, so `docs[1][name] is missing` came out as `docs[name] is missing`, and two failing elements deduped into one message. Sharing the predicate fixes that too -- `Array[JSON]` now reports per element exactly as `type: Array do` does. Co-Authored-By: Claude Opus 5 (cherry picked from commit adeb5499fae69ec491b8e9fa012f982517924733) --- CHANGELOG.md | 2 + lib/grape/validations/attributes_iterator.rb | 18 +++- lib/grape/validations/params_scope.rb | 26 ++++- spec/grape/validations/params_scope_spec.rb | 95 +++++++++++++++++++ .../validators/coerce_validator_spec.rb | 32 ++++++- 5 files changed, 165 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0049465b9..d6e57e613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ * [#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). +* [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx). +* [#2838](https://github.com/ruby-grape/grape/pull/2838): Reject request params nested in more arrays than the block declares, instead of silently unwrapping them and passing validation, and report `type: Array[JSON]` errors against the element that failed - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb index 578ca3fab..dbc119d27 100644 --- a/lib/grape/validations/attributes_iterator.rb +++ b/lib/grape/validations/attributes_iterator.rb @@ -10,6 +10,11 @@ class AttributesIterator def initialize(attrs, scope) @attrs = attrs @scope = scope + # How many times #do_each may descend into a nested array. The + # declaration allows one level per Array-typed scope on the chain, less + # the one +Array.wrap+ already consumes in #each. Anything deeper was + # put there by the request, not by the declaration. + @max_nesting = [scope.array_depth - 1, 0].max end def each(params, &) @@ -24,19 +29,24 @@ def do_each(params_to_process, original_params, parent_indices = [], &block) params_to_process.each_with_index do |resource_params, index| # when we get arrays of arrays it means that target element located inside array # we need this because we want to know parent arrays indices - if resource_params.is_a?(Array) + # + # Only descend as far as the declaration nests. A request that wraps + # its elements deeper than that is yielded as-is, so the attribute + # validators see a non-hash and fail it the same way any other + # unexpected element type does. + if resource_params.is_a?(Array) && parent_indices.size < @max_nesting do_each(resource_params, original_params, [index] + parent_indices, &block) next end - if @scope.type == Array + if @scope.iterates_elements? next unless original_params.is_a?(Array) # do not validate content of array if it isn't array store_indices(@scope, index, parent_indices) elsif original_params.is_a?(Array) # Lateral scope (no @element) whose params resolved to an array — - # delegate index tracking to the nearest array-typed ancestor so - # that full_name produces the correct bracketed index. + # delegate index tracking to the nearest element-iterating ancestor + # so that full_name produces the correct bracketed index. target = @scope.nearest_array_ancestor store_indices(target, index, parent_indices) if target end diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index dcfd44095..8c810d914 100644 --- a/lib/grape/validations/params_scope.rb +++ b/lib/grape/validations/params_scope.rb @@ -3,7 +3,7 @@ module Grape module Validations class ParamsScope - attr_reader :parent, :type, :nearest_array_ancestor, :full_path + attr_reader :parent, :type, :nearest_array_ancestor, :array_depth, :full_path def qualifying_params ParamScopeTracker.current&.qualifying_params(self) @@ -78,6 +78,9 @@ def initialize(api:, element: nil, element_renamed: nil, parent: nil, optional: # configure_declared_params consumes it and clears @declared_params to nil. @declared_params = [] @full_path = build_full_path + # Read by the validators instantiated from the block below, so it has to + # be settled before the instance_eval. + @array_depth = find_array_depth instance_eval(&block) if block @@ -169,6 +172,17 @@ def nested? @parent && @element end + # Whether this scope's params resolve to one entry per element, which is + # what makes both an element index and a nesting level meaningful. + # + # +type: Array[JSON]+ counts as much as +type: Array+ does. It is easy to + # miss because it evaluates to the Array *instance* +[JSON]+ rather than + # the Array class, so an +== Array+ test quietly excluded it. + # @return [Boolean] + def iterates_elements? + @type == Array || @type == SPECIAL_JSON.last + end + # A lateral scope is subordinate to its parent, but its keys are at the # same level as its parent and thus is not contained within an element. # @return [Boolean] whether or not this scope is lateral @@ -320,10 +334,18 @@ def configure_declared_params def find_nearest_array_ancestor scope = @parent - scope = scope.parent while scope && scope.type != Array + scope = scope.parent while scope && !scope.iterates_elements? scope end + # Every element-iterating scope on the chain adds one level of nesting to + # what {#params} returns, because +map_params+ maps over the array it + # resolved from the parent. Counting them tells {AttributesIterator} how + # deep the declaration says the params for this scope may legitimately be. + def find_array_depth + (iterates_elements? ? 1 : 0) + (@parent&.array_depth || 0) + end + def validates(attrs, validations) process_oneof!(validations) if validations.key?(:oneof) spec = ValidationsSpec.from(validations) diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index a6733a772..b1df98d1e 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -761,6 +761,101 @@ def initialize(value) end end + context 'when the request nests its arrays deeper than the declaration' do + before do + subject.params do + requires :lines, type: Array do + requires :name, type: String + end + end + subject.post('/lines') { 'ok' } + end + + it 'accepts the declared shape' do + post '/lines', { lines: [{ name: 'x' }] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(201) + end + + # Without this the elements are silently unwrapped, validation passes, and + # the endpoint receives an Array where it declared a Hash. + it 'rejects elements wrapped in an extra array' do + post '/lines', { lines: [[{ name: 'x' }]] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('lines[0][name] is missing, lines[0][name] is invalid') + end + + it 'rejects elements wrapped in several extra arrays' do + post '/lines', { lines: [[[{ name: 'x' }]]] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + end + + it 'rejects an empty array element' do + post '/lines', { lines: [[]] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + end + end + + context 'when an array is declared inside a hash' do + before do + subject.params do + requires :outer, type: Hash do + requires :inner, type: Array do + requires :leaf, type: String + end + end + end + subject.post('/hash_array') { 'ok' } + end + + it 'accepts the declared shape' do + post '/hash_array', { outer: { inner: [{ leaf: 'x' }] } }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(201) + end + + it 'rejects elements wrapped in an extra array' do + post '/hash_array', { outer: { inner: [[{ leaf: 'x' }]] } }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + end + end + + context 'when arrays are nested in the declaration' do + before do + subject.params do + requires :a, type: Array do + requires :b, type: Array do + requires :c, type: String + end + end + end + subject.post('/nested_arrays') { 'ok' } + end + + it 'still descends as deep as the declaration nests' do + post '/nested_arrays', { a: [{ b: [{ c: 'x' }] }] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(201) + end + + it 'reports errors against the inner elements' do + post '/nested_arrays', { a: [{ b: [{}] }] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq('a[0][b][0][c] is missing') + end + + it 'rejects one level deeper than declared' do + post '/nested_arrays', { a: [{ b: [[{ c: 'x' }]] }] }.to_json, 'CONTENT_TYPE' => 'application/json' + + expect(last_response.status).to eq(400) + end + end + context 'array without given' do before do subject.params do diff --git a/spec/grape/validations/validators/coerce_validator_spec.rb b/spec/grape/validations/validators/coerce_validator_spec.rb index 50083c16a..263e491f4 100644 --- a/spec/grape/validations/validators/coerce_validator_spec.rb +++ b/spec/grape/validations/validators/coerce_validator_spec.rb @@ -1011,13 +1011,41 @@ def self.parse(_val) expect(last_response).to be_successful expect(last_response.body).to eq("#{integer_class_name}.#{integer_class_name}") + # A bare object is coerced into a one-element array, so it reports + # against that element the same way an explicit array does. get '/', splines: '{"x":"4","y":"woof"}' expect(last_response).to be_bad_request - expect(last_response.body).to eq('splines[x] does not have a valid value') + expect(last_response.body).to eq('splines[0][x] does not have a valid value') get '/', splines: '[{"x":"4","y":"woof"}]' expect(last_response).to be_bad_request - expect(last_response.body).to eq('splines[x] does not have a valid value') + expect(last_response.body).to eq('splines[0][x] does not have a valid value') + end + + it 'reports Array[JSON] errors against the element that failed' do + subject.params do + requires :splines, type: Array[JSON] do + requires :x, type: Integer + end + end + subject.get('/') { 'ok' } + + get '/', splines: '[{"x":1},{},{"x":3}]' + expect(last_response).to be_bad_request + expect(last_response.body).to eq('splines[1][x] is missing') + end + + it 'reports every failing Array[JSON] element, not just one' do + subject.params do + requires :splines, type: Array[JSON] do + requires :x, type: Integer + end + end + subject.get('/') { 'ok' } + + get '/', splines: '[{},{}]' + expect(last_response).to be_bad_request + expect(last_response.body).to eq('splines[0][x] is missing, splines[1][x] is missing') end it "doesn't make sense using coerce_with" do From 386718bb21cd31349c40110f52540ea2703e5aa8 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 29 Jul 2026 22:59:04 +0200 Subject: [PATCH 02/11] Tag path params as UTF-8 instead of leaving them binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mustermann decodes path captures out of PATH_INFO, which Rack hands over tagged ASCII-8BIT, and nothing re-tagged the result. Query and body params arrive UTF-8 because Rack tags those itself, so the same value reached the endpoint with a different encoding depending on where it came from. That made an API's declarations disagree with themselves -- a binary string never equals the UTF-8 literal it was written as: params { requires :id, type: String, values: ['café'] } GET /?id=café -> 200 GET /café -> 400 "id does not have a valid value" The same held for same_as, except_values and any comparison an endpoint made against a non-ASCII literal. It also leaked into serialization: a non-ASCII path param rendered into a JSON response drew an encoding warning from the json gem, which that gem says will become an error in json 3.0. Re-tag in Route#params_for, the single funnel for path-extracted values. Nothing obliges a client to send UTF-8 -- HTTP treats the request target as octets, and Rack's SPEC has CGI keys carry non-ASCII as ASCII-8BIT -- so UTF-8 is the convention rather than a guarantee: it is what browsers percent-encode with, what an IRI maps to, and what Rails settles on (ActionDispatch::Journey::Router force_encodes every path capture to UTF-8 after unescaping it). Only the encoding changes here: the bytes are untouched, so octets that are not UTF-8 stay invalid and are still caught downstream instead of being silently scrubbed into something the client never sent. Unnamed splats capture into an Array, so those are walked too. No extra allocations -- the captures are freshly built and unfrozen, so the re-tag happens in place. Co-Authored-By: Claude Opus 5 (cherry picked from commit 80e9e9994191d87d68c0462fea84020297593a2a) --- CHANGELOG.md | 1 + UPGRADING.md | 39 ++++++++++++++++++++++++ lib/grape/router/route.rb | 29 +++++++++++++++++- spec/grape/api_spec.rb | 62 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6e57e613..3a8fb5106 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ * [#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). * [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx). * [#2838](https://github.com/ruby-grape/grape/pull/2838): Reject request params nested in more arrays than the block declares, instead of silently unwrapping them and passing validation, and report `type: Array[JSON]` errors against the element that failed - [@ericproulx](https://github.com/ericproulx). +* [#2839](https://github.com/ruby-grape/grape/pull/2839): Tag path params as UTF-8 instead of leaving them ASCII-8BIT, so they compare equal to the non-ASCII literals an API declares (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/UPGRADING.md b/UPGRADING.md index ab4838229..248a34c83 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,45 @@ Upgrading Grape ### Upgrading to >= 4.0.0 +#### Path params are tagged UTF-8 instead of ASCII-8BIT + +Params captured from the request path — `route_param`, `:id`-style segments, splats — now come back tagged `UTF-8`. They used to carry the `ASCII-8BIT` encoding of Rack's `PATH_INFO`, because Mustermann decodes the path against that raw string and nothing re-tagged the result. Query and body params were already `UTF-8`, since Rack tags those itself. + +**Why UTF-8.** Nothing obliges a client to send it — HTTP treats the request target as octets, and Rack's SPEC has CGI keys carry non-ASCII as `ASCII-8BIT`. UTF-8 is a convention rather than a guarantee. But it is the convention Rack itself already applies to everything *except* the path: `Rack::QueryParser#unescape` decodes the query string and form bodies with `URI.decode_www_form_component(string, Encoding::UTF_8)`, which tags the result without validating it. + +One request carrying the same invalid octets in three places, before this change: + +| source | encoding | `valid_encoding?` | +| --- | --- | --- | +| query — `?q=%C3%28` | `UTF-8` | `false` | +| form body — `form=%C3%28` | `UTF-8` | `false` | +| path — `/%C3%28` | **`ASCII-8BIT`** | `true` | + +The bytes are equally malformed in all three; only the label differed. The path reads `true` merely because `ASCII-8BIT` considers every byte sequence valid. So this change is not Grape adopting an outside convention — it is Grape agreeing with the library handing it the request. The path param was the odd one out only because Mustermann decodes against `PATH_INFO` directly and never had Rack's `unescape` applied to it. + +Grape does exactly what Rack does: re-tag, do not validate. The bytes are untouched, so octets that are *not* UTF-8 stay detectably invalid rather than being scrubbed into something the client never sent. (Rails takes the same approach for path captures — `ActionDispatch::Journey::Router` force-encodes each one to UTF-8 after unescaping.) + +**What this fixes.** An API's own declarations used to disagree with themselves depending on where a value arrived from — a binary string never equals the UTF-8 literal it was written as: + +```ruby +params { requires :id, type: String, values: ['café'] } +``` + +| request | before | 4.0 | +| --- | --- | --- | +| `GET /?id=café` (query) | `200` | unchanged | +| `GET /café` (path) | `400 "id does not have a valid value"` | `200` | + +The same held for `same_as`, `except_values`, and any comparison an endpoint made against a non-ASCII literal. Serialization was affected too: a non-ASCII path param rendered into a JSON response emitted an encoding warning from the `json` gem, and is slated to raise there in json 3.0. + +**What can break.** Only the encoding tag changes; the bytes are untouched, and an invalid byte sequence stays invalid rather than being scrubbed. Comparisons against pure-ASCII strings are unaffected. Code that relied on a path param being binary — concatenating one with genuinely binary data, for instance — can now raise `Encoding::CompatibilityError`, and should call `.b` on the param to opt back into binary: + +```ruby +params[:id].b + binary_blob +``` + +An application that worked around the old behavior with its own `force_encoding(Encoding::UTF_8)` needs no change; that call is now a no-op. + #### `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: diff --git a/lib/grape/router/route.rb b/lib/grape/router/route.rb index 7afa40bc1..4f3e5e55b 100644 --- a/lib/grape/router/route.rb +++ b/lib/grape/router/route.rb @@ -49,7 +49,7 @@ def params_for(input) parsed = pattern.params(input) return unless parsed - parsed.compact.symbolize_keys + parsed.compact.symbolize_keys.transform_values! { |value| tag_utf8(value) } end protected @@ -60,6 +60,33 @@ def convert_to_head_request! private + # Mustermann decodes path captures out of +PATH_INFO+, which Rack hands us + # tagged ASCII-8BIT, so path params came back binary while Rack tags query + # and body params UTF-8. That split makes an API's own declarations + # disagree with themselves: `values: ['café']` matched `?id=café` but not + # `/café`, since a binary string never equals the UTF-8 literal it was + # written as. + # + # Re-tag as UTF-8. Nothing obliges a client to send UTF-8: the request + # target is octets to HTTP, and Rack's SPEC has CGI keys carry non-ASCII + # as ASCII-8BIT. But UTF-8 is what browsers percent-encode with, what an + # IRI maps to, and what Rails settles on — ActionDispatch::Journey::Router + # force_encodes every path capture to UTF-8 after unescaping it. + # + # Only the encoding changes; the bytes are untouched. Octets that are not + # UTF-8 therefore stay invalid and are caught downstream rather than being + # silently scrubbed into something the client never sent. + def tag_utf8(value) + # String first: every capture but a multi-splat is one. + if value.is_a?(String) + value.encoding == Encoding::UTF_8 ? value : (+value).force_encoding(Encoding::UTF_8) + elsif value.is_a?(Array) + value.map { |element| tag_utf8(element) } + else + value + end + end + def upcase_method(method) method_s = method.to_s Grape::HTTP_SUPPORTED_METHODS.detect { |m| m.casecmp(method_s).zero? } || method_s.upcase diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ff3454f15..ba32313b3 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -309,6 +309,68 @@ expect(last_response.body).to eq('{"foo":1234}') end end + + context 'with a non-ascii segment' do + it 'tags the param as UTF-8 rather than leaving it binary' do + subject.route_param :id do + get { params[:id].encoding.name } + end + + get '/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + it 'tags every captured segment' do + subject.namespace :a do + route_param :one do + route_param :two do + get { [params[:one].encoding.name, params[:two].encoding.name].join(',') } + end + end + end + + get '/a/caf%C3%A9/th%C3%A9' + expect(last_response.body).to eq('UTF-8,UTF-8') + end + + it 'tags a splat capture' do + subject.get('/files/*path') { params[:path].encoding.name } + + get '/files/a/b/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + # An unnamed splat captures into an Array rather than a String. + it 'tags every element of an unnamed splat capture' do + subject.get('/files/*') { params[:splat].map { |s| s.encoding.name }.join(',') } + + get '/files/caf%C3%A9' + expect(last_response.body).to eq('UTF-8') + end + + # A path param used to arrive binary while the same value arrived UTF-8 + # through the query string, so an API's own declaration disagreed with + # itself depending on where the value came from. + it 'compares equal to the utf-8 literal the API declares' do + subject.params { requires :id, type: String, values: ['café'] } + subject.route_param :id do + get { 'matched' } + end + + get '/caf%C3%A9' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('matched') + end + + it 'leaves the bytes untouched' do + subject.route_param :id do + get { params[:id] } + end + + get '/caf%C3%A9' + expect(last_response.body.b).to eq('caf%C3%A9'.b.gsub('%C3%A9', "\xC3\xA9".b)) + end + end end describe '.route' do From 0e1d67cf9e451b7b18a54540a248cf6d677a62d7 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 29 Jul 2026 23:06:25 +0200 Subject: [PATCH 03/11] Answer 500 when an error response cannot be rendered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Grape::Middleware::Error#call! renders the error response from inside its own rescue clause, so that clause never covered the rendering. An error formatter that raised on the payload it was handed took the exception straight out through every middleware above Grape and into the application server — `rescue_from :all` did not help, because the failure happened after the handler had already returned. A rescue_from handler echoing request-derived bytes was enough to hit it: rescue_from(Missing) { |e| error!({ detail: e.message }, 404) } with an invalid UTF-8 byte in the path, the JSON formatter raised JSON::GeneratorError and the request died rather than being answered. Guard the rendering in error_response. On failure, first retry the API's own format with the framework's InternalServerError, whose message is a static string and so cannot be what defeated the first attempt; if that fails too — a formatter broken outright rather than one payload it choked on — answer without a formatter at all. Both attempts call format_message directly instead of re-entering error_response, so the fallback cannot recurse. The guard sits on the rendering rather than around run_rescue_handler on purpose. Wrapping the handler call too would have swallowed things that must keep propagating, the deprecation raised when a handler returns a Hash among them. Exceptions that no rescue_from matches still propagate unchanged; only rendering failures are caught. The exception that defeated rendering is put on env['grape.exception'], the key the existing unrecognised-error path already uses, so upstream loggers can still observe it. Co-Authored-By: Claude Opus 5 (cherry picked from commit 4b7a3b3b08052e2c7fe224504127d4809e1321db) --- CHANGELOG.md | 1 + UPGRADING.md | 13 ++++++ lib/grape/middleware/error.rb | 47 +++++++++++++++++++- spec/grape/middleware/error_spec.rb | 67 +++++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a8fb5106..135bfd92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ * [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx). * [#2838](https://github.com/ruby-grape/grape/pull/2838): Reject request params nested in more arrays than the block declares, instead of silently unwrapping them and passing validation, and report `type: Array[JSON]` errors against the element that failed - [@ericproulx](https://github.com/ericproulx). * [#2839](https://github.com/ruby-grape/grape/pull/2839): Tag path params as UTF-8 instead of leaving them ASCII-8BIT, so they compare equal to the non-ASCII literals an API declares (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). +* [#2840](https://github.com/ruby-grape/grape/pull/2840): Answer 500 instead of letting an exception escape the middleware stack when an error response cannot be rendered (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/UPGRADING.md b/UPGRADING.md index 248a34c83..8639c925c 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -41,6 +41,19 @@ params[:id].b + binary_blob ``` An application that worked around the old behavior with its own `force_encoding(Encoding::UTF_8)` needs no change; that call is now a no-op. +#### A failed error rendering answers 500 instead of escaping the middleware stack + +When Grape could not render an error response — an error formatter handed a payload it cannot serialize, most often — the exception escaped every middleware above Grape and reached the application server. Rendering runs inside `Grape::Middleware::Error#call!`'s own `rescue` clause, so that clause did not cover it. + +Grape now answers `500` instead: first retrying the API's format with the framework's own `Internal Server Error` message, then falling back to a bare `text/plain` body if even that cannot be rendered. + +**What can break.** Code that observed these exceptions by letting them propagate — an error tracker mounted as Rack middleware above Grape, or a test asserting `expect { get '/' }.to raise_error` — no longer sees them. The exception is exposed on the rack env instead, under the same key the existing unrecognised-error path uses: + +```ruby +env[Grape::Env::GRAPE_EXCEPTION] # => the exception that defeated rendering +``` + +Exceptions that no `rescue_from` matches still propagate exactly as before; only rendering failures changed. #### `Array`/`Set` of an unsupported type is rejected when the API is defined diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index f12fa398e..b1c569da8 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -48,6 +48,13 @@ def initialize( def_delegator :rescue_options, :backtrace, :include_backtrace def_delegator :rescue_options, :original_exception, :include_original_exception + # Emitted by {#failsafe_response} once even the framework's own message + # could not be rendered. Deliberately built without a formatter, an i18n + # lookup or anything else that could be the thing that is broken. + FAILSAFE_STATUS = 500 + FAILSAFE_MESSAGE = '500 Internal Server Error' + FAILSAFE_CONTENT_TYPE = 'text/plain' + def call!(env) @env = env error_response(catch(:error) { return @app.call(@env) }) @@ -104,7 +111,45 @@ def error_response(error = nil) backtrace: raw.backtrace || raw.original_exception&.backtrace || [] ) env[Grape::Env::API_ENDPOINT].status(payload.status) # error! may not have been called - rack_response(payload.status, payload.headers, format_message(payload)) + begin + rack_response(payload.status, payload.headers, format_message(payload)) + rescue StandardError => e + failsafe_response(e) + end + end + + # Last resort for an error response that could not be rendered — an error + # formatter handed a payload it cannot serialize, typically. Rendering runs + # inside #call!'s rescue clause, so it is not covered by that rescue and + # anything raised here would escape the entire middleware stack. Grape has + # committed to answering with an error by this point, so it answers with + # one that does not depend on the payload rather than dropping the request. + # + # First retry the API's own format with the framework's InternalServerError, + # whose message is a static string and so cannot be what defeated the first + # attempt. Should even that fail — a wholesale broken formatter, rather than + # one payload it choked on — drop the formatter entirely. Both attempts call + # {#format_message} directly rather than re-entering {#error_response}, so + # this path cannot recurse. + # + # The exception is exposed on the rack env so upstream middleware (loggers, + # error trackers) can still observe what actually went wrong. + def failsafe_response(error) + env[Grape::Env::GRAPE_EXCEPTION] = error + headers = { Rack::CONTENT_TYPE => content_type } + rack_response(FAILSAFE_STATUS, headers, format_message(failsafe_payload(headers))) + rescue StandardError + rack_response(FAILSAFE_STATUS, { Rack::CONTENT_TYPE => FAILSAFE_CONTENT_TYPE }, FAILSAFE_MESSAGE) + end + + def failsafe_payload(headers) + Grape::Exceptions::ErrorResponse.new( + status: FAILSAFE_STATUS, + message: Grape::Exceptions::InternalServerError.new.message, + headers:, + backtrace: [], + original_exception: nil + ) end def default_rescue_handler(exception) diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 320a68d61..5edf8be84 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -99,6 +99,73 @@ def self.call(_env) end end + # Rendering happens inside #call!'s rescue clause, so it is not covered by + # that rescue: without a failsafe an error formatter that raises takes the + # exception straight out through every middleware above. + describe 'when the error response cannot be rendered' do + subject(:response) do + get '/' + last_response + end + + context 'and the formatter chokes on the payload' do + let(:app) do + Class.new(Grape::API) do + format :json + + rescue_from(:all) { |e| error!({ detail: e.message }, 404) } + + # A message the JSON formatter cannot serialize. + get('/') { raise StandardError, +"bad \xC3 byte".b } + end + end + + it 'answers with the framework message in the API format' do + expect(response.status).to eq(500) + expect(response.headers[Rack::CONTENT_TYPE]).to include('application/json') + expect(JSON.parse(response.body)).to eq('error' => 'Internal Server Error') + end + + it 'exposes the rendering failure on the rack env' do + get '/' + expect(last_request.env[Grape::Env::GRAPE_EXCEPTION]).to be_a(StandardError) + end + end + + context 'and the formatter is broken outright' do + let(:app) do + Class.new(Grape::API) do + format :json + + error_formatter :json, ->(**) { raise 'formatter is broken' } + rescue_from(:all) { error!({ detail: 'nope' }, 404) } + + get('/') { raise StandardError, 'boom' } + end + end + + it 'drops the formatter rather than recursing' do + expect(response.status).to eq(500) + expect(response.headers[Rack::CONTENT_TYPE]).to include('text/plain') + expect(response.body).to eq('500 Internal Server Error') + end + end + + context 'and nothing rescues the original exception' do + let(:app) do + Class.new(Grape::API) do + format :json + + get('/') { raise ArgumentError, 'kaboom' } + end + end + + it 'keeps propagating it' do + expect { get '/' }.to raise_error(ArgumentError, 'kaboom') + end + end + end + describe 'when a rescue_from block raises' do subject(:response) do get '/' From 3c6abe026997eaea1ec54602124c7662b399ba8f Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 11:04:13 +0200 Subject: [PATCH 04/11] Give point-in-time copies their own stackable and rescue-handler stores 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 source. A registration made after an endpoint was defined therefore still reached that endpoint -- but only when the key already held a registration at the time the copy was taken, since otherwise the writer allocated a fresh store on the source alone. That made the outcome depend on something the API never expressed: use Middleware1 get('/x') { } use Middleware2 # applied to GET /x get('/x') { } use Middleware2 # did NOT apply to GET /x and the same for rescue_from: rescue_from ArgumentError { } get('/x') { raise Boom } rescue_from Boom { } # rescued GET /x get('/x') { raise Boom } rescue_from Boom { } # did NOT rescue GET /x Two APIs stating the same thing, behaving differently. Not a deliberate "late registration" feature -- helpers defined after an endpoint already did not leak, because they resolve down a different path. Dup the nested stores as well as the Hash holding them. A copy is a point in time: what the source registers afterwards must not reach it. Inheritance is untouched -- it resolves by walking #parent, so a scope still sees values an enclosing scope gains later, which is what the existing "decouples namespace stackable values" spec actually exercised (its value lives on the parent, so nothing was ever shared and it passed either way). Boot cost is negligible: 300 endpoints under scopes carrying middleware, helpers and filters allocate 1800 more objects (+0.4%) with no measurable change in time, all at definition time. Co-Authored-By: Claude Opus 5 (cherry picked from commit 2599501faa3700a0c9d1b4958392fe669a7f6f4f) --- CHANGELOG.md | 1 + UPGRADING.md | 34 +++++++++++ lib/grape/util/inheritable_setting.rb | 13 +++-- spec/grape/api_spec.rb | 42 ++++++++++++++ spec/grape/util/inheritable_setting_spec.rb | 62 +++++++++++++++++++++ 5 files changed, 147 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 135bfd92c..c80dd8a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ * [#2838](https://github.com/ruby-grape/grape/pull/2838): Reject request params nested in more arrays than the block declares, instead of silently unwrapping them and passing validation, and report `type: Array[JSON]` errors against the element that failed - [@ericproulx](https://github.com/ericproulx). * [#2839](https://github.com/ruby-grape/grape/pull/2839): Tag path params as UTF-8 instead of leaving them ASCII-8BIT, so they compare equal to the non-ASCII literals an API declares (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2840](https://github.com/ruby-grape/grape/pull/2840): Answer 500 instead of letting an exception escape the middleware stack when an error response cannot be rendered (see UPGRADING) - [@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) diff --git a/UPGRADING.md b/UPGRADING.md index 8639c925c..49b6db4a0 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -54,6 +54,40 @@ env[Grape::Env::GRAPE_EXCEPTION] # => the exception that defeated rendering ``` Exceptions that no `rescue_from` matches still propagate exactly as before; only rendering failures changed. +#### `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 diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index 21593255f..65cba6a59 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ba32313b3..8f2348fe6 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1579,6 +1579,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 diff --git a/spec/grape/util/inheritable_setting_spec.rb b/spec/grape/util/inheritable_setting_spec.rb index 729866879..312d74193 100644 --- a/spec/grape/util/inheritable_setting_spec.rb +++ b/spec/grape/util/inheritable_setting_spec.rb @@ -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 From 7b18c989c8eb0d7c0a10e0a3d2b2eec6a177076a Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 12:00:53 +0200 Subject: [PATCH 05/11] Warn when a rescue_from handler can never run Grape::Middleware::Error resolves a registered handler with #find, so within a scope the first matching class wins. A handler registered for a class an earlier one already covers is therefore dead code, and silently so: rescue_from StandardError do ... end # wins rescue_from ArgumentError do ... end # never runs Swapping the two lines is all it takes, but nothing said so -- and the neighbouring `rescue_from :all` behaves the other way round, since it is consulted only after the registered handlers, so a specific class registered after it still wins. Two documented ways of saying "catch everything", ordering differently against a later specific handler. Warn at definition time rather than reorder: which handler should win is the author's call, and :all already covers "broad first, specific still wins". The check lives in Grape::Util::ShadowedRescueHandlers, called from #add_rescue_handlers where the scope's own registrations are known. Compared within a scope only -- across scopes the nearest registration deliberately wins, so an inner rescue_from StandardError shadowing an outer rescue_from ArgumentError is the documented behaviour, not a mistake. Classes sharing a handler object are skipped too, since `rescue_from A, B` registers one handler for both and the entry that loses changes nothing. Exact-match handlers (rescue_subclasses: false) are consulted before the subclass-matching ones and never match a descendant, so they cannot shadow each other either. README gains the ordering rule, which was only implied. Co-Authored-By: Claude Opus 5 (cherry picked from commit 8f6202a01b2ce1994ef79e3869389882c35b71ee) --- CHANGELOG.md | 1 + README.md | 11 ++++ lib/grape/util/inheritable_setting.rb | 1 + lib/grape/util/shadowed_rescue_handlers.rb | 49 ++++++++++++++++++ spec/grape/util/inheritable_setting_spec.rb | 56 +++++++++++++++++++++ 5 files changed, 118 insertions(+) create mode 100644 lib/grape/util/shadowed_rescue_handlers.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c80dd8a05..694e3e247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,7 @@ * [#2839](https://github.com/ruby-grape/grape/pull/2839): Tag path params as UTF-8 instead of leaving them ASCII-8BIT, so they compare equal to the non-ASCII literals an API declares (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2840](https://github.com/ruby-grape/grape/pull/2840): Answer 500 instead of letting an exception escape the middleware stack when an error response cannot be rendered (see UPGRADING) - [@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). +* [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/README.md b/README.md index efae8b900..f8ac3ab84 100644 --- a/README.md +++ b/README.md @@ -2747,6 +2747,17 @@ end In this case ```UserDefinedError``` must be inherited from ```StandardError```. +When several classes could match, the one registered **first** in a scope wins — as with the clauses of a Ruby `rescue`. Register the more specific class before the broader one, or the narrower handler never runs: + +```ruby +class Twitter::API < Grape::API + rescue_from ArgumentError do ... end # matched first + rescue_from StandardError do ... end # everything else +end +``` + +Grape warns when a `rescue_from` is registered for a class an earlier one in the same scope already covers. This is about ordering within a scope; a handler in a nested namespace or a mounted API always takes precedence over one inherited from an enclosing scope, whatever the classes are. + Notice that you could combine these two approaches (rescuing custom errors takes precedence). For example, it's useful for handling all exceptions except Grape validation errors. ```ruby diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index 65cba6a59..f202b5cb8 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -384,6 +384,7 @@ def base_only_rescue_handlers def add_rescue_handlers(mapping, subclasses:) @rescue_handler_maps ||= {} own = (@rescue_handler_maps[subclasses ? :rescue_handlers : :base_only_rescue_handlers] ||= {}) + ShadowedRescueHandlers.warn_about(own, mapping) if subclasses own.merge!(mapping) { |_klass, registered, _new| registered } end diff --git a/lib/grape/util/shadowed_rescue_handlers.rb b/lib/grape/util/shadowed_rescue_handlers.rb new file mode 100644 index 000000000..b5180adc5 --- /dev/null +++ b/lib/grape/util/shadowed_rescue_handlers.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Grape + module Util + # Diagnostics for +rescue_from+ registrations that can never run. + # + # Middleware::Error resolves with +find+, so within a scope the first + # matching class wins and one registered for a class an earlier handler + # already covers is dead code — silently, before this warned: + # + # rescue_from StandardError do ... end # wins + # rescue_from ArgumentError do ... end # never runs + # + # Warn rather than reorder: which should win is the author's call, and + # +rescue_from :all+ (consulted only after the registered handlers) already + # offers "broad first, specific still wins" to anyone who wants it. + module ShadowedRescueHandlers + module_function + + # @param registered [Hash] the scope's own handlers, in registration order + # @param mapping [Hash] the handlers being registered now + # @return [void] + # + # Only a scope's own registrations are compared: across scopes the nearest + # one deliberately wins, so an inner +rescue_from StandardError+ shadowing + # an outer +rescue_from ArgumentError+ is the documented behaviour rather + # than a mistake. Classes sharing a handler object are skipped too, since + # +rescue_from A, B+ registers one handler for both and the entry that + # loses to the other changes nothing. + def warn_about(registered, mapping) + return if registered.empty? + + mapping.each do |klass, handler| + covered_by, = registered.find { |already, existing| klass <= already && !existing.equal?(handler) } + next unless covered_by + + warn(message_for(klass, covered_by)) + end + end + + def message_for(klass, covered_by) + return "Grape: rescue_from #{klass} was already registered in this scope; the first handler is kept and this one will never run." if klass == covered_by + + "Grape: rescue_from #{klass} will never run — #{covered_by} was registered earlier in the same scope " \ + 'and is matched first. Register the more specific class before the broader one.' + end + end + end +end diff --git a/spec/grape/util/inheritable_setting_spec.rb b/spec/grape/util/inheritable_setting_spec.rb index 312d74193..7fa22f360 100644 --- a/spec/grape/util/inheritable_setting_spec.rb +++ b/spec/grape/util/inheritable_setting_spec.rb @@ -227,6 +227,62 @@ subject.add_rescue_handlers({ StandardError => :child }, subclasses: true) expect(subject.rescue_handlers).to eq(StandardError => :child) end + + # Middleware::Error resolves with #find, so the first matching class in a + # scope wins and a narrower one registered after it is dead code. + describe 'shadowing warnings' do + def add(mapping, subclasses: true) + subject.add_rescue_handlers(mapping, subclasses:) + end + + it 'warns when a broader class was registered first' do + add({ StandardError => :broad }) + + expect { add({ ArgumentError => :narrow }) } + .to output(/rescue_from ArgumentError will never run — StandardError was registered earlier/).to_stderr + end + + it 'warns when the same class is registered twice' do + add({ ArgumentError => :first }) + + expect { add({ ArgumentError => :second }) } + .to output(/rescue_from ArgumentError was already registered in this scope/).to_stderr + end + + it 'does not warn when the narrower class was registered first' do + add({ ArgumentError => :narrow }) + + expect { add({ StandardError => :broad }) }.not_to output.to_stderr + end + + it 'does not warn for unrelated classes' do + add({ ArgumentError => :one }) + + expect { add({ TypeError => :two }) }.not_to output.to_stderr + end + + # `rescue_from A, B` registers one handler for both, so the entry that + # loses to the other changes nothing. + it 'does not warn when both classes share a handler' do + expect { add({ StandardError => :shared, ArgumentError => :shared }) }.not_to output.to_stderr + end + + # Exact-match handlers are consulted before the subclass-matching ones and + # never match a descendant, so they cannot shadow each other. + it 'does not warn for exact-match handlers' do + add({ StandardError => :broad }, subclasses: false) + + expect { add({ ArgumentError => :narrow }, subclasses: false) }.not_to output.to_stderr + end + + # Across scopes the nearest registration deliberately wins. + it 'does not warn about a handler inherited from an enclosing scope' do + parent = described_class.new.tap { |s| s.add_rescue_handlers({ StandardError => :outer }, subclasses: true) } + subject.inherit_from parent + + expect { add({ ArgumentError => :inner }) }.not_to output.to_stderr + end + end end describe '#route' do From 37f967e558fa3c0c07afc6e8696b1acdf050b4a5 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 12:04:51 +0200 Subject: [PATCH 06/11] Let rescue_from :grape_exceptions outrank a catch-all class handler rescue_from :grape_exceptions is an opt-in to keep Grape's own errors rendering with their own status -- a validation failure answers 400 rather than whatever the application's catch-all returns. It only ever worked against rescue_from :all, which lives in all_rescue_handler and is consulted last. Written as a class instead, rescue_from StandardError is a registered handler, matched first, and Grape's exceptions are StandardErrors -- so the opt-in silently did nothing and validation errors still came back as 500s: rescue_from StandardError { error!('server error', 500) } rescue_from :grape_exceptions # inert Let the opt-in win over a handler that only matched through a non-Grape ancestor. A handler registered for a Grape exception class is more precise than the opt-in and still wins, so an explicit rescue_from Grape::Exceptions::ValidationErrors keeps its handler. registered_rescue_handler gains an _entry variant returning the matched class alongside its handler, since deciding this needs to know *which* class matched, not just that one did. find_handler resolves the entry once and passes it down. Application errors still reach the catch-all, rescue_from :all is unchanged, and InvalidVersionHeader is left alone so it keeps reaching Rack and version cascading still works. Co-Authored-By: Claude Opus 5 (cherry picked from commit f67e86c7b8f8ad8a332b0dfb6f3f40b40b8f1604) --- CHANGELOG.md | 1 + README.md | 13 +++++++++ UPGRADING.md | 31 ++++++++++++++++++++ lib/grape/middleware/error.rb | 48 +++++++++++++++++++++++++++---- spec/grape/api_spec.rb | 53 +++++++++++++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 694e3e247..0aa4037d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ * [#2840](https://github.com/ruby-grape/grape/pull/2840): Answer 500 instead of letting an exception escape the middleware stack when an error response cannot be rendered (see UPGRADING) - [@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). * [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx). +* [#2843](https://github.com/ruby-grape/grape/pull/2843): Let `rescue_from :grape_exceptions` take precedence over a catch-all registered as a class, so Grape errors keep their own status (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/README.md b/README.md index f8ac3ab84..f90cd37c9 100644 --- a/README.md +++ b/README.md @@ -2737,6 +2737,19 @@ rescue_from :grape_exceptions do |e| end ``` +The opt-in takes precedence over a catch-all handler, whether that catch-all is written as `rescue_from :all` or as a class such as `rescue_from StandardError`. So a validation failure stays a `400` rather than becoming whatever the catch-all returns: + +```ruby +class Twitter::API < Grape::API + rescue_from StandardError do + error!('server error', 500) + end + rescue_from :grape_exceptions # validation errors still answer 400 +end +``` + +A handler registered for a specific Grape exception class is more precise than the opt-in and still wins, so `rescue_from Grape::Exceptions::ValidationErrors` keeps its own handler. The opt-in only outranks handlers that matched through a non-Grape ancestor. + You can also rescue specific exceptions. ```ruby diff --git a/UPGRADING.md b/UPGRADING.md index 49b6db4a0..cc505a2db 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -88,6 +88,37 @@ 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. +#### `rescue_from :grape_exceptions` now outranks a catch-all class handler + +`rescue_from :grape_exceptions` is an opt-in to keep Grape's own errors rendering with their own status — a validation failure answers `400` rather than whatever the application's catch-all returns. + +It only ever worked against `rescue_from :all`. Written as a class instead, a catch-all is a *registered* handler, which `Grape::Middleware::Error` consults first, and Grape's exceptions are `StandardError`s — so the opt-in was silently inert: + +```ruby +rescue_from StandardError do + error!('server error', 500) +end +rescue_from :grape_exceptions +``` + +| request | before | 4.0 | +| --- | --- | --- | +| fails parameter validation | **500** `server error` | `400` | +| raises an application error | `500` `server error` | unchanged | + +**What can break.** An API that registers a catch-all as a class *and* opts into `:grape_exceptions` will now answer Grape's own status for Grape's own errors, where it previously answered the catch-all's. That is what the opt-in asks for, so the change makes the two spellings agree — but a client or test asserting the catch-all's status for a validation failure will see the new one. + +Precedence is unchanged in every other case. A handler registered for a specific Grape exception class is more precise than the opt-in and still wins: + +```ruby +rescue_from Grape::Exceptions::ValidationErrors do + error!('unprocessable', 422) # still runs +end +rescue_from StandardError { ... } +rescue_from :grape_exceptions +``` + +Application errors still reach the catch-all, `rescue_from :all` behaves as before, and `Grape::Exceptions::InvalidVersionHeader` is still never rescued, so version cascading keeps working. An API that does not use `rescue_from :grape_exceptions` is unaffected. #### `Array`/`Set` of an unsupported type is rejected when the API is defined diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index b1c569da8..a7db81c9b 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -94,7 +94,10 @@ def format_message(error) end def find_handler(klass) - registered_rescue_handler(klass) || + registered_entry = registered_rescue_handler_entry(klass) + + grape_exceptions_precedence_handler(klass, registered_entry) || + registered_entry&.last || rescue_handler_for_grape_exception(klass) || rescue_handler_for_any_class(klass) || raise @@ -163,16 +166,51 @@ def default_rescue_handler(exception) end def registered_rescue_handler(klass) - rescue_handler_from(base_only_rescue_handlers) { |err| klass == err } || - rescue_handler_from(rescue_handlers) { |err| klass <= err } + registered_rescue_handler_entry(klass)&.last + end + + # The matched entry rather than just its handler, so callers can tell + # *which* class matched — see {#grape_exceptions_precedence_handler}. + # @return [Array(Class, #call), nil] + def registered_rescue_handler_entry(klass) + rescue_handler_entry_from(base_only_rescue_handlers) { |err| klass == err } || + rescue_handler_entry_from(rescue_handlers) { |err| klass <= err } end - def rescue_handler_from(handlers) + def rescue_handler_entry_from(handlers) error, handler = handlers&.find { |err, _handler| yield(err) } return unless error - handler || method(:default_rescue_handler) + [error, handler || method(:default_rescue_handler)] + end + + # +rescue_from :grape_exceptions+ is an opt-in to keep Grape's own errors + # rendering with their own status — a validation failure stays a 400 + # instead of becoming whatever the app's catch-all returns. + # + # It only ever worked against +rescue_from :all+, which lives in + # +all_rescue_handler+ and is consulted last. Spelled as a class instead, + # +rescue_from StandardError+ is a *registered* handler, matched first, + # and Grape's exceptions are StandardErrors — so the opt-in silently did + # nothing and validation errors came back as 500s either way. + # + # Let it win over a handler that only matched through a non-Grape + # ancestor. One registered for a Grape exception class is more specific + # than the opt-in and still wins, so an explicit + # +rescue_from Grape::Exceptions::ValidationErrors+ keeps its handler. + # + # InvalidVersionHeader is left alone: it must keep reaching Rack so the + # next versioned route is tried. + def grape_exceptions_precedence_handler(klass, registered_entry) + return unless rescue_grape_exceptions + return unless klass <= Grape::Exceptions::Base + return if klass == Grape::Exceptions::InvalidVersionHeader + + matched, = registered_entry + return if matched.nil? || matched <= Grape::Exceptions::Base + + grape_exceptions_rescue_handler || method(:error_response) end def rescue_handler_for_grape_exception(klass) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 8f2348fe6..7633029b0 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2633,6 +2633,59 @@ def rescue_all_errors expect(last_response).to be_forbidden expect(last_response.body).to eq('Redefined Error') end + + # The opt-in exists to keep Grape's own errors rendering with their own + # status. A catch-all registered as a class is matched before it, and + # Grape's exceptions are StandardErrors, so it used to be silently inert + # unless the catch-all happened to be spelled `rescue_from :all`. + context 'with a catch-all class handler' do + subject(:app) do + Class.new(Grape::API) do + format :json + rescue_from(StandardError) { error!({ h: 'catch-all' }, 500) } + rescue_from(:grape_exceptions) { |e| error!({ h: 'grape' }, e.status) } + params { requires :n, type: Integer } + get('/validated') { 'ok' } + get('/boom') { raise ArgumentError, 'app error' } + end + end + + it 'keeps a validation failure a 400' do + get '/validated' + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq({ h: 'grape' }.to_json) + end + + it 'still sends the application’s own errors to the catch-all' do + get '/boom' + + expect(last_response.status).to eq(500) + expect(last_response.body).to eq({ h: 'catch-all' }.to_json) + end + end + + it 'lets a handler registered for a grape exception class win over the opt-in' do + subject.rescue_from(Grape::Exceptions::ValidationErrors) { error!('specific', 422) } + subject.rescue_from(StandardError) { error!('catch-all', 500) } + subject.params { requires :n, type: Integer } + subject.get('/validated') { 'ok' } + + get '/validated' + + expect(last_response.status).to eq(422) + expect(last_response.body).to eq('specific') + end + + it 'does not intercept an exception that is not a grape exception' do + subject.rescue_from(StandardError) { error!('catch-all', 500) } + subject.get('/boom') { raise ArgumentError } + + get '/boom' + + expect(last_response.status).to eq(500) + expect(last_response.body).to eq('catch-all') + end end describe '.error_format' do From bef3adb4fec0e0c17e65d05033779f7853025b24 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 13:25:29 +0200 Subject: [PATCH 07/11] Look up an entity by the object's own class before its element's Grape::DSL::Entity#object_class decided which class to look an entity up for by duck-typing: return object.klass if object.respond_to?(:klass) return object.first.class if object.respond_to?(:first) object.class Both tests are answered by plenty of single objects, and the object's own class was consulted only last. A Struct is Enumerable, so it responds to #first; so does any model that includes Enumerable; and #klass is hardly exclusive to ActiveRecord::Relation. For all of those, `represent Model, with: Entity` was silently ignored and the entity for whatever #first returned was looked up instead -- usually nothing, so the raw object was serialized. Try the object's own class first and fall back to the collection or wrapped class only when that comes up empty. An Array still resolves through its element class, since Array itself has no entity, and a relation still resolves through #klass. Deferring the fallback also stops #first from being called at all when the object resolves on its own class, which matters for anything where taking the first element is expensive or has side effects. Longstanding: the duck-typing dates to 2014 (v0.10.0) and behaves the same way in 3.3.4. Co-Authored-By: Claude Opus 5 (cherry picked from commit e70e85801be6b828bf21a60986063dea2aeb6041) --- CHANGELOG.md | 1 + lib/grape/dsl/entity.rb | 44 +++++++++++++++++-------- spec/grape/api_spec.rb | 72 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aa4037d2..e05abbb72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ * [#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). * [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx). * [#2843](https://github.com/ruby-grape/grape/pull/2843): Let `rescue_from :grape_exceptions` take precedence over a catch-all registered as a class, so Grape errors keep their own status (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). +* [#2844](https://github.com/ruby-grape/grape/pull/2844): Look an entity up by the presented object's own class before treating it as a collection, so `represent` is no longer skipped for models that respond to `#first` or `#klass` - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/dsl/entity.rb b/lib/grape/dsl/entity.rb index 9dbb42d99..735753a16 100644 --- a/lib/grape/dsl/entity.rb +++ b/lib/grape/dsl/entity.rb @@ -50,11 +50,39 @@ def present(*args, root: nil, with: nil, **options) # @param object [Object] the object to locate the Entity class for # @return [Class] the located Entity class, or nil if none is found def entity_class_for_obj(object) - klass = object_class(object) + entity_for_class(object.class) || entity_for_class(element_class(object)) + end + + private + + # The class standing in for a collection or wrapper: ActiveRecord::Relation + # and the like expose #klass, anything else falls back to the class of its + # first element. + # + # Consulted only once the object's own class has come up empty, because + # both tests are duck-typed and plenty of single objects answer them — + # a Struct is Enumerable, so it responds to #first, and so does any model + # that includes Enumerable. Asking this first meant `represent Model, + # with: Entity` was silently ignored for those, the entity for the + # *element* type being looked up instead. Deferring it also keeps #first + # from being called at all when the object resolves on its own class. + # + # @param object [Object] the object to represent. + # @return [Class, nil] + def element_class(object) + return object.klass if object.respond_to?(:klass) + + object.first.class if object.respond_to?(:first) + end + + # @param klass [Class, nil] the class to look an entity up for. + # @return [Class, nil] the registered or conventionally named entity. + def entity_for_class(klass) + return if klass.nil? representations = inheritable_setting.representations if representations - potential = klass.ancestors.detect { |potential| representations.key?(potential) } + potential = klass.ancestors.detect { |ancestor| representations.key?(ancestor) } return representations[potential] if potential && representations[potential] end @@ -65,18 +93,6 @@ def entity_class_for_obj(object) entity if entity.respond_to?(:represent) end - private - - # Resolves the class used to look up the Entity for +object+. - # @param object [Object] the object to represent. - # @return [Class] the object's collection element class, wrapped class, or its own class. - def object_class(object) - return object.klass if object.respond_to?(:klass) - return object.first.class if object.respond_to?(:first) - - object.class - end - # @param entity_class [Class] the entity class to use for representation. # @param object [Object] the object to represent. # @param options [Hash] additional options forwarded to the entity's `represent` call. diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 7633029b0..d84d611f0 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -177,6 +177,78 @@ subject.represent represent_object, with: dummy_presenter_klass expect(subject.inheritable_setting.representations).to eq(represent_object => dummy_presenter_klass) end + + # Both the collection tests are duck-typed, and plenty of single objects + # answer them, so a registered entity used to be skipped for those in favour + # of the entity for whatever #first returned. + context 'when the presented object also looks like a collection' do + let(:entity) do + Class.new do + def self.represent(object, **) + { presented: object.class.name.to_s } + end + end + end + + def present_with(api, model, object, entity) + api.format :json + api.represent model, with: entity + api.get('/') { present object } + end + + it 'uses the entity registered for a Struct' do + model = Struct.new(:name) + present_with(subject, model, model.new('x'), entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'uses the entity registered for an Enumerable model' do + model = Class.new do + include Enumerable + + def each(&) = [1, 2].each(&) + end + present_with(subject, model, model.new, entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'uses the entity registered for an object exposing #klass' do + model = Class.new do + def klass = String + end + present_with(subject, model, model.new, entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => model.name.to_s) + end + + it 'still resolves an array through its element class' do + model = Class.new + present_with(subject, model, [model.new], entity) + + get '/' + expect(JSON.parse(last_response.body)).to eq('presented' => 'Array') + end + + # A relation resolves through #klass, so #first must not be reached. + it 'does not call #first when the object resolves without it' do + model = Class.new + relation = Class.new do + def initialize(klass) = (@klass = klass) + attr_reader :klass + + def first = raise('#first should not have been called') + end + present_with(subject, model, relation.new(model), entity) + + expect { get '/' }.not_to raise_error + expect(last_response.status).to eq(200) + end + end end describe '.namespace' do From ea82c0952e909d0d926f550a70f7a6831482fd86 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 13:46:16 +0200 Subject: [PATCH 08/11] Render a redirect message as the plain text it claims to be #redirect announces its message as text/plain and has done since it was introduced in 2015 ("Redirect as plain text with optional message override"), but it only set the header. The body was still handed to the API's own formatter, so on a JSON API the sentence came back JSON-encoded: format :json get('/r') { redirect '/there' } Content-Type: text/plain "This resource has been moved temporarily to /there." quotes included -- neither valid plain text nor something a client reading the content type would expect. The existing specs missed it because they run on the default :txt format, where the formatter is a no-op. Set api.format alongside the header, the same lever an endpoint already has via #api_format, so the message is rendered by the txt formatter whatever the API declares. It is per-request env, so other routes on the same API are untouched. Co-Authored-By: Claude Opus 5 (cherry picked from commit 4e7d9d5c0255855e5660f219fd77a863176ded18) --- CHANGELOG.md | 1 + lib/grape/dsl/inside_route.rb | 5 +++++ spec/grape/endpoint_spec.rb | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e05abbb72..d2cb615da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,7 @@ * [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx). * [#2843](https://github.com/ruby-grape/grape/pull/2843): Let `rescue_from :grape_exceptions` take precedence over a catch-all registered as a class, so Grape errors keep their own status (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2844](https://github.com/ruby-grape/grape/pull/2844): Look an entity up by the presented object's own class before treating it as a collection, so `represent` is no longer skipped for models that respond to `#first` or `#klass` - [@ericproulx](https://github.com/ericproulx). +* [#2845](https://github.com/ruby-grape/grape/pull/2845): Render a `redirect` message as the plain text its content type announces, instead of letting the API's formatter re-encode it - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 2a48fed36..b46ff99b9 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -52,7 +52,12 @@ def redirect(url, permanent: false, body: nil) body_message ||= "This resource has been moved temporarily to #{url}." end header 'Location', url + # The message is plain text, so say so and render it as such. Setting + # only the header left the body to the API's own formatter, which on a + # JSON API returned the sentence wrapped in quotes under a text/plain + # content type. content_type 'text/plain' + api_format :txt body body_message end diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 17cadc2d3..6535b918b 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -667,6 +667,38 @@ def handle_argument_error get '/hey' expect(last_response.body).to eq 'test body' end + + # The message is announced as text/plain, so it has to be rendered as such + # whatever the API's own format is. Left to the JSON formatter it came back + # as a quoted JSON string under a text/plain content type. + context 'when the API declares a format of its own' do + before do + subject.format :json + subject.get('/hey') { redirect '/ha' } + end + + it 'renders the message as plain text' do + get '/hey' + + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain') + expect(last_response.body).to eq 'This resource has been moved temporarily to /ha.' + end + + it 'renders an overridden body as plain text too' do + subject.get('/there') { redirect '/ha', body: 'go away' } + + get '/there' + expect(last_response.body).to eq 'go away' + end + + it 'leaves the format of other routes alone' do + subject.get('/plain') { { a: 1 } } + + get '/plain' + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('application/json') + expect(last_response.body).to eq({ a: 1 }.to_json) + end + end end describe 'NameError' do From 28bbc530d5897451a7b095fb6920e6ed725bd2bd Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 13:52:02 +0200 Subject: [PATCH 09/11] 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 (cherry picked from commit 09abf2bdadb1f91cde7aebc706f5e92b25ed700a) --- CHANGELOG.md | 1 + lib/grape/request.rb | 28 ++++++++++++++++++++++++++-- spec/grape/request_spec.rb | 22 +++++++++++++++++++--- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2cb615da..607b91dd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ * [#2843](https://github.com/ruby-grape/grape/pull/2843): Let `rescue_from :grape_exceptions` take precedence over a catch-all registered as a class, so Grape errors keep their own status (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * [#2844](https://github.com/ruby-grape/grape/pull/2844): Look an entity up by the presented object's own class before treating it as a collection, so `represent` is no longer skipped for models that respond to `#first` or `#klass` - [@ericproulx](https://github.com/ericproulx). * [#2845](https://github.com/ruby-grape/grape/pull/2845): Render a `redirect` message as the plain text its content type announces, instead of letting the API's formatter re-encode it - [@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 From bf9510909d1f062684d50e717b3b79ef3cacbf98 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 14:07:08 +0200 Subject: [PATCH 10/11] Match Accept media types case-insensitively Media types are case-insensitive (RFC 9110 section 8.3.1), but the registered ones are spelled in lower case and matched literally, so a differently-cased Accept header found nothing: Accept: TEXT/PLAIN -> served application/json Accept: APPLICATION/VND.TWITTER-V1+JSON -> api.version nil Neither failed loudly. Content negotiation fell through to the default format, and header versioning behaved as though no version had been asked for, so the request was served by whichever version matched first -- the client quietly got something other than what it asked for. Three sites decided this, all comparing against lower-case registered types: the formatter's Accept lookup, MediaType.best_quality_media_type, and the vendor pattern in MediaType.parse / .match?. Down-case the incoming media type at each. The vendor pattern stays lower-case, which is the case a vendor and version are declared in and therefore compared in. Grape already treats media types this way when deciding whether to escape an error body (Middleware::Error#html_content_type?, from #2789). Co-Authored-By: Claude Opus 5 (cherry picked from commit 550dd628b16a92c5e2780b31952714b321870b55) --- CHANGELOG.md | 1 + lib/grape/middleware/formatter.rb | 6 +++++- lib/grape/util/media_type.rb | 13 ++++++++++--- spec/grape/api_spec.rb | 26 ++++++++++++++++++++++++++ spec/grape/util/media_type_spec.rb | 19 +++++++++++++++++++ 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 607b91dd9..9684d409c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ * [#2844](https://github.com/ruby-grape/grape/pull/2844): Look an entity up by the presented object's own class before treating it as a collection, so `represent` is no longer skipped for models that respond to `#first` or `#klass` - [@ericproulx](https://github.com/ericproulx). * [#2845](https://github.com/ruby-grape/grape/pull/2845): Render a `redirect` message as the plain text its content type announces, instead of letting the API's formatter re-encode it - [@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). +* [#2847](https://github.com/ruby-grape/grape/pull/2847): Match `Accept` media types case-insensitively, so a differently-cased header still negotiates the content type and resolves a vendor version - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 0eaf2c9cb..26336d1a1 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -156,11 +156,15 @@ def format_from_extension extension if content_type_for(extension) end + # Media types are case-insensitive (RFC 9110 §8.3.1) but the registered + # ones are spelled in lower case and Rack matches them literally, so an + # `Accept: TEXT/PLAIN` found nothing and fell through to the default + # format — the client quietly got something other than what it asked for. def format_from_header accept_header = try_scrub(env['HTTP_ACCEPT']) return if accept_header.blank? || accept_header == ALL_MEDIA_TYPES - media_type = Rack::Utils.best_q_match(accept_header, mime_types.keys) + media_type = Rack::Utils.best_q_match(accept_header.downcase, mime_types.keys) mime_types[media_type] if media_type end end diff --git a/lib/grape/util/media_type.rb b/lib/grape/util/media_type.rb index e93956b12..e4f041f19 100644 --- a/lib/grape/util/media_type.rb +++ b/lib/grape/util/media_type.rb @@ -7,6 +7,10 @@ class MediaType # based on the HTTP Accept header with the pattern: # application/vnd.:vendor-:version+:format + # + # Matched against a down-cased media type: they are case-insensitive + # (RFC 9110 §8.3.1), while a vendor and version are declared in the DSL + # in the case they will be compared in. VENDOR_VERSION_HEADER_REGEX = /\Avnd\.(?[a-z0-9.\-_!^]+?)(?:-(?[a-z0-9*.]+))?(?:\+(?[a-z0-9*\-.]+))?\z/ def initialize(type:, subtype:) @@ -41,7 +45,7 @@ def best_quality(header, available_media_types) def parse(media_type) return if media_type.blank? - type, subtype = media_type.split('/', 2) + type, subtype = media_type.downcase.split('/', 2) return if type.blank? || subtype.blank? new(type:, subtype:) @@ -50,14 +54,17 @@ def parse(media_type) def match?(media_type) return false if media_type.blank? - subtype = media_type.split('/', 2).last + subtype = media_type.downcase.split('/', 2).last return false if subtype.blank? VENDOR_VERSION_HEADER_REGEX.match?(subtype) end + # The available types are registered in lower case and Rack matches them + # literally, so the header has to be down-cased to be compared against + # them at all. def best_quality_media_type(header, available_media_types) - header.blank? ? available_media_types.first : Rack::Utils.best_q_match(header, available_media_types) + header.blank? ? available_media_types.first : Rack::Utils.best_q_match(header.downcase, available_media_types) end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index d84d611f0..9d36c991f 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4412,6 +4412,32 @@ def my_method end end + # Media types are case-insensitive (RFC 9110 §8.3.1). The registered ones are + # spelled in lower case and matched literally, so a differently-cased Accept + # used to find nothing: content negotiation fell through to the default format + # and header versioning behaved as though no version had been asked for. + describe 'a differently-cased Accept header' do + it 'still negotiates the content type' do + subject.content_type :json, 'application/json' + subject.content_type :txt, 'text/plain' + subject.default_format :json + subject.get('/x') { { a: 1 } } + + get '/x', {}, 'HTTP_ACCEPT' => 'TEXT/PLAIN' + expect(last_response.headers[Rack::CONTENT_TYPE]).to eq('text/plain') + end + + it 'still resolves the version of a vendor media type' do + subject.version 'v1', using: :header, vendor: 'twitter' + subject.format :json + subject.get('/x') { env[Grape::Env::API_VERSION] } + + get '/x', {}, 'HTTP_ACCEPT' => 'APPLICATION/VND.TWITTER-V1+JSON' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('v1'.to_json) + end + end + describe '.format' do context ':txt' do before do diff --git a/spec/grape/util/media_type_spec.rb b/spec/grape/util/media_type_spec.rb index 09e328513..ae2456f93 100644 --- a/spec/grape/util/media_type_spec.rb +++ b/spec/grape/util/media_type_spec.rb @@ -41,6 +41,25 @@ it_behaves_like 'MediaType' end end + + # Media types are case-insensitive (RFC 9110 §8.3.1); the vendor pattern is + # written in lower case, so anything else used to parse as no vendor at all. + context 'when the header is not in lower case' do + subject(:media_type) { described_class.parse(header) } + + let(:header) { 'APPLICATION/VND.TEST-V1+JSON' } + + it 'parses the vendor, version and format' do + expect(media_type.vendor).to eq('test') + expect(media_type.version).to eq('v1') + expect(media_type.format).to eq('json') + end + + it 'down-cases the type and subtype' do + expect(media_type.type).to eq('application') + expect(media_type.subtype).to eq('vnd.test-v1+json') + end + end end describe '.match?' do From 07516ee024e7547faf294fcb160e8a0f8d560047 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sat, 1 Aug 2026 14:52:22 +0200 Subject: [PATCH 11/11] Remove http_digest Nothing it could reach has existed since 2.0.0. #2361 removed Rack::Auth::Digest and Grape's :http_digest strategy after Rack 3 dropped digest authentication, but Grape::Middleware::Auth::DSL#http_digest survived and kept recording its settings, so an API declaring it still booted -- and then raised Grape::Exceptions::UnknownAuthStrategy on the first request, from inside the middleware build, as an uncaught exception rather than a response. A misconfiguration only visible in production. The 4.0 UPGRADING notes still used `auth :http_digest, realm: 'API', opaque: 'secret'` as a worked example of a supported call, so the documentation pointed at it too. That section is rewritten, and the removal documented. Removing the sugar rather than validating the strategy when `auth` is called: #auth deliberately records whatever it is given and resolves the strategy when the middleware is built, which is what lets an application register its own. That contract is specified -- validating early breaks it, and would break registering a strategy after the API class is defined. `auth :http_digest` therefore still works for anyone who registered one; only the sugar is gone, along with the two defaults it supplied (realm 'API Authorization', opaque 'secret'), which UPGRADING spells out. The DSL specs used :http_digest as their example label for #auth itself. They now use a neutral :custom, which keeps the distinction the specs are actually about: #auth records a label, the strategy behind it is looked up later. Co-Authored-By: Claude Opus 5 (cherry picked from commit 2e12a20a2b8e25894f86fda3b5db8a0c91c09fd2) --- CHANGELOG.md | 1 + UPGRADING.md | 30 +++++++++++++++---- lib/grape/middleware/auth/dsl.rb | 13 -------- spec/grape/middleware/auth/dsl_spec.rb | 41 +++++++------------------- 4 files changed, 36 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9684d409c..ce3e03407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ * [#2837](https://github.com/ruby-grape/grape/pull/2837): Document the definition-time rejection of an `Array`/`Set` of an uncoercible element type introduced by #2817, and pin it with specs - [@ericproulx](https://github.com/ericproulx). * [#2836](https://github.com/ruby-grape/grape/pull/2836): Define `#hash` alongside the `eql?`/`==` pairs on `Grape::Endpoint`, `Grape::Util::InheritableSetting`, `Grape::Middleware::Stack::Middleware`, `Grape::ServeStream::StreamResponse` and `Grape::ServeStream::FileBody`, so equal objects hash alike in a `Hash`, `Set` or `uniq` - [@ericproulx](https://github.com/ericproulx). * [#2835](https://github.com/ruby-grape/grape/pull/2835): Return the compiled instance from `Grape::API::Instance.compile!` so `call` and `recognize_path` no longer re-read `@instance`, which a concurrent `change!` could nil between the two reads - [@ericproulx](https://github.com/ericproulx). +* [#2849](https://github.com/ruby-grape/grape/pull/2849): Remove `http_digest`, which has had no strategy behind it since 2.0.0 and raised on the first request rather than when the API was defined (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/UPGRADING.md b/UPGRADING.md index cc505a2db..f0b9cb076 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -421,16 +421,16 @@ rescue_from MyError, with: :other_handler Calls that only use one meta selector or only use exception classes (the documented forms) are unaffected. -#### `auth`, `http_basic` and `http_digest` now take keyword arguments +#### `auth` and `http_basic` now take keyword arguments -`Grape::Middleware::Auth::DSL#auth`, `#http_basic` and `#http_digest` now accept their options as keyword arguments instead of a positional `Hash`. Calls using bare keyword syntax or a block are unaffected: +`Grape::Middleware::Auth::DSL#auth` and `#http_basic` now accept their options as keyword arguments instead of a positional `Hash`. Calls using bare keyword syntax or a block are unaffected: ```ruby http_basic realm: 'API' do |u, p| # ... end -auth :http_digest, realm: 'API', opaque: 'secret', &proc +auth :my_strategy, realm: 'API', &proc ``` Passing a positional options `Hash` still works but is deprecated and will be removed in a future release: @@ -438,13 +438,33 @@ Passing a positional options `Hash` still works but is deprecated and will be re ```ruby # deprecated http_basic({ realm: 'API' }) -auth :http_digest, { realm: 'API', opaque: 'secret' } +auth :my_strategy, { realm: 'API' } # preferred http_basic(realm: 'API') -auth :http_digest, realm: 'API', opaque: 'secret' +auth :my_strategy, realm: 'API' ``` +#### `http_digest` is removed + +`Grape::Middleware::Auth::DSL#http_digest` is gone. Calling it now raises `NoMethodError` while the API class is being defined. + +Nothing it could reach has existed since **2.0.0**, which removed `Rack::Auth::Digest` along with Grape's `:http_digest` strategy ([#2361](https://github.com/ruby-grape/grape/pull/2361)) after Rack 3 dropped digest authentication. The method survived that removal and kept recording its settings happily, so an API declaring `http_digest` still booted — and then raised `Grape::Exceptions::UnknownAuthStrategy` on the *first request*, from inside the middleware build, as an uncaught exception rather than a response. Failing while the class is defined is the point of removing it. + +**If you registered your own `:http_digest` strategy**, it still works; call `auth` directly: + +```ruby +Grape::Middleware::Auth::Strategies.add(:http_digest, MyDigestStrategy, ->(settings) { [settings[:realm]] }) + +class API < Grape::API + auth :http_digest, realm: 'API Authorization', opaque: 'secret' do |username| + # ... + end +end +``` + +The removed method supplied two defaults that `auth` does not, so pass them explicitly if you were relying on them: `realm` defaulted to `'API Authorization'`, and `opaque` to `'secret'` (nested inside `realm` when `realm` was itself a Hash). + #### Middleware options now route through per-class `Options` `Data` value objects `Grape::Middleware::Error`, `Grape::Middleware::Formatter`, and `Grape::Middleware::Versioner::Base` each declare an `Options` `Data.define` and route their `**options` kwargs through it on `initialize`. This means **unknown kwargs now raise `ArgumentError`** instead of being silently swallowed: diff --git a/lib/grape/middleware/auth/dsl.rb b/lib/grape/middleware/auth/dsl.rb index 52829cd22..5c62673c0 100644 --- a/lib/grape/middleware/auth/dsl.rb +++ b/lib/grape/middleware/auth/dsl.rb @@ -22,19 +22,6 @@ def http_basic(*legacy_options, **options, &) auth(:http_basic, **options, &) end - def http_digest(*legacy_options, **options, &) - options = merge_legacy_auth_options(:http_digest, legacy_options, options) - options[:realm] ||= 'API Authorization' - - if options[:realm].respond_to?(:values_at) - options[:realm][:opaque] ||= 'secret' - else - options[:opaque] ||= 'secret' - end - - auth(:http_digest, **options, &) - end - private # @deprecated Passing a positional options Hash is deprecated; pass diff --git a/spec/grape/middleware/auth/dsl_spec.rb b/spec/grape/middleware/auth/dsl_spec.rb index 612c511c8..29d9e2775 100644 --- a/spec/grape/middleware/auth/dsl_spec.rb +++ b/spec/grape/middleware/auth/dsl_spec.rb @@ -4,12 +4,15 @@ subject { Class.new(Grape::API) } let(:block) { -> {} } + # #auth records whatever it is given; the strategy behind the label is looked + # up when the middleware is built. A label with no built-in sugar method keeps + # that distinction visible. let(:settings) do { opaque: 'secret', proc: block, realm: 'API Authorization', - type: :http_digest + type: :custom } end @@ -17,7 +20,7 @@ it 'sets auth parameters' do expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings) - subject.auth :http_digest, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] + subject.auth :custom, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] expect(subject.auth).to eq(settings) end @@ -25,10 +28,10 @@ expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings) expect(subject.base_instance).to receive(:use).with(Grape::Middleware::Auth::Base, settings.merge(realm: 'super_secret')) - subject.auth :http_digest, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] + subject.auth :custom, realm: settings[:realm], opaque: settings[:opaque], &settings[:proc] first_settings = subject.auth - subject.auth :http_digest, realm: 'super_secret', opaque: settings[:opaque], &settings[:proc] + subject.auth :custom, realm: 'super_secret', opaque: settings[:opaque], &settings[:proc] expect(subject.auth).to eq(settings.merge(realm: 'super_secret')) expect(subject.auth.object_id).not_to eq(first_settings.object_id) @@ -42,29 +45,13 @@ end end - describe '.http_digest' do - context 'when realm is a hash' do - it 'sets auth parameters' do - subject.http_digest realm: { realm: 'my_realm', opaque: 'my_opaque' }, &settings[:proc] - expect(subject.auth).to eq(realm: { realm: 'my_realm', opaque: 'my_opaque' }, type: :http_digest, proc: block) - end - end - - context 'when realm is not hash' do - it 'sets auth parameters' do - subject.http_digest realm: 'my_realm', opaque: 'my_opaque', &settings[:proc] - expect(subject.auth).to eq(realm: 'my_realm', type: :http_digest, proc: block, opaque: 'my_opaque') - end - end - end - describe 'deprecated positional options Hash' do it 'deprecates a positional Hash for `auth` but still works when silenced' do - expect { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block } + expect { subject.auth :custom, { realm: 'r', opaque: 'o' }, &block } .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `auth`/) - Grape.deprecator.silence { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block } - expect(subject.auth).to eq(realm: 'r', opaque: 'o', type: :http_digest, proc: block) + Grape.deprecator.silence { subject.auth :custom, { realm: 'r', opaque: 'o' }, &block } + expect(subject.auth).to eq(realm: 'r', opaque: 'o', type: :custom, proc: block) end it 'deprecates a positional Hash for `http_basic` but still works when silenced' do @@ -74,13 +61,5 @@ Grape.deprecator.silence { subject.http_basic({ realm: 'my_realm' }, &block) } expect(subject.auth).to eq(realm: 'my_realm', type: :http_basic, proc: block) end - - it 'deprecates a positional Hash for `http_digest` but still works when silenced' do - expect { subject.http_digest({ realm: 'my_realm' }, &block) } - .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `http_digest`/) - - Grape.deprecator.silence { subject.http_digest({ realm: 'my_realm' }, &block) } - expect(subject.auth).to eq(realm: 'my_realm', opaque: 'secret', type: :http_digest, proc: block) - end end end