Make var_export/debug_zval_dump check for infinite recursion on the *object* #9448
Conversation
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
…object* Switch the recursion check from the result of `get_properties_for` (the returned hash table of properties) to just checking for infinite recursion on the object. - In order for a native datastructure to correctly implement `*get_properties_for` for var_export's cycle detection, it would need to return the exact same array every time prior to this PR. Prior to this commit, the requirements for cycle detection would prevent SplFixedArray or similar classes from returning a temporary array that: 1. Wouldn't be affected by unexpected mutations from error handlers 2. Could be garbage collected instead. Note that SplFixedArray continues to need to return `object->properties` ~~until php 9.0, when dynamic properties are forbidden.~~ (even after php 9.0, because `#[\AllowDynamicProperties]` can be used in subclasses?)
|
This was approved in #8046 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Switch the recursion check from the result of
get_properties_for(the returned hash table of properties) to just checking for
infinite recursion on the object.
In order for a native datastructure to correctly implement
*get_properties_forfor var_export's cycle detection,it would need to return the exact same array every time prior to this PR.
Prior to this commit, the requirements for cycle detection
would prevent SplFixedArray or similar classes from returning a
temporary array that:
Note that SplFixedArray continues to need to return
object->propertiesuntil php 9.0, when dynamic properties are forbidden.(even after php 9.0, because
#[\AllowDynamicProperties]can be used in subclasses?)This is the same as #8046 ,
which was accidentally deleted after deleting a similar branch name of an earlier version of this.
Note that an earlier commit already introduced ext/spl/tests/fixedarray_022.phpt with the same test case contents for the original bug
Thankfully, #8046 (comment) are the only remaining issues I'm aware of after the last bug fix https://github.com/php/php-src/pull/8105/files
(delayed __destruct of elements after a var_export call on a collection followed by change such as remove()/pop(), extra memory usage on internal data structures such as SPL after a var_export call, poor worst-case performance for cycle detection in var_export and related functions)