Skip to content

Add fallback hydration for properties#411

Open
madmajestro wants to merge 1 commit intoigbinary:masterfrom
madmajestro:fix-fallback-hydration-of-properties
Open

Add fallback hydration for properties#411
madmajestro wants to merge 1 commit intoigbinary:masterfrom
madmajestro:fix-fallback-hydration-of-properties

Conversation

@madmajestro
Copy link

@madmajestro madmajestro commented Feb 23, 2026

If the visibility of a property has changed between serialization and unserialization, or if a private or protected property needs to be restored, but the serialized data was generated by a __serialize method while no __unserialize method exists in the class, the property value cannot be found during hydration in the object's hash table. The reason for this is that in these cases the property name in the serialized representation does not match the prefixed/mangled property name of the current class definition. To resolve this, an attempt is made to determine the correctly prefixed property name by performing a lookup with the unprefixed property name in the properties_info hash table of the class. This aligns the behavior to the serializer of PHP >= 7.2.

Closes #410
Closes #387
Closes #156

@stof
Copy link

stof commented Feb 24, 2026

I would suggest adding another test where both __serialize and __unserialize are implemented, to make sure the new logic does not break that case.

int mangle_property_names = 0;

#if PHP_VERSION_ID >= 80000
if (ce->__serialize) mangle_property_names = 1;
Copy link
Member

@tricky tricky Feb 24, 2026

Choose a reason for hiding this comment

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

Have not yet reviewed actual substance yet, but for style consistency; please use an explicit comparison and braces:

if (ce->__serialize != NULL) {
...
}

Also please use tabs for indentation.

Copy link
Author

Choose a reason for hiding this comment

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

I'm now using an explicit comparison and curly braces, but I've already used tabs in the C code and spaces in the PHPT tests, just like the other __serialize*.phpt tests do. Should I still use tabs in the tests?

@tricky
Copy link
Member

tricky commented Feb 24, 2026

I would suggest adding another test where both __serialize and __unserialize are implemented, to make sure the new logic does not break that case.

Indeed this is very much required, though there are quite a few tests already. Tests can always be improved.

@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from 157ec93 to 3973a6a Compare February 24, 2026 22:22
@madmajestro
Copy link
Author

I've added a test to check the behavior of __unserialize. Additionally, the values ​​in the __serialize and __unserialize methods are now multiplied by 10. This allows to check if the __serialize / __unserialize functions were actually used.

@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from 3973a6a to 7e01f54 Compare February 25, 2026 15:02
@madmajestro
Copy link
Author

I have just pushed an optimized version. This version uses the mangled representation of the property name already present in zend_property_info. This saves the memory allocation and string copying done in zend_mangle_property_name().

@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from 7e01f54 to 31f2cf3 Compare February 26, 2026 16:08
@madmajestro
Copy link
Author

@tricky
Friendly Ping

@madmajestro
Copy link
Author

Hm, i just saw that #387 and maybe #156 could also be resolved with this PR, if i implement the fallback logic in a different way. I will push a new version to resolve that as well...

@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from 31f2cf3 to b15eb2a Compare March 3, 2026 09:21
@madmajestro madmajestro changed the title Fix fallback hydration of private and protected properties Add fallback hydration for properties Mar 3, 2026
@madmajestro
Copy link
Author

I have now implemented the fallback hydration, as it exists in PHP's internal serializer since PHP 7.2. This should resolve several issues. In addition, the behavior of igbinary hopefully should now be identical to PHP's serializer, allowing it to be used again as a drop-in replacement.

@madmajestro madmajestro requested a review from tricky March 3, 2026 09:43
@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from b15eb2a to 1d0cb26 Compare March 4, 2026 09:11
@madmajestro
Copy link
Author

@tricky
Friendly Ping

If the visibility of a property has changed between serialization and
unserialization, or if a private or protected property needs to be
restored, but the serialized data was generated by a __serialize method
while no __unserialize method exists in the class, the property value
cannot be found during hydration in the object's hash table. The reason
for this is that in these cases the property name in the serialized
representation does not match the prefixed/mangled property name of the
current class definition. To resolve this, an attempt is made to
determine the correctly prefixed property name by performing a lookup
with the unprefixed property name in the properties_info hash table
of the class. This aligns the behavior to the serializer of PHP >= 7.2.
@madmajestro madmajestro force-pushed the fix-fallback-hydration-of-properties branch from 1d0cb26 to a78b797 Compare March 14, 2026 00:36
@madmajestro
Copy link
Author

I've re-analyzed the behavior of the PHP serializer and Igbinary once more and noticed a few edge cases where PHP and Igbinary behaved differently. I've fixed these issues and added / revised tests so that the behavior of PHP and Igbinary is now tested and compared during the tests. I hope this simplifies the review process and helps to identify differences between PHP and Igbinary in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants