-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Serializer] Fix denormalizing empty string into object|null parameter
#52172
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.
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
Conversation
object|null parameter
|
Looks like a bugfix to me, in this case 🎯 |
|
This would need tests for sure. But I'm no expert in the Serializer component so I'd appreciate if someone more familiar with it could review. |
|
I've added tests. I also discovered that |
|
Another interesting failure scenario: // this fails
$this->serializer->denormalize(['element' => '0'], Example::class, context: [AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true]);
class Example {
public function __construct(public Args|int $element) {}
}
class Args {
public function __construct(public Uuid $nested, private string $test)
}
// [Symfony\Component\Serializer\Exception\MissingConstructorArgumentsException]
// Cannot create an instance of "App\Args" from serialized data because its constructor requires the following parameters to be present : "$nested", "$test".
// whereas this returns an instance succesfully
$this->serializer->denormalize(['element' => '0'], Test::class, context: [AbstractObjectNormalizer::DISABLE_TYPE_ENFORCEMENT => true]);
class Test {
public function __construct(public Uuid|int $element)
}Because the first denormalization throws I'm not sure what the reasoning was here or if this is actually not intended. It could be fixed by adding |
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
Show resolved
Hide resolved
|
Let's resume this one? Is this a bugfix as it's documented in the PR description? If yes can you please target the lowest affected branch? |
940e9ca to
6556d98
Compare
|
Applied feedback and targeting |
|
Isn't 5.4 affected also? |
85ca4e0 to
3057b6a
Compare
3057b6a to
55c11e9
Compare
|
Eh yes, I was looking into the wrong part when checking that branch. Now based on 5.4. |
55c11e9 to
4901953
Compare
|
Parse error on PHP 7.2 ;) |
1a725d3 to
1df027a
Compare
|
Serializer tests are ok now |
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
Outdated
Show resolved
Hide resolved
1df027a to
64eaf96
Compare
|
(rebase needed ;) ) |
64eaf96 to
62f2203
Compare
|
Thank you @Jeroeny. |
This PR was merged into the 5.4 branch. Discussion ---------- [Serializer] fix nullable int cannot be serialized | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT Hello, previous to [this PR](#52172) such XML could be deserialized correctly, setting null in the value: ```xml <?xml version="1.0" encoding="UTF-8"?> <DummyNullableInt> <value/> </DummyNullableInt> ``` ```php class DummyNullableInt { public function __construct( public int|null $value = null ) {} } ``` but now it creates the following error: ``` Uninitialized string offset 0 /home/nikophil/works/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:495 /home/nikophil/works/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:630 /home/nikophil/works/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php:377 /home/nikophil/works/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:246 /home/nikophil/works/symfony/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php:346 ``` I looked for any issue or PR mentioning this problem, but couldn't find it. So here is a fix ping `@Jeroeny` Commits ------- 5d62dea [Serializer] fix regression where nullable int cannot be serialized
The
AbstractObjectNormalizerhandles this for bool, int and float types. But if a parameter is typedObject|null, a serialization and then deserialization will fail.This will throw:
Reproducer: https://github.com/Jeroeny/reproduce/blob/xmlnull/src/Test.php