Skip to content

Reject an element that is not a Hash in an Array param given a block - #2957

Open
ericproulx wants to merge 1 commit into
masterfrom
fix/array-scope-non-hash-elements
Open

ericproulx wants to merge 1 commit into
masterfrom
fix/array-scope-non-hash-elements

Conversation

@ericproulx

Copy link
Copy Markdown
Contributor

Summary

The block of requires :items, type: Array do ... end declares the keys of each element, but nothing checks that an element has keys at all. A non-Hash element only fails where the block requires a key of it, so a block of optional params lets a String, a number or a nested Array through to the endpoint:

optional :lines, type: Array do
  optional :sku, type: String
  optional :qty, type: Integer
end
# {"lines":["x", 5, [{"sku":"a"}]]}  => 201; `params[:lines].sum { |l| l[:qty] }` raises TypeError

#2838 stopped the iterator from unwrapping an Array nested deeper than the declaration, and relied on those required keys to fail it. When the nested scope's params are optional there are none. Against an Array scope nested in another, {"orders":[[{"lines":5}]]} therefore passes validation (3.3.5 answered 400), and declared then raises NoMethodError calling any? on the 5. Bisected to #2838.

This PR makes two changes:

  • Element check. ParamsScope registers a HashElementsValidator for every type: Array param given a block. It answers items[1] is invalid for the first element that is neither a Hash nor blank. Blank elements (nil, '', false, [], {}) are still passed over, the same way the block's own validators skip an optional scope whose elements are all blank. That keeps the existing "doesn't validate the group when every element is blank" spec. Array[JSON] already checks its elements when it parses them, so it's left out.
  • declared. It stops raising on a value that is neither a Hash nor an Array where nested params are declared, and returns it as it came in, as Return the elements of an Array params scope that are not a Hash from declared #2935 does for such an element. Validation no longer lets one through an Array, but a given whose dependency isn't met still leaves its params unvalidated. So {"address":"somewhere"} against a Hash scope inside such a given raised the same NoMethodError, on 3.3.5 and every release since.
body master this PR
{"lines":["x",5]} (all-optional block) 201 400 lines[0] is invalid
{"orders":[[{"lines":5}]]} (nested, optional) 500 (declared) 400 orders[0] is invalid
{"orders":[{"lines":[{"sku":"a"},"b"]}]} 201 400 orders[0][lines][1] is invalid
{"lines":[" ",false,{"sku":"a"}]} 201 201
unmet given, {"address":"somewhere"} 500 (declared) 201, returned as sent

Backward compatibility

This is stricter validation, as discussed, so UPGRADING has an entry. Where a block does require a key, the element's own error now comes first (items[0] is invalid, items[0][key] is missing). Three existing specs pinned the old messages and are updated. #2935's declared example used 'x' and 1, which are now rejected, so it now uses the blank elements validation still lets through.

Like every validator, the new class registers under its short name (hash_elements), so requires :x, type: Array, hash_elements: true would work as an undocumented option. I kept it a Base subclass so it gets the iterator (nested arrays, element indices) for free. The alternative is a standalone object like ContractScopeValidator, which would have to re-implement that. Happy to switch if you'd rather not have the name registered.

Verification

  • Mutation-checked: without the registration, 6 examples fail; with master's declared, the new given example fails; with the narrower "empty" test in place of blank?, the blank-element examples fail.
  • Replayed 95k fuzzed requests from random params schemas against the earlier master run: the two declared crashes are gone, and every request that went from 201 to 400 (61) holds a non-blank, non-Hash element at the reported index.

Test plan

  • Full RSpec suite passes locally (2916 examples).
  • RuboCop clean.
  • CI green.

🤖 Generated with Claude Code

@ericproulx
ericproulx force-pushed the fix/array-scope-non-hash-elements branch from a770ed6 to 5362c06 Compare September 18, 2026 10:03
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown

Danger Report

No issues found.

View run

@ericproulx
ericproulx force-pushed the fix/array-scope-non-hash-elements branch from 5362c06 to 5c70fae Compare September 18, 2026 10:24
The block of `requires :items, type: Array do ... end` declares the keys of
each element, but nothing checked that an element had keys at all. One
that was not a Hash only failed where the block required a key of it --
`items[0][key] is missing` -- so a block of optional params let a String, a
number or a nested Array straight through to the endpoint:

    optional :lines, type: Array do
      optional :sku, type: String
      optional :qty, type: Integer
    end
    # {"lines":["x", 5, [{"sku":"a"}]]}  => passes; lines.sum { _1[:qty] } raises

#2838 stopped the iterator unwrapping an Array nested deeper than the
declaration, and relied on those required keys to fail it. Where the
nested scope's params are optional there are none, so
`{"orders":[[{"lines":5}]]}` against an Array scope nested in another now
passed validation -- 3.3.5 had answered 400 -- and `declared` then raised
NoMethodError calling `any?` on the 5.

ParamsScope now registers a HashElementsValidator for every `type: Array`
param given a block, which answers `items[1] is invalid` for the first
element that is neither a Hash nor blank. Blank elements are still passed
over, as the block's own validators pass over an optional scope whose
elements are all blank. `Array[JSON]` already checks its elements when it
parses them.

`declared` also stops raising on a value that is neither a Hash nor an
Array where nested params are declared, and returns it as it came in, as
#2935 made it do for such an element. Validation no longer lets one reach
it through an Array, but a `given` whose dependency is not met still leaves
its params unvalidated, and `{"address":"somewhere"}` against a Hash scope
inside one raised the same NoMethodError on 3.3.5 and since.

UPGRADING covers the stricter validation and the element error that now
leads a required key's.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@ericproulx
ericproulx requested a review from dblock September 18, 2026 12:34
@ericproulx
ericproulx force-pushed the fix/array-scope-non-hash-elements branch from 5c70fae to 83daa2e Compare September 18, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants