8000 [Serializer] Handle type error constructing input by kylekatarnls · Pull Request #51003 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Conversation

kylekatarnls
Copy link
Contributor
@kylekatarnls kylekatarnls commented Jul 17, 2023
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Issues? Fix #50904
License MIT

When mapping a input to an object, for instance with:

public function myApiRoute(#[MapQueryString] MyInput $input)
{
}

And MyInput has an hard-type:

class MyInput
{
    public function __construct(
        public readonly ?string $id = null,
    ) {
    }
}

Then you try to call this route with invalid type: /api/endpoint?id[]=abc

The construction of MyInput will fail with a TypeError MyInput::__construct(): Argument #1 ($id) must be of type ?string, array given.

But this is caught in AbstractNormalizer then you get a 500 error (unhandled) Typed property MyInput::id must not be accessed before initialization.

With this PR, I propose to handle it instead as a 400 error (bad request) The type of the "id" parameter for class "MyInput" must be of type "?string" ("array" given).

@kylekatarnls kylekatarnls requested a review from dunglas as a code owner July 17, 2023 15:19
@carsonbot carsonbot added this to the 6.4 milestone Jul 17, 2023
@kylekatarnls kylekatarnls changed the title [Serialize] Handle type error constructing input [Serializer] Handle type error constructing input Jul 17, 2023
Comment on lines +430 to +447
if (preg_match('/#\d+ \(\$([^)]+)\) must be of type ([^,]+), ([^,]+) given/', $message, $match)) {
$message = sprintf(
'The type of the "%s" parameter for class "%s" must be of type "%s" ("%s" given).',
$match[1],
$class,
$match[2],
$match[3],
);
}

$exception = NotNormalizableValueException::createForUnexpectedDataType(
$message,
$data,
[$class],
$context['deserialization_path'] ?? null,
true,
previous: $e,
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it would be better this way

Suggested change
if (preg_match('/#\d+ \(\$([^)]+)\) must be of type ([^,]+), ([^,]+) given/', $message, $match)) {
$message = sprintf(
'The type of the "%s" parameter for class "%s" must be of type "%s" ("%s" given).',
$match[1],
$class,
$match[2],
$match[3],
);
}
$exception = NotNormalizableValueException::createForUnexpectedDataType(
$message,
$data,
[$class],
$context['deserialization_path'] ?? null,
true,
previous: $e,
);
$type = $class;
if (preg_match('/#\d+ \(\$([^)]+)\) must be of type ([^,]+), ([^,]+) given/', $message, $match)) {
$message = sprintf(
'The type of the "%s" parameter for class "%s" must be of type "%s" ("%s" given).',
$match[1],
$class,
$match[2],
$match[3],
);
$type = $match[2];
}
$exception = NotNormalizableValueException::createForUnexpectedDataType(
$message,
$data,
[$type],
$context['deserialization_path'] ?? null,
true,
previous: $e,
);

@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MapQueryString & MapRequestPayload empty when type missmatch

6 participants

0