-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] Exclude non-initialized properties accessed with getters #38900
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
[Serializer] Exclude non-initialized properties accessed with getters #38900
Conversation
I'm not sure the failed |
src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php
Outdated
Show resolved
Hide resolved
It looks like we have a similar issue in API Platform: api-platform/core#3974 |
PR welcome on PropertyAccess if someone is up to having a look there. |
Thank you @BoShurik. |
…getters (julienfalque) This PR was merged into the 4.4 branch. Discussion ---------- [Serializer] Prevent access to private properties without getters | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When upgrading `symfony/serializer` from `v5.2.1` to `v5.2.2`, the serializer starts throwing exceptions because it cannot access some private properties that don't have a getter. This looks related to #38900. Commits ------- f0409b4 [Serializer] Prevent access to private properties without getters
Hey guys, I'm having an issue with this PR, since it's been merged, I'm receiving the following error from the Property Accessor. I've searched through the whole project and could not find the methods described in the error, what I can find is the domainEvents property, which is included in a Trait being used by the class "App\Domain\Shift\Rating". Does anybody have any insights on this issue or has had any similar issues with this update? I've restricted the Serializer package to version 4.4.18 and this has prevented the problem, but I would like to find a better solution. Thanks!! code: 500 |
…essed with getters (BoShurik) This PR was merged into the 4.4 branch. Discussion ---------- [Serializer] Exclude non-initialized properties accessed with getters | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | no <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Allow to serialize ```php final class Php74DummyPrivate { private string $uninitializedProperty; private string $initializedProperty = 'defaultValue'; public function getUninitializedProperty(): string { return $this->uninitializedProperty; } public function getInitializedProperty(): string { return $this->initializedProperty; } } ``` Similar to symfony#34791 Commits ------- da91003 Exclude non-initialized properties accessed with getters
…(ivannemets-sravniru) This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [Serializer] #36594 attributes cache breaks normalization | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36594 | License | MIT | Doc PR | symfony/symfony-docs#15823 The bug itself is explained in the following (simplified) example: ```php class Dummy { public array $requiredData; // supposed to be set for any object public array $optionalData; // can be not initialized (and if so - ignored by serializer) } $object1 = new Dummy(); $object1->requiredData = ['username' => 'foo']; $json1 = $serializer->serialize($object1, 'json'); // {"requiredData": {"username": "foo"}} // at this point object normalizer has already cached attributes for Dummy::class and context, // now it contains array ['requiredData'] - optionalData has been ignored as it's unitialized // then, while the script is still running, we have another object of the same class with optionalData set $object2 = new Dummy(); $object2->requiredData = ['username' => 'bar']; $object2->optionalData = ['email' => 'bar@test.com']; $json2 = $serializer->serialize($object2, 'json'); // expected: {"requiredData": {"username": "bar"}, "optionalData": {"email": "bar@test.com"}} // actual: {"requiredData": {"username": "bar"}} // here normalizer has no clue about optionalData attribute since it reuses attributes cached // in \Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::$attributesCache // while normalizing the first object ``` Though this PR created for 5.4 branch, it actually fixes a bug reproducible in 5.3. The reason why I use 5.4 for this fix is that 5.4 introduces a [new feature](symfony/symfony-docs#15823) related to the same problem. If this PR gets approved and merged, I can potentially implement the same fix for 5.3, but `SKIP_UNINITIALIZED_VALUES` will cause some merge conflicts in the future.. As of v 5.3 symfony ignores uninitialized properties by default in `ObjectNormalizer::extractAttributes` and `PropertyNormalizer::extractAttributes` (implemented in #38900 and #34791) but this approach is wrong - **`extractAttributes` method MUST return the same attributes list for any instance of the same class**, otherwise cached attributes do not match actual attributes list for different instances having different set of initialized/uninitialized properties So, this PR does a few things: 1. removes ignoring attributes from all built-in implementations of `\Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::extractAttributes` so that even uninitialized attributes will be cached by normalizer. Instead, we ignore uninitialized attributes when trying to access them while calling `\Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer::getAttributeValue` 2. sets `true` as the default value for `SKIP_UNINITIALIZED_VALUES` context setting as I believe this is the expected default behavior for any built-in object normalizer 3. makes `SKIP_UNINITIALIZED_VALUES` compatible with not just `ObjectNormalizer`, but with two other built-in normalizers `PropertyNormalizer` and `GetSetMethodNormalizer` as well Commits ------- 55818c3 [Serializer] #36594 attributes cache breaks normalization
Allow to serialize
Similar to #34791