diff --git a/CHANGELOG.md b/CHANGELOG.md index ca46339de..ba8a57bc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ * [#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 - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.4 (2026-07-25) diff --git a/lib/grape/validations/attributes_iterator.rb b/lib/grape/validations/attributes_iterator.rb index 578ca3fab..4e717a6f6 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,7 +29,12 @@ 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 diff --git a/lib/grape/validations/params_scope.rb b/lib/grape/validations/params_scope.rb index dcfd44095..0837220df 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 @@ -324,6 +327,14 @@ def find_nearest_array_ancestor scope end + # Every Array-typed 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 + (@type == Array ? 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