Skip to content

Conversation

@santysisi
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #61552
License MIT

Description

This PR resolves a crash in the Serializer when deserializing an object with a protected(set) or private(set) property

@santysisi santysisi requested a review from dunglas as a code owner September 1, 2025 02:15
@carsonbot carsonbot added this to the 6.4 milestone Sep 1, 2025
@santysisi santysisi force-pushed the fix/serializer-asymmetric-visibility-error branch 3 times, most recently from 559a53e to a91d37e Compare September 1, 2025 03:30
@santysisi santysisi changed the title [Serializer] Fix serializer crash due to asymmetric visibility on protected(set) properties [Serializer] Fix serializer crash due to asymmetric visibility on protected(set) or private(set) properties Sep 1, 2025
@santysisi
Copy link
Contributor Author

I think (though I might be wrong) that the pipeline errors are not relevant in the context of this PR.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

thanks for opening this!

@santysisi santysisi force-pushed the fix/serializer-asymmetric-visibility-error branch from a91d37e to 7407d03 Compare September 1, 2025 08:12
@santysisi
Copy link
Contributor Author

Hi 😊 I hope you're doing well!
Thank you so much for your comments ❤️
I've made the changes, please let me know if anything else needs to be adjusted 😄

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

On closer review, this makes me think we also need to fix the call to isReadOnly in ReflectionExtractor on L365 - we might need to check for asym there also. Can you have a look?

@santysisi santysisi force-pushed the fix/serializer-asymmetric-visibility-error branch from 7407d03 to 81f3a04 Compare September 1, 2025 11:47
@santysisi
Copy link
Contributor Author

santysisi commented Sep 1, 2025

On closer review, this makes me think we also need to fix the call to isReadOnly in ReflectionExtractor on L365 - we might need to check for asym there also. Can you have a look?

Thanks for pointing that out! Maybe I’m wrong, but I think it might not be necessary to update the call to isReadOnly on L365, since the isAllowedProperty method is executed beforehand and already checks for asymmetric visibility.

@nicolas-grekas nicolas-grekas changed the title [Serializer] Fix serializer crash due to asymmetric visibility on protected(set) or private(set) properties [Serializer] Fix dealing with asymmetric visilibity for properties Sep 1, 2025
@nicolas-grekas nicolas-grekas force-pushed the fix/serializer-asymmetric-visibility-error branch from 81f3a04 to 6310c44 Compare September 1, 2025 13:36
@nicolas-grekas
Copy link
Member

Thank you @santysisi.

@nicolas-grekas nicolas-grekas merged commit bbfcf84 into symfony:6.4 Sep 1, 2025
6 of 11 checks passed
This was referenced Sep 27, 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.

3 participants