Make WordPress Core

Opened 11 months ago

Last modified 3 weeks ago

#59231 new task (blessed)

Prepare for PHP 8.3

Reported by: jrf's profile jrf Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version: 6.4
Component: General Keywords: php83 has-patch has-unit-tests
Focuses: php-compatibility Cc:

Description (last modified by jrf)

This is a meta ticket to track the efforts to prepare for PHP 8.3.

For PHP 8.0/8.1/8.2 specific fixes, please refer to the generic WP 6.4 PHP 8.x ticket: #58850

Please link patches related to a specific PHP 8.3 related task to the appropriate dedicated issue, if there is one (see the links in the description below).

Generic/one-off PHP 8.3 related patches can be linked to this ticket.


PHP 8.3: Important dates

PHP 8.3 is expected to be released on November 23 2023.

Other note-worthy dates:

  • The first alpha was released on June 8th 2023.
  • Feature freeze started on July 18, 2023.

Note:

The below represents the status per August 28, 2023. As PHP 8.3 is in feature freeze, these statuses should be reasonably reliable.

Readiness of essential tooling

Composer

Current status:

  • CI for Composer itself was not yet being run against PHP 8.3. I've opened a PR for this. [JRF: this PR has since been merged]
  • I've ran linting, PHPCompatibility (bleeding edge) and the test suites against PHP 8.3 and found no problems for PHP 8.3 though.
  • The only issues I've managed to identify are in the test suite of Composer, which has no impact on end-users of Composer.

PHPUnit

Current status:

  • CI for PHPUnit itself is being run against PHP 8.3.
  • No known issues in the last release supported for the WP test suite (9.6.11).

PHPUnit Polyfills

Current status:

  • CI for PHPUnit Polyfills itself is being run against PHP 8.3.
  • No known issues in the last release (1.1.0).

WP-CLI

Current status:

Other tooling

Other (PHP) tooling doesn't necessarily have to run against PHP 8.3 (yet), so has not been evaluated.

Initial DevOps Tasks

Typical tasks which need to be executed to allow WordPress to prepare for PHP 8.3:

Docker

  • Add PHP 8.3 to the Docker images. A PR for this was merged on July 26, 2023

GitHub Actions

  • Add PHP 8.3 to the GitHub Actions phpunit-tests.yml configuration. GH PR #5106 [JRF: this PR has since been merged]

Notes:

  • Test failures on PHP 8.3 should not (yet) fail the build, but as the actual script to run the tests has been moved, it is currently impossible to use continue-on-error as that keyword is not supported when calling a reusable workflow... /cc @desrosj

PHP 8.3 changes for which WordPress will need to prepare

Generic deprecations for PHP 8.3

Based on initial (bleeding edge) PHPCompatibility scans + the tests, WP is not affected by the deprecations which passed from this RFC (not all of them did).

Deprecation of functions with overloaded signatures

This RFC only partially affects PHP 8.3. If a replacement is readily available already, the deprecation of the overloaded signature takes place in PHP 8.3.
If no replacement was available, the replacement functions are being introduced in PHP 8.3 and the actual deprecation of the overloaded signature takes place in PHP 8.4.

Based on initial (bleeding edge) PHPCompatibility scans + the tests, WP is affected by two of the deprecations in PHP 8.3:

  • get_class() and get_parent_class() - this is already being tracked in #58876, there is a patch available, which IMO is ready for commit. [JRF: the PR for this has since been merged]
  • ReflectionProperty::setValue() with static properties. GH PR #5105 [JRF: this PR has since been merged]

The other deprecations in this RFC do not appear to affect WP Core at this time.

There is one - stream_context_set_option(), which will impact Requests, but only in PHP 8.4 and a patch has already been pulled for this.

Saner increment/decrement operators

To my surprise, I have not found any issues in WP with this change based on the tests alone, but I would not be surprised if the odd issue around this gets reported over time.

Marking overridden methods

This is a new feature with limited validation functionality attached. The attribute basically allows to mark methods in a (child) class/interface which overload a method in a parent class or from an interface, as doing so intentionally.

Per the RFC:

... being able to express if a method is intended to override another method or implement an interface would make it easier to debug a mistake, to refactor and to clean up existing code. Another possible use case is to easily detect a possibly breaking change in a parent class that was provided by a library without needing to read the changelog in detail or missing some item in the list of changes

