-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Serializer] Fix readonly property initialization from incorrect scope
#61028
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] Fix readonly property initialization from incorrect scope
#61028
Conversation
2f87dc4 to
5c28257
Compare
| } | ||
|
|
||
| $reflectionProperty->setValue($object, $value); | ||
| if (!$reflectionProperty->isReadOnly()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we should deal with asymmetric visibility also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out!
I tested the current implementation using properties with asymmetric visibility in PHP 8.4 and everything works as expected. The values are correctly set using ReflectionProperty::setValue() without throwing errors.
I believe this is because, starting from PHP 8.4.0, ReflectionProperty::setValue() still bypasses visibility restrictions (including asymmetric ones)
To make this explicit, I’ve added some tests that cover this behavior specifically for PHP 8.4.
Let me know if you'd prefer a stricter approach that checks write visibility explicitly.
5c28257 to
caee87a
Compare
caee87a to
79c2ea6
Compare
readonly property initialization from incorrect scope
|
Thank you @santysisi. |
Readonly properties couldn't be initialized during denormalization due to scope restrictions. This change checks if a property is
readonlyand uninitialized, if so, it sets the value using the declaring class's scope.Also added a safety check to throw a
LogicExceptionif areadonlyproperty is already initialized, to avoid accidental mutation.