deepclone_hydrate: reject hydrating shell-less internal final classes (BcMath\Number)#20
Merged
Merged
Conversation
BcMath\Number (and any final internal class with a serialization API whose empty serialization payload does not reconstruct an instance) was hydrated by injecting properties into a bare create_object() shell. The engine refuses ReflectionClass::newInstanceWithoutConstructor() for such classes precisely because the shell is not usable on its own; for BcMath\Number the bc_num stays NULL, so the result crashed on first use. deepclone_hydrate() now probes an empty serialization payload (under serialize_lock, so it stays isolated when called from inside another unserialize() such as Serializable::unserialize()) and throws the documented \DeepClone\NotInstantiableException when no valid shell can be built, matching the polyfill. Round-trip via deepclone_to_array() / deepclone_from_array() is unaffected: it replays the real state through __unserialize().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BcMath\Numberis afinalinternal class with a customcreate_objectand__serialize/__unserialize.deepclone_hydrate()builds an instance viaobject_init_ex()(i.e.create_object) and then injects the supplied properties. ForBcMath\Numberthat shell has aNULLbc_num, and itsvalue/scaleare virtual read-only properties with no injectable backing, so hydrate produced a broken instance that crashed (or, with the companion bcmath guard, threw) on first use.deepclone_hydrate(BcMath\Number::class, ['value' => '7.5'])was the reproducer from symfony/symfony#64323.The engine already refuses
ReflectionClass::newInstanceWithoutConstructor()forINTERNAL + create_object + finalclasses, and the polyfill refuses to hydrate them. This brings the extension in line: for such a class with a serialization API,deepclone_hydrate()now probes whether an emptyO:/C:payload reconstructs an object and, if not, throws the documented\DeepClone\NotInstantiableException(Class "..." is not instantiable.) instead of returning a half-built shell.The probe runs under
serialize_lock, so whendeepclone_hydrate()is called from inside another inlineunserialize()(e.g.Serializable::unserialize(), which does not raise the lock) it gets an isolated unserialize buffer. Otherwise the probe's deferred__unserialize([])would not fire (defeating the check) and its throwaway object would leak into the caller's dtor list and corrupt the outerunserialize().Round-trip (
deepclone_to_array()/deepclone_from_array()) is unaffected: it reconstructsBcMath\Numberby replaying the real state through__unserialize(), and round-trips value, scale and shared identity. New.phpts cover the round-trip, the hydrate refusal, the reentrancy isolation, and that an (abnormally) uninitialized instance throws cleanly rather than crashing.Companion bcmath hardening: php/php-src#22259
Companion polyfill fix: symfony/polyfill#628