Improve error handling for unserialize()

[jrf]

From https://core.trac.wordpress.org/ticket/59231:

​Make unserialize() emit a warning for trailing bytes

While based on the current test suite, WP is not directly affected by this, the ​`maybe_unserialize()` function could still be confronted by data with trailing bytes.

However, the call to the PHP native unserialize() within maybe_unserialize() silences all (PHP 8.0+: non-fatal) errors, so this new warning will not affect WP or its ecosystem as long as the maybe_unserialize() function is used.

Having said that, a critical look at maybe_unserialize() may be warranted as the new warning in PHP is related to security issues discovered in other projects, so WP may want to consider rejecting unserialization for data throwing this warning.

Also note that there are 7 uses of unserialize() in total within WP Core, one within maybe_unserialize(), but the function is also used in 6 other places and 5 of those do not use error silencing.

​Improve unserialize() error handling

This, again, affects the ​`maybe_unserialize()` function and this time, the code should probably be adjusted to handle the new errors which unserialize() can now throw.

The change does not affect unserializing valid data, but in the case of invalid data, the type of and severity of the notices/warnings/catchable exceptions have been changed.

All 7 uses of unserialize() in WP Core should be reviewed and for the 6 uses outside of the maybe_unserialize() function, it should be reviewed whether they can/should switch to using maybe_unserialize() and/or whether they should get their own (improved) error handling.

[hellofromTonya]

6.4 RC1 is happening a few hours. Moving this to 6.5.

[chaion07]

Thanks @jrf for reporting this. We reviewed this ticket during a recent bug-scrub session. Based on the feedback received we are making the following changes:

1. Considering the lack of progress with this ticket we are in n favor of updating the milestone to 6.6
2. However this is a PHP Compatibility ticket (and a Task) which often only get attention late in a cycle, punting too early may delay the ticket getting necessary discussion. That said, Tonya is active and may pull it back into 6.5 if that's appropriate

Cheers!

Props to @costdev & @mukesh27

[azaozz]

a critical look at maybe_unserialize() may be warranted as the new warning in PHP is related to security issues discovered in other projects

+1. As a minimum thinking that the warnings from unserialize() should not be silenced when WP is in development mode (see ​https://developer.wordpress.org/reference/functions/wp_is_development_mode/).

Also thinking it makes sense to use maybe_unserialize() instead of unserialize() in more places/as appropriate as an attempt to maintain backwards compatibility (no warnings) in production in PHP 8.0+.

[hellofromTonya]

With the last scheduled 6.6 beta is next week, moving this ticket to the next major. This ticket needs more time for discussion and investigation to determine its resolution.

[chaion07]

Thanks @jrf for reporting this. As we move closer to the RC1 for 6.7 in a matter of days and with no progress in this cycle, we are updating the milestone based on the feedback received from a recent bug-scrub session. Thanks.

Props to @peterwilsoncc

Cheers!