The Wayback Machine - https://web.archive.org/web/20220809171657/https://github.com/symfony/symfony/pull/46958
Skip to content
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] Ignore getter with required parameters (Fix #46592) #46958

Merged
merged 1 commit into from Jul 19, 2022

Conversation

astepin
Copy link
Contributor

@astepin astepin commented Jul 16, 2022

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #46592
License MIT
Doc PR na

If no Ignore annotation is used, the attributes for serialization are obtained using Symfony\Component\Serializer\Normalizer\ObjectNormalizer::extractAttributes.
There it is checked if the method has required parameters and if yes, the method is ignored.

However, if you use the Ignore annotation, the attributes are determined with a different method. Here I have adapted at least for get methods the behavior as it was before.

If someone serialized a class with Ignore annotations before, he got here \Symfony\Component\PropertyAccess\PropertyAccessor::readProperty an exception as written in ticket #46592. With this fix the methods are ignored and there is no exception anymore.

@carsonbot

This comment was marked as outdated.

@astepin astepin marked this pull request as ready for review Jul 16, 2022
@astepin astepin requested a review from dunglas as a code owner Jul 16, 2022
@carsonbot carsonbot added this to the 5.4 milestone Jul 16, 2022
@carsonbot

This comment was marked as outdated.

mtarld
mtarld approved these changes Jul 18, 2022
fabpot
fabpot approved these changes Jul 19, 2022
@fabpot
Copy link
Member

@fabpot fabpot commented Jul 19, 2022

Thank you @astepin.

@fabpot fabpot merged commit c89721e into symfony:5.4 Jul 19, 2022
8 of 11 checks passed
@astepin astepin deleted the fix_46592 branch Jul 19, 2022
@@ -100,6 +100,11 @@ public function loadClassMetadata(ClassMetadataInterface $classMetadata)
continue;
}

$getAccessor = preg_match('/^(get|)(.+)$/i', $method->name);
Copy link
Contributor

@guilliamxavier guilliamxavier Jul 20, 2022

Choose a reason for hiding this comment

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

This regex looks "fishy": ignoring capturing groups (as matches are not used), it's equivalent to /^(get)?.+$/i i.e. just /^.+$/ i.e. will match any method (not only getters)

@fabpot fabpot mentioned this pull request Jul 29, 2022
This was referenced Jul 29, 2022
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.

None yet

6 participants