Skip to content

Fix #18542: unserialize() respects class_alias for private properties#21875

Open
HakanIST wants to merge 1 commit into
php:masterfrom
HakanIST:fix/unserialize-class-alias-properties
Open

Fix #18542: unserialize() respects class_alias for private properties#21875
HakanIST wants to merge 1 commit into
php:masterfrom
HakanIST:fix/unserialize-class-alias-properties

Conversation

@HakanIST
Copy link
Copy Markdown

Problem

When a class with private properties is serialized, the property names are mangled as \0ClassName\0propName. If the class is later aliased via class_alias(), unserialize() fails to match the mangled class name against the actual class entry, causing properties to be treated as dynamic instead of declared.

This results in:

  • Creation of dynamic property is deprecated warning (PHP 8.2+)
  • Typed property must not be accessed before initialization fatal error

Root Cause

In is_property_visibility_changed() (ext/standard/var_unserializer.re), the mangled class name is only compared against ce->name via strcasecmp(). When class_alias() is used, the mangled string contains the alias name while ce->name holds the canonical class name — these don't match, so the property is not recognized as declared.

Fix

Added a fallback check that uses zend_hash_str_find_ptr_lc(EG(class_table), ...) to verify whether the mangled class name resolves to the same zend_class_entry. This:

  • Uses zero heap allocations (zend_hash_str_find_ptr_lc uses stack-allocated do_alloca internally)
  • Is only triggered as a fallback when the direct name comparison fails, so there is zero overhead on the normal (non-alias) path
  • Does not trigger autoloading (direct hash table lookup)

Test

Added ext/standard/tests/serialize/unserialize_class_alias_private_props.phpt covering private, protected, and public properties with class_alias().

All existing serialization tests (164) and class_alias tests (26) pass with zero regressions.

Closes #18542

@HakanIST
Copy link
Copy Markdown
Author

CI Note: All test failures across every platform are the same pre-existing pdo_set_fetch_mode_errors.phpt test, also failing on current master. The FREEBSD_NTS failure is an SSH infrastructure issue. Our new test unserialize_class_alias_private_props.phpt passes on all platforms.

@HakanIST
Copy link
Copy Markdown
Author

This bug also manifests in MediaWiki when using APCU cache with class_alias for backward compatibility during namespace migrations (Phabricator T405901). Verified locally that this patch resolves it.

@Girgias
Copy link
Copy Markdown
Member

Girgias commented Apr 29, 2026

Please rebase on the latest master to fix CI.

@HakanIST HakanIST force-pushed the fix/unserialize-class-alias-properties branch from 60737b3 to a6aaf3a Compare April 29, 2026 07:54
@HakanIST HakanIST force-pushed the fix/unserialize-class-alias-properties branch 2 times, most recently from 3b92419 to 687ad19 Compare May 23, 2026 12:55
@HakanIST
Copy link
Copy Markdown
Author

Rebased on latest master as requested, and expanded test coverage with additional cases: inheritance with parent private properties, multiple typed properties via alias, and a regression check using the canonical class name. @Girgias @bukka ready for review.

@TimWolla TimWolla self-requested a review May 23, 2026 13:58
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

I don't see @IMSoP's remark being accounted for in the tests: #18542 (comment)

When a class with private properties is serialized, the property names
are mangled as \0ClassName\0propName. If the class is later aliased via
class_alias(), unserialize() fails to match the mangled class name
against the actual class entry, causing properties to be treated as
dynamic instead of declared.

This adds a fallback check in is_property_visibility_changed() that
uses zend_hash_str_find_ptr_lc() to look up the mangled class name in
EG(class_table) and verify it resolves to the same zend_class_entry.
This is only triggered when the direct name comparison fails, so there
is zero overhead on the normal (non-alias) path.

Closes phpGH-18542
@HakanIST HakanIST force-pushed the fix/unserialize-class-alias-properties branch from 687ad19 to b9b2df8 Compare May 23, 2026 16:01
@HakanIST
Copy link
Copy Markdown
Author

Added tests to cover @IMSoP's concern from #18542:

Test 7: Shadowed private properties at different inheritance levels with alias. \0ChildAlias\0prop resolves to Child's property, while \0Base\0prop stays with Base's property. No cross-contamination happens between levels.

Test 8: Single class (no inheritance) with both canonical and alias mangled names:

  • \0BravoAlias\0value alone (alias path)
  • \0Bravo\0value alone (canonical path)
  • Both in same payload, verifying they resolve to the same declared slot without creating dynamic property

The fix only triggers zend_hash_str_find_ptr_lc() lookup as fallback when direct strcasecmp(unmangled_class, ce->name) fails, and checks == ce so it cannot match a different class entry in the hierarchy.

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.

unserialize doesn't respect class_alias for properties

4 participants