PHP 8.5: Fix deprecation notices for __sleep and __wakeup magic methods

[TobiasBg]

Parent ticket for all things PHP 8.5: #63061

---

PHP 8.5 deprecates the magic methods __sleep and __wakeup:
​https://wiki.php.net/rfc/deprecations_php_8_5#deprecate_the_sleep_and_wakeup_magic_methods

This currently causes warnings like "Deprecated: The __sleep() serialization magic method has been deprecated. Implement __serialize() instead (or in addition, if support for old PHP versions is necessary)".

WordPress doesn't seem to be using __sleep but has several __wakeup methods, in Core files (in Block management classes and the HTML API) and in (semi-)external libraries (Requests, sodium-compat).

Using __serialize and __unserialize is recommended as a replacement.

These have been introduced in PHP 7.4, so for PHP < 7.4, __sleep and __wakeup need to be kept.
Fortunately, per ​the docs, it's fine to have both present, which then also won't trigger the deprecation notices.

Thus, the logic of __sleep and __wakeup methods should be moved to __serialize and __unserialize. As long as PHP < 7.4 is supported, __sleep and __wakeup methods should then be (re-)added as wrappers.

[TobiasBg]

An GitHub issue for the Requests library is at ​https://github.com/WordPress/Requests/issues/949.

[TobiasBg]

It looks like all/most of these __wakeup calls were added via [56835], including those in the Requests library (despite it being an external library).

This PR adds __unserialize() implementations across WordPress core classes to address PHP 8.5 deprecation of __wakeup(). The new methods mirror existing __wakeup() behavior - either throwing LogicException for classes that shouldn't be unserialized, or performing validation/cleanup for classes that can be safely restored. __wakeup() methods are kept as wrappers calling the same logic as __unserialize().

Trac ticket: https://core.trac.wordpress.org/ticket/63962

What's with the failing tests?

What's with the failing tests?

checking the error it seems like src/wp-includes/Requests/src/Utility/FilteredIterator.php unserialize function has undefined $callback which it wasnt giving on wakeup

[TobiasBg]

#64008 is for updating sodium_compat which fixes this issue there.

This should please be reopened as it still contains changes that need to be committed.

[SergeyBiryukov]

In 60796:

Code Modernization: Address __sleep() and __wakeup() deprecations in PHP 8.5.

PHP 8.5 deprecates the __sleep() and __wakeup() magic methods in favor of __serialize() and __unserialize():

Deprecated: The __wakeup() serialization magic method has been deprecated. Implement __unserialize() instead (or in addition, if support for old PHP versions is necessary)

For PHP < 7.4 compatibility, __sleep() and __wakeup() need to be kept for the time being.

This commit moves the logic of __wakeup() methods in core to __unserialize(), and turns the former into wrappers. WordPress core does not use __sleep() methods, so these are the only changes required.

Reference: ​PHP RFC: Deprecations for PHP 8.5: Deprecate the __sleep() and __wakeup() magic methods.

Follow-up to [56835], [60787], [60795].

Props TobiasBg, tusharbharti, swissspidy, dmsnell, SergeyBiryukov.
Fixes #63962. See #63061.

[dmsnell]

Thanks all for getting this updated and merged.

One question I have about the change: is it possible to have accomplished it without adding a new array of phpcs:ignore comments? Do we not have a mechanism to add the excludes externally from the source code in some PHPCS configuration?

This raises another question: if we know it’s important to provide these magic methods to prevent unintentional security issues, why are we having our automations reject people’s work when they add them?

[TobiasBg]

Switching from phpcs:ignore comments to a config file should be possible as in [60795].
I assume that @SergeyBiryukov chose that config over comments there as sodium_compat is an external library.

I believe that the idea of this phpcs sniff is not to reject using __serialize()/__unserialize() per se, but ​to check if they are used in code running with PHP < 7.4, where these did not yet exist (but __sleep() and `wakeup() need to be used).

[desrosj]

I think that [60796] is causing an issue. Here is what I've been able to determine so far.

Previously, because there was no __unserlialize() method, the data was restored using unserialize(). However, now that the magic method is present, that method completely replaces the default behavior. Because of this, the properties that are checked are always empty/not set when __unserialize() is invoked.

For example, in WP_Block_Patterns_Registry, $registered_patterns property is always empty and the first conditional check will always return early.

In classes where actual logic was moved into __unserialize() (this should not matter for any that throw a LogicException should), the logic needs to be updated to restore the passed data from the $data parameter before performing checks for the presence or datatype of the expected properties.

[TobiasBg]

Indeed, looks like actually setting the object properties has to be done ourselves with __unserialize.

See e.g. ​this PHP docs example for __unserialize, where properties and DP connection are restored.
Contrary, ​the example for __wakeup doesn't restore the properties but only the DB connection.

Trac ticket: https://core.trac.wordpress.org/ticket/63962

[SergeyBiryukov]

Replying to desrosj:

In classes where actual logic was moved into __unserialize() (this should not matter for any that throw a LogicException should), the logic needs to be updated to restore the passed data from the $data parameter before performing checks for the presence or datatype of the expected properties.

Thanks! ​PR 10031 fixes the issue in my testing, though the tests on GitHub appear to be failing currently due to some unrelated issue:

> WordPress@6.9.0 env:start
> node ./tools/local-env/scripts/start.js && node ./tools/local-env/scripts/docker.js run -T --rm php composer update -W

 wordpress-develop Pulling 
 php Pulling 
 cli Pulling 
 mysql Pulling 
 cli Error unauthorized: authentication required
 wordpress-develop  Interrupted
 mysql  Interrupted
 php  Interrupted
Error response from daemon: unauthorized: authentication required

[TobiasBg]

Yes, those failing tests were likely caused by global Docker Hub outages last night, for which I saw multiple reports throughout social media.

[desrosj]

My comment on the PR did not make its way over here. So adding it to make it easier to follow:

This fixes the PHP >= 7.4 instances, but I think there will be problems for PHP 7.2 & 7.3 when __wakeup() is called. That method will only ever pass an empty array to __unserialized(), but the previous behavior was to restore the properties within that method.

I have not been able to find a good way to pass the instance through to __unserialize(). The only way to fix this may be to preserve the previous logic within __wakeup() in addition to adding it to __unserialize(). It's a bit of duplication, but it will be removed once support for PHP 7.2 & 7.3 is dropped (hopefully in 7.0).

I submitted a PR to @SergeyBiryukov's that re-adds the previous logic to __wakeup(). After that, I am not seeing issues on any supported version of PHP.

Technically, the duplication aspect could be resolved by calling __wakeup() from __serialize(), no? I'm not sure if that's worthwhile or necessary though, given that __wakeup() is bound to be removed in the future.

[johnbillion]

In 60804:

Code Modernization: Revert [60796]. This change needs some more work before it's fully ready.

See #63962

[swissspidy]

Note: this might become a soft deprecation, see ​https://wiki.php.net/rfc/soft-deprecate-sleep-wakeup

In which case no action should be needed.

[TobiasBg]

Indeed, these deprecations have been changed to "soft deprecate", implemented via ​https://github.com/php/php-src/pull/19966

Thus, no action is necessary at this time.