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

[Serializer] Handle type error constructing input #51003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
from
Open

[Serializer] Handle type error constructing input #51003

wants to merge 1 commit into from

Conversation

kylekatarnls
Copy link
Contributor
@kylekatarnls kylekatarnls commented Jul 17, 2023 •
edited
Loading
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
Victor-Truhanovich
Victor-Truhanovich reviewed Oct 30, 2023
View reviewed changes
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php
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
@Wolg Wolg mentioned this pull request Jan 8, 2024
Closed
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Victor-Truhanovich Victor-Truhanovich Victor-Truhanovich left review comments

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Feature Status: Needs Review
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

MapQueryString & MapRequestPayload empty when type missmatch
6 participants
@kylekatarnls @Victor-Truhanovich @fabpot @nicolas-grekas @xabbuh @carsonbot
0