I'd like to advocate for adding these attributes to WP Core in all the relevant places as it:

  • Increases awareness of the method overload for contributors.
  • Can serve as a warning that the method signature should not be touched (unless the parent method signature changes).
  • Has no downside as attributes are ignored in older PHP versions and in PHP versions where the attribute referenced does not exist.

In the rare case that the attribute, once added, would result in a fatal error, that would be fantastic, as that means we have actually found a bug in WP before it got into a stable release.

Separate ticket to allow for discussing this proposal in more detail and for patches: #59232.

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.

Separate ticket to allow for discussing this and the previously listed RFC in more detail and for patches: #59233.

Deprecate remains of string evaluated code assertions

As WP Core does not use assertions, it is not affected by the changes in this RFC.

Plugins/themes may still be affected, though I'd hope none of those would use assert().*

  • assert() is intended for dev-only use. The behaviour of assert() is heavily affected by ini settings which cannot be changed at runtime, which means that end-users may be confronted by unexpected fatal errors due to the use of assert() if they run on an incorrectly configured webhost.

Define proper semantics for range() function

This RFC adds a number of errors and warnings for incorrect use of the range() function.

WP Core has 8 uses of this function in src, 2 in class-wp-text-diff-renderer-table.php and 6 in various files from external dependencies.

I've visually reviewed each of these and they all look to be okay, though a check to safeguard that the WP native uses are covered sufficiently by tests would be prudent. [TODO]

More Appropriate Date/Time Exceptions

This RFC reclassifies warnings and errors from the DateTime extension to catchable Exceptions when the OO-interface is used (procedural use of the DateTime functionality is not affected).

Based on the tests, WP Core is not affected by this and as the DateTime use of WP Core is pretty well tested, I'm fairly confident, we'll be fine.

New json_validate() function

This function is a high-performance way to validate json prior to decoding it. This function cannot be polyfilled without a performance hit.

However, due to the potential for using json for Denial-of-Service attack vectors (via a HUGE file/stream), I would strongly recommend for WP Core to start using this new function in all appropriate places wrapped within an if ( function_exists() ) {}.

The json_decode() function is used 44 times within src (excluding external dependencies).

We may want to consider introducing a wp_json_decode() function to ensure the use of json_validate() (when available).
This would then mirror the already existing `wp_json_encode()` function.

See: #59234

Status of External Dependencies

GetID3

Current status:

  • Linting is enabled against PHP 8.3. The build passes without finding any PHP 8.3 related issues.
  • Important: the project has no test suite, so the linting passing on PHP 8.3 is only a small comfort and does not provide any real security.
  • In other words: status unknown.
  • WordPress is using the latest version (1.9.22), see #56692

PHPMailer

