-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
DOMNode serialization on PHP ^8.1 #8996
Labels
Comments
Oh, it likely has been overlooked that this might break code if the class is subclassed. I agree that we need to document this change (some other classes/extensions may have the same issue), and that we should look into supporting (un)serialization for respective subclasses. |
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 12, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 12, 2023
…sses if the appropriate methods are present This introduces the class flag ZEND_ACC_SUBCLASS_SERIALIZABLE, which allows classes which are not serializable / deserializable to become that if a subclass implements the appropriate methods. Setting this flag through the stubs can be done using @subclass-serializable. Introducing this through behaviour a new flag allows for opt-in behaviour, which is backwards compatible. Fixes phpGH-8996
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 12, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 13, 2023
…sses if the appropriate methods are present This introduces the class flag ZEND_ACC_SUBCLASS_SERIALIZABLE, which allows classes which are not serializable / deserializable to become that if a subclass implements the appropriate methods. Setting this flag through the stubs can be done using @subclass-serializable. Introducing this through behaviour a new flag allows for opt-in behaviour, which is backwards compatible. Fixes phpGH-8996
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 13, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 19, 2023
…sses if the appropriate methods are present This introduces the class flag ZEND_ACC_SUBCLASS_SERIALIZABLE, which allows classes which are not serializable / deserializable to become that if a subclass implements the appropriate methods. Setting this flag through the stubs can be done using @subclass-serializable. Introducing this through behaviour a new flag allows for opt-in behaviour, which is backwards compatible. Fixes phpGH-8996
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 19, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 19, 2023
…sses if the appropriate methods are present This introduces the class flag ZEND_ACC_SUBCLASS_SERIALIZABLE, which allows classes which are not serializable / deserializable to become that if a subclass implements the appropriate methods. Setting this flag through the stubs can be done using @subclass-serializable. Introducing this through behaviour a new flag allows for opt-in behaviour, which is backwards compatible. Fixes phpGH-8996
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Jan 19, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Oct 8, 2023
…sses if the appropriate methods are present This introduces the class flag ZEND_ACC_SUBCLASS_SERIALIZABLE, which allows classes which are not serializable / deserializable to become that if a subclass implements the appropriate methods. Setting this flag through the stubs can be done using @subclass-serializable. Introducing this through behaviour a new flag allows for opt-in behaviour, which is backwards compatible. Fixes phpGH-8996.
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Oct 8, 2023
Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Oct 8, 2023
PHP 8.1 introduced a seemingly unintentional BC break in ca94d55 by blocking the (un)serialization of DOM objects. This was done because the serialization never really worked and just resulted in an empty object, which upon unserialization just resulted in an object that you can't use. Users can however implement their own serialization methods, but the commit made that impossible as the ACC flag gets passed down to the child class. An approach was tried in php#10307 with a new ACC flag to selectively allow serialization with subclasses if they implement the right methods. However, that was found to be too ad hoc. Instead, let's abuse how the __sleep and __wakeup methods work to throw the exception instead. If the child class implements the __serialize / __unserialize method, then the throwing methods won't be called. Similarly, if the child class implements __sleep and __wakeup, then they're overridden and it doesn't matter that they throw. For the user, this PR has the exact same behaviour for (sub)classes that don't implement the serialization methods: an exception will be thrown. For code that previously implemented subclasses with these methods, this approach will make that code work again. This approach should be both BC preserving and unbreak user's code. For the test: Co-authored-by: wazelin <contact@sergeimikhailov.com>
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Oct 10, 2023
PHP 8.1 introduced a seemingly unintentional BC break in ca94d55 by blocking the (un)serialization of DOM objects. This was done because the serialization never really worked and just resulted in an empty object, which upon unserialization just resulted in an object that you can't use. Users can however implement their own serialization methods, but the commit made that impossible as the ACC flag gets passed down to the child class. An approach was tried in php#10307 with a new ACC flag to selectively allow serialization with subclasses if they implement the right methods. However, that was found to be too ad hoc. Instead, let's abuse how the __sleep and __wakeup methods work to throw the exception instead. If the child class implements the __serialize / __unserialize method, then the throwing methods won't be called. Similarly, if the child class implements __sleep and __wakeup, then they're overridden and it doesn't matter that they throw. For the user, this PR has the exact same behaviour for (sub)classes that don't implement the serialization methods: an exception will be thrown. For code that previously implemented subclasses with these methods, this approach will make that code work again. This approach should be both BC preserving and unbreak user's code. For the test: Co-authored-by: wazelin <contact@sergeimikhailov.com>
isfedorov
added a commit
to JetBrains/phpstorm-stubs
that referenced
this issue
Oct 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
The following code:
Resulted in this output:
But I expected this output instead:
It appears to be sort of an expected behavior of PHP 8.1 due to ca94d55, but I would still expect:
__serialize()
/__unserialize()
or__sleep()
/__wakeup()
method pairs.PHP Version
8.1.0 - 8.1.9
Operating System
No response
The text was updated successfully, but these errors were encountered: