Skip to content

fix: an array parameter with a bad element is refused, not a 500 - #938

Merged
blaipr merged 1 commit into
mainfrom
fix/an-array-parameter-with-a-bad-element-is-refused-cleanly
Sep 24, 2026
Merged

blaipr merged 1 commit into
mainfrom
fix/an-array-parameter-with-a-bad-element-is-refused-cleanly

Conversation

@blaipr

@blaipr blaipr commented Sep 24, 2026

Copy link
Copy Markdown
Member

Filter::getArray() picked a filter per element with is_numeric() and handed the element straight
on. Under strict types getInt() accepts only int|string and getString() only ?string, so a
bool, a float, a nested array or an object threw a TypeError:

[true] -> TypeError: Filter::getString(): Argument #1 ($value) must be of type ?string
[1.5]  -> TypeError: Filter::getInt(): Argument #1 ($value) must be of type string|int
[[1]]  -> TypeError: Filter::getString(): Argument #1 ($value) must be of type ?string

Nothing caught it, and the API's catch-all answers a TypeError as a 500 with the class, the
method and the server's absolute path in the body
— the leak that getParamInt(),
getParamString() and getParamRaw() were already fixed to refuse with a 400. getParamArray() was
the one reader ParameterTypesTest called "always right". It was right about the container
(is_array()) and never looked inside it.

Both doors reach it

  • API — getParamArray() → Filter::getArray(). Sending {"tagsId": [true]} to account create
    was enough.
  • Web — Request::analyzeArray() → Filter::getArray(). It reads through InputBag::all(),
    which skips Symfony's own scalar check, so a form field named other_users_view[x][] makes one
    element an array from an ordinary authenticated submission.

The change, in both places it belongs

  • Filter::getArray() decides by type rather than by is_numeric() alone, and answers an
    element it cannot represent as null, the same as a missing one. This is the shared point both
    doors meet, so neither can 500 from it again.
  • Api::getParamArray() also checks the elements, and refuses a bad one with the same
    Wrong parameters 400 as the scalar readers — so the API keeps its documented contract rather
    than silently dropping what a caller sent.

Tests

  • FilterTest is new — Filter had no unit test at all. A bool, a float, a nested array and an
    object each come back as null; ids and text still come through.
  • ParameterTypesTest gains anArrayParameterWithABadElementIsRefused over the same four shapes,
    plus a good id beside a bad one.

Mutation-verified in both directions: reverting src/ fails all four FilterTest cases with the
TypeError, and all four API cases with a non-400.

Filter::getArray() picked a filter per element with is_numeric() and passed the element on,
but under strict types getInt() accepts only int|string and getString() only ?string. So a
bool, a float, a nested array or an object threw a TypeError that nothing caught, and the
API's catch-all answered it as a 500 with the class, the method and the server's absolute
path in the body — the leak the scalar readers were already fixed to refuse with a 400.
getParamArray() checked is_array() and never looked inside.

Both doors reach it: the API through getParamArray(), and the web through
Request::analyzeArray(), which reads via InputBag::all() and so skips Symfony's own scalar
check — a form field named x[a][] makes one element an array.

Filter::getArray() now decides by type and answers an element it cannot represent as null,
at the point both doors share; and getParamArray() also checks the elements and refuses a
bad one with the same 'Wrong parameters' 400 as the scalar readers, so the API keeps its
documented contract rather than silently dropping what a caller sent.
@blaipr
blaipr merged commit 261e35f into main Sep 24, 2026
8 checks passed
@blaipr
blaipr deleted the fix/an-array-parameter-with-a-bad-element-is-refused-cleanly branch September 24, 2026 13:25
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.

1 participant