Current status:

  • Linting and tests are being run against PHP 8.3.
  • No known issues in the last release (6.8.0) (aside from something in the PHPMailer test suite, which doesn't affect WP).
  • WordPress is using the latest version, see #57873

Requests

Current status:

  • Linting and tests are being run against PHP 8.3.
  • No known issues in the last release (2.0.7) (aside from something in the Requests test suite, which doesn't affect WP).
  • WordPress is using the latest relevant version 2.0.6, see #58079. Requests 2.0.7 only updated the certificates bundle, while WP uses its own)

SimplePie

Current status:

  • Tests are being run against PHP 8.3.
  • No known issues in the current master branch.
  • WordPress is behind and is still using version 1.5.8, while the latest release is 1.6.0, see #55604

I've done a test run of SimplePie 1.5.8 against PHP 8.3 and based on the tests, there are no relevant PHP 8.3 issues known at this moment.

Sodium Compat

Current status:

  • A PR has been opened to enable running of the tests against PHP 8.3. The build passes without finding any PHP 8.3 related issues. [JRF: this PR has since been merged]
  • No known issues in the last release (1.20.0).
  • WordPress is using the latest version, see #58224.

PHPass

Current status:

  • Tests are being run against PHP 8.3.
  • No known issues in the current main branch, which translates to the 0.5.4 version.
  • WordPress is using version 0.5.0, but the script is a little out of sync with upstream, though not in a way that it impacts the running of WP on PHP 8.3.

Change History (19)

This ticket was mentioned in PR #5105 on WordPress/wordpress-develop by @jrf.


11 months ago
#1

  • Keywords has-patch has-unit-tests added

The single parameter signature, which was used for setting the value on a static property, is deprecated since PHP 8.3. A cross-version solution is to pass null as the first parameter.

This fixes all instances of the use of the deprecated signature in WP Core.

https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures#reflectionpropertysetvalue

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

#2 @jrf
11 months ago

  • Description modified (diff)

This ticket was mentioned in PR #5106 on WordPress/wordpress-develop by @jrf.


11 months ago
#3

Providing PRs #4887 and #5105 are committed first, this PR can go in without the need for a continue-on-error as the build will pass on PHP 8.3.

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

#4 @jrf
11 months ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core-php by jrf. View the logs.


11 months ago

#6 @jrf
11 months ago

  • Description modified (diff)

#7 @SergeyBiryukov
11 months ago

  • Description modified (diff)

#8 @SergeyBiryukov
11 months ago

In 56492:

Tests: Correct uses of ReflectionProperty::setValue() for static properties.

The single parameter signature, which was used for setting the value on a static property, is deprecated since PHP 8.3. A cross-version solution is to pass null as the first parameter.

This commit updates all the instances that use the deprecated signature in WordPress core.

Reference: PHP RFC: Deprecate functions with overloaded signatures: ReflectionProperty::setValue().

Follow-up to [53152], [54493], [54799].

Props jrf, costdev, Tests: Correct uses of ReflectionProperty::setValue() for static properties.

The single parameter signature, which was used for setting the value on a static property, is deprecated since PHP 8.3. A cross-version solution is to pass null as the first parameter.

This commit updates all the instances that use the deprecated signature in WordPress core.

Reference: PHP RFC: Deprecate functions with overloaded signatures: ReflectionProperty::setValue().

Follow-up to [53152], [54493], [54799].

Props jrf, costdev, sc0ttkclark.
See #59231.

@SergeyBiryukov commented on PR #5105:


11 months ago
#9

Thanks for the PR! Merged in r56492.

@SergeyBiryukov commented on PR #5106:


11 months ago
#10

Thanks for the PR!

With the prerequisite PRs both merged, it appears that the build still fails on Xdebug tests:

error: 'xdebug' does not exist

Apparently the Docker image for PHP 8.3 does not contain Xdebug yet, so we might need to disable those tests for now, like we did for PHP 8.2 in r53922.

@jrf commented on PR #5106:


11 months ago
#11

Thanks for the PR!

With the prerequisite PRs both merged, it appears that the build still fails on Xdebug tests:

error: 'xdebug' does not exist

Apparently the Docker image for PHP 8.3 does not contain Xdebug yet, so we might need to disable those tests for now, like we did for PHP 8.2 in r53922.

Ah, yes, of course! Xdebug for PHP 8.3 is not yet available, we're ready for PHP 8.3 too early ... 🤪

I'm currently in a workshop. If you like, I'll have a look at this tonight and rebase the PR as well. Does that work for you ?

#12 @SergeyBiryukov
11 months ago

In 56498:

Build/Test Tools: Enable running the tests on PHP 8.3.

PHP 8.3 is expected to be released at the end of November 2023.

Enabling the tests to run in CI on PHP 8.3 allows WordPress core to start getting ready.

Note: Xdebug-related tests will not be run on PHP 8.3 at this time as the Docker image for PHP 8.3 does not contain Xdebug yet. Once a stable release of Xdebug 3.3.0 is available, it can be added to the Docker image and the test step can then be enabled for PHP 8.3.

Follow-up to [53922], [56492], [56495].

Props jrf, costdev.
See #59231.

@SergeyBiryukov commented on PR #5106:


11 months ago
#13

Ah, yes, of course! Xdebug for PHP 8.3 is not yet available

Thanks for the confirmation 🙂 I have added a conditional to skip Xdebug-related tests for the time being, and the build is passing now 🎉 Merged in r56498.

@jrf commented on PR #5106:


11 months ago
#14

Thanks @SergeyBiryukov ! This means the base PHP 8.3 compatibility for WP Core is now done and WP Core can now be called "beta-compatible" (with the caveats of the three split off tickets with some known improvements to make, though those are not necessarily incompatibilities with PHP 8.3).

/cc @hellofromtonya

#15 @jrf
11 months ago

  • Description modified (diff)

I've updated the ticket description with the current status of some of the related PRs/tickets.

#16 @hellofromTonya
9 months ago

  • Milestone changed from 6.4 to 6.5

6.4 RC1 is in a few hours. Moving remaining work to 6.5.

This ticket was mentioned in Slack in #core by jrf. View the logs.


9 months ago

#18 @swissspidy
5 months ago

  • Milestone changed from 6.5 to 6.6

#19 @desrosj
3 weeks ago

  • Milestone changed from 6.6 to 6.7
Note: See TracTickets for help on using tickets.