Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#55656 closed task (blessed) (fixed)

PHP 8.x: various compatibility fixes for WordPress 6.1

Reported by: hellofromtonya's profile hellofromTonya Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: General Keywords: php8 php81 has-patch has-unit-tests
Focuses: coding-standards Cc:

Description

Previously:

This ticket will be used as an "epic", allowing a variety of small patches each fixing a specific failure to be added to and committed against this ticket.

For patches addressing all instances of failures related to one specific PHP version (such as PHP 8.0, 8.1, or 8.2) change across the codebase, separate tickets should still be opened.

For an example of issues/patches with separate tickets, see:

  • #53299 PHP 8.1: Update is_serialized function to accept Enums
  • #53465 PHP 8.1.: the default value of the flags parameter for htmlentities() et all needs to be explicitly set

When opening a separate ticket, please tag it with the appropriate PHP version keyword so that these tickets can be easily found:

Attachments (2)

jjj-on-2022-05-25-at-16-20-29@2x.png (595.2 KB) - added by johnjamesjacoby 3 years ago.
Deprecated notices in #wp-auth-check interstitial – PHP 8.1.6 here – preg_replace(), trim(), rtrim()
54798-follow-up-fix-php81-deprecation.patch (1014 bytes) - added by jrf 3 years ago.
Fix newly introduced PHP 8.1 incompatibility introduced in [52567] / #54798

Download all attachments as: .zip

Change History (71)

This ticket was mentioned in PR #2133 on WordPress/wordpress-develop by hellofromtonya.


3 years ago
#1

  • Keywords has-patch has-unit-tests added

The PHP native trim() function expects to be passed a string and it is not a nullable parameter. Passing null to it will result in a trim(): Passing null to parameter #1 ($string) of type string is deprecated notice on PHP 8.1.

In the WP_User::get_data_by() static method, the value given is expected to be int or string. However no input validation was done for fields other than the id field.

This PR adds input validation for the field $value to ensure it is a scalar type and if no, bails out returning false. Why is_scalar()? For backwards-compatible, the native behavior of trim() is retained as it coerces non-string scalar types into a string.

This PR also adds a new test class specifically for WP_User::get_data_by() static method.

Trac ticket: https://core.trac.wordpress.org/ticket/55656
Trac ticket: https://core.trac.wordpress.org/ticket/54730

References:

Notes:

This PR resolves these tests:

8) WP_Test_REST_Users_Controller::test_update_item_no_change
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1610

9) WP_Test_REST_Users_Controller::test_update_item_en_US_locale
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1714

10) WP_Test_REST_Users_Controller::test_update_item_empty_locale
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1739

11) WP_Test_REST_Users_Controller::test_update_item_username_attempt
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1768

12) WP_Test_REST_Users_Controller::test_update_item_existing_nicename
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1793

13) WP_Test_REST_Users_Controller::test_update_user_role
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1852

14) WP_Test_REST_Users_Controller::test_update_user_multiple_roles
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1873

15) WP_Test_REST_Users_Controller::test_update_user_role_invalid_privilege_deescalation
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1917

16) WP_Test_REST_Users_Controller::test_update_user_role_invalid_role
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1977

17) WP_Test_REST_Users_Controller::test_update_item_only_roles_as_site_administrator
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2072

18) WP_Test_REST_Users_Controller::test_update_item_including_roles_and_other_params
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2094

19) WP_Test_REST_Users_Controller::test_user_roundtrip_as_editor
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2172
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2217

20) WP_Test_REST_Users_Controller::test_user_roundtrip_as_editor_html
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2172
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2268

21) WP_Test_REST_Users_Controller::test_user_roundtrip_as_superadmin
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2172
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2304

22) WP_Test_REST_Users_Controller::test_user_roundtrip_as_superadmin_html
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2172
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2333

23) WP_Test_REST_Users_Controller::test_get_additional_field_registration
trim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1798
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1141
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:988
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:2764

This ticket was mentioned in PR #1658 on WordPress/wordpress-develop by jrfnl.


3 years ago
#2

Potential fix for the below issue, but the fix _may_ have unforeseen side-effects. Needs second opinion from someone with detailed domain knowledge of the XMLRPC functionality.

Tests_XMLRPC_mw_newPost::test_post_thumbnail
strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated

path/to/src/wp-includes/class-wp-xmlrpc-server.php:5681
path/to/src/wp-includes/src/wp-includes/class-wp-xmlrpc-server.php:5606
path/to/tests/phpunit/tests/xmlrpc/mw/newPost.php:147

Trac ticket: https://core.trac.wordpress.org/ticket/55656
Trac ticket: https://core.trac.wordpress.org/ticket/54730

Update: Changed the Trac ticket to the 6.0 one.

This ticket was mentioned in PR #1556 on WordPress/wordpress-develop by jrfnl.


3 years ago
#3

Fixes strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated notices on PHP 8.1.

Impact: 38 of the pre-existing tests are affected by this issue.

Note: trac ticket link below not added yet on purpose while this ticket is still WIP.

Trac ticket: https://core.trac.wordpress.org/ticket/55656
Trac ticket: https://core.trac.wordpress.org/ticket/54730

Update: Though this is WIP, adding the new 6.0 ticket to ensure this PR moves forward in the 6.0 cycle.

@johnjamesjacoby
3 years ago

Deprecated notices in #wp-auth-check interstitial – PHP 8.1.6 here – preg_replace(), trim(), rtrim()

#6 @johnjamesjacoby
3 years ago

wp_debug_backtrace_summary() of the above screenshot reveals:

string(85) "require('wp-login.php'), wp_signon, wp_authenticate, sanitize_user, wp_strip_all_tags"

All of the above functions would benefit internally from a pretty standard round of type checking, better default variable & parameter values, and isset/empty checks to avoid assuming array keys exist (or array_key_exists() if that's preferable at the time & place).

#7 @ocean90
3 years ago

#55411 was marked as a duplicate.

This ticket was mentioned in PR #2804 on WordPress/wordpress-develop by ocean90.


3 years ago
#8

Fixes PHP 8.1 notices on the login screen caused by passing null values to wp_authenticate().

Currently, user_login and user_password are set by a side-effect of this line https://github.com/WordPress/wordpress-develop/blob/793fa4185d6512967e557eed5ac9c419176d2a28/src/wp-includes/user.php#L67
See https://3v4l.org/8JMRM for an example.

I used a simple empty() check which matches the behaviour of functions like `wp_authenticate_username_password()` which are hooked into the authenticate filter.

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

@jrf
3 years ago

Fix newly introduced PHP 8.1 incompatibility introduced in [52567] / #54798

#9 @jrf
3 years ago

I've uploaded a patch to fix the issue described here:

Commit [52569] introduced an new incompatibility with PHP 8.1 Implicit conversion from float 1.5 to int loses precision....

This needs a follow-up to fix that.

#10 @jrf
3 years ago

  • Keywords php82 removed

FYI: I've opened a dedicated issue to serve as an overview of and track the tasks needing to be done for PHP 8.2 compatibility: #56009

#11 @SergeyBiryukov
3 years ago

In 53555:

Code Modernization: Use the integer portion of an item position in add_submenu_page().

This fixes an Implicit conversion from float to int loses precision PHP 8.1 deprecation notice when adding a new admin menu item with a float value passed as the $position parameter.

This change is covered by existing unit tests and addresses 8 errors when running the test suite on PHP 8.1.

References:

Follow-up to [52569], [53104].

Props jrf.
See #55656, #54798.

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by chaion07. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by amykamala. View the logs.


3 years ago

This ticket was mentioned in PR #3181 on WordPress/wordpress-develop by jrfnl.


3 years ago
#17

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

The tests being run in these particular test groups are passing on PHP 8.1, however, the test runs are still allowed to "continue on error".

This creates the risk that new PHP 8.1 incompatibilities will be introduced without anyone noticing.

By no longer allowing these test runs to "continue on error", that risk is removed.

Trac ticket: https://core.trac.wordpress.org/ticket/55656
Trac ticket: https://core.trac.wordpress.org/ticket/55652

This ticket was mentioned in PR #3184 on WordPress/wordpress-develop by jrfnl.


3 years ago
#18

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

Introduced in [46754] in response to Trac#46686.

This particular code block only makes sense to run when $this->return_url is not null and causes a "passing null to non-nullable" deprecation notice on PHP 8.1 as it was.

By moving the code into the if ( $this->return_url ) condition block, the code will only be run when $this->return_url contains a non-falsy/non-null value.

No additional tests added as this issue was found via the existing tests for the function containing the bug.

This solves the following two PHP 8.1 test errors:

1) Tests_WP_Customize_Manager::test_return_url
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/tests/phpunit/tests/customize/manager.php:2975
/var/www/vendor/bin/phpunit:123

2) Tests_WP_Customize_Manager::test_customize_pane_settings
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/src/wp-includes/class-wp-customize-manager.php:4898
/var/www/tests/phpunit/tests/customize/manager.php:3085
/var/www/vendor/bin/phpunit:123

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

This ticket was mentioned in PR #3185 on WordPress/wordpress-develop by jrfnl.


3 years ago
#19

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

This function was previously already problematic as it does not do proper input validation and the function already received tweaks related to PHP 8.0 in [50408] / Trac#52534, which also introduced the _doing_it_wrong() and added tests.

The short of it is:

  • The function expects to receive an array for the $l10n parameter;
  • ... but silently supported the parameter being passed as a string;
  • ... and would expect PHP to gracefully handle everything else or throw appropriate warnings/errors.

In the previous fix, a _doing_it_wrong() was added for all non-array inputs.
The function would also cause a PHP native "Cannot use a scalar value as an array" warning (PHP < 8.0)/error (PHP 8.0) for all scalar values, except false.

PHP 8.1 deprecated autovivification from false to array, so now false starts throwing a "Automatic conversion of false to array is deprecated" deprecation notice.

By rights, the function should just throw an exception when a non-array/string input is received, but BC...

So, while I really hate doing this, the current change will maintain the previous behaviour, but will prevent both the "Cannot use a scalar value as an array" warning/error as well as the "Automatic conversion of false to array" deprecation notice for invalid inputs.

Invalid inputs _will_ still receive a _doing_it_wrong(), which is the only reason I find this fix even remotely acceptable.

Includes adding a test passing an empty array.
Includes adding an additional test to the data provider for a null input to safeguard the function will not throw a PHP 8.1 "passing null to non-nullable" notice.

Fixes:

3) Tests_Dependencies_Scripts::test_wp_localize_script_data_formats with data set #8 (false, '[""]')
Automatic conversion of false to array is deprecated

/var/www/src/wp-includes/class.wp-scripts.php:514
/var/www/src/wp-includes/functions.wp-scripts.php:221
/var/www/tests/phpunit/tests/dependencies/scripts.php:1447
/var/www/vendor/bin/phpunit:123

Refs:

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

This ticket was mentioned in PR #3186 on WordPress/wordpress-develop by jrfnl.


3 years ago
#20

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

The $path parameter has a default value of null, but would be passed onto the untrailingslashit() function without any input validation, even though the untrailingslashit() function explicitly only expects/supports a string input.

:point_right: An alternative - and possibly better - solution would be to change the default value of the $path parameter to an empty string.
However, such a change would have a wider impact as all function calls which currently pass the null default value for $path would need to be updated as well. Within WP, this is largely an issue in the tests, but it _could_ potentially have an impact on external calls to the function as well.

Note: I'm explicitly not changing the untrailingslashit() function, but fixing this at the point where the invalid input is incorrectly (not) validated.

Includes adding a dedicated unit test for this issue.

This fix also allows to remove a couple of calls to expectDeprecation() in unrelated tests.

Fixes:

4) Tests_Dependencies_Scripts::test_wp_external_wp_i18n_print_order
rtrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2782
/var/www/src/wp-includes/l10n.php:1068
/var/www/src/wp-includes/class.wp-scripts.php:605
/var/www/src/wp-includes/class.wp-scripts.php:320
/var/www/src/wp-includes/class.wp-dependencies.php:136
/var/www/src/wp-includes/functions.wp-scripts.php:109
/var/www/tests/phpunit/tests/dependencies/scripts.php:1505
/var/www/tests/phpunit/includes/utils.php:436
/var/www/tests/phpunit/tests/dependencies/scripts.php:1507
/var/www/vendor/bin/phpunit:123

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

This ticket was mentioned in PR #3187 on WordPress/wordpress-develop by jrfnl.


3 years ago
#21

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

### PHP 8.1 | wp_xmlrpc_server::mw_newPost(): fix passing null to non-nullable and more

The wp_xmlrpc_server::mw_newPost() method creates a new post via wp_insert_post(), but the default/fall-back values used in the function were not in line with the default/fall-back values used in the wp_insert_post() function.

The wp_insert_post() function does a wp_parse_args() (array merge) of the received arguments with the defaults.
If any of the received arguments would be null, this would overwrite the default value (see: https://3v4l.org/bfVlv ) and will lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

For this commit, I've:

  • Ensured that all arguments are defined before they are compact()-ed together to the arguments array.
  • Verified that the default/fall-back value of the arguments as set within the wp_xmlrpc_server::mw_newPost() method are the same as the default/fall-back values used in the wp_insert_post() function.
  • Verified that arguments which do not have a default/fall-back value defined in the wp_insert_post() function are handled correctly.
    • This was not the case for $post_name, which would previously already get an empty string default value in the wp_xmlrpc_server::mw_newPost() function, but then in the wp_insert_post() function, this would prevent the slug generation from being activated.

Fixed now by setting the default in the wp_xmlrpc_server::mw_newPost() function to null..

  • The page_template argument was handled, but not documented in the wp_insert_post() function. I've now documented the argument in the wp_insert_post() function docblock.

Note: there are more than likely several other potential arguments missing from that list, but verifying the whole list is outside the scope of this particular commit.

Includes minor simplifications, such as:

  • Setting a default ahead of an if, instead of in an else clause (as long as no function call is needed to set the default).
  • Remove the unnecessary logic duplication in the $post_status switch.
  • Using a combined concatenation + assignment operator for adding $post_more.

Fixes various errors along the lines of:

1) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
strpos(): Passing null to parameter https://github.com/WordPress/wordpress-develop/pull/1 ($haystack) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2497
/var/www/src/wp-includes/class-wp-hook.php:308
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5616
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315
23) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
json_decode(): Passing null to parameter https://github.com/WordPress/wordpress-develop/pull/1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5615
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315
/var/www/vendor/bin/phpunit:123

### PHP 8.1 | wp_xmlrpc_server::_insert_post(): fix passing null to non-nullable and more

The wp_xmlrpc_server::_insert_post() method creates a new post via wp_insert_post() or updates an existing one via wp_update_post() (which subsequently calls wp_insert_post()), but the default/fall-back values used in the function were not in line with the default/fall-back values used in the wp_insert_post() function.

The wp_insert_post() function does a wp_parse_args() (array merge) of the received arguments with the defaults.
If any of the received arguments would be null, this would overwrite the default value (see: https://3v4l.org/bfVlv ) and will lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

Unfortunately, the conditional logic within the wp_xmlrpc_server::_insert_post() function itself often uses an isset() to trigger certain code blocks., so syncing the defaults with those used in the wp_insert_post() function was not an option.

So for this commit, I've:

  • I've updated the default/fall-back values in the $defaults array only for those values where this would not lead to a change in the behaviour of the function.
  • I've also added a safeguard function, filtering out all remaining null values from the $post_data array before it is passed on to the wp_insert_post() or wp_update_post() functions.

Removing those values is safe as this means that these array keys will now:

  • either be set to the default/fall-back value as defined in wp_insert_post().
  • or not be set and for those values which don't have a default/fall-back value in wp_insert_post(), wp_insert_post() does an ! empty() or isset() check anyway and those array keys not being defined means that the result of those checks will remain the same.

Includes removing a couple of conditions which are now redundant.

Includes removing an expectDeprecation() in the Tests_Date_XMLRPC test class, which is now no longer needed.

Fixes various errors along the lines of:

36) Tests_XMLRPC_wp_newPost::test_no_content
json_decode(): Passing null to parameter https://github.com/WordPress/wordpress-develop/pull/1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1683
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1347
/var/www/tests/phpunit/tests/xmlrpc/wp/newPost.php:25
/var/www/vendor/bin/phpunit:123

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

Closes #1658

This ticket was mentioned in PR #3188 on WordPress/wordpress-develop by jrfnl.


3 years ago
#22

👉🏻 This PR is part of a series of PRs to get the builds passing on PHP 8.1 (to prevent having to _also_ allow failures on PHP 8.2, even when the PHP 8.2 issues are fixed (for a certain definition of "fixed")).

---

### PHP 8.1 | WP_REST_Users_Controller::update_item(): fix passing null to non-nullable

Not all requests are accompanied by a $request['email']. This leads to a PHP 8.1 "passing null to non-nullable" deprecation notice when the WP_REST_Users_Controller::update_item() method passes a null email address onto email_exists(), which eventually reached the WP_User::get_data_by() method where things go wrong.

In the next condition in the code of the WP_REST_Users_Controller::update_item() method - if ( $owner_id && $owner_id !== $id ) - you can see that the code already takes this into account as it will not throw a WP_Error if $owner_id is falsey.

WP_User::get_data_by() returns false for a failed field request. The other functions through which the return value is passed through, do the same.

So, by setting a default value for $owner_id of false and only checking email_exists() when there is an email to check, the "passing null to non-nullable" deprecation notice is bypassed without breaking BC.

Fixes a whole slew of test errors along the lines of:

6) WP_Test_REST_Users_Controller::test_update_item_en_US_locale
trim(): Passing null to parameter https://github.com/WordPress/wordpress-develop/pull/1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1953
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1143
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:990
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1719
/var/www/vendor/bin/phpunit:123

### WP_REST_Users_Controller::update_item(): bug fix

While looking at the code of the WP_REST_Users_Controller::update_item() method, I also noticed another bug.

At the start of the function, the user is retrieved and it is verified that this doesn't result in a WP_Error.
Next, the WP_User $user variable is used to retrieve the user id.

Then in the next condition, it is checked if the $user is falsey. This condition would never match as WP_REST_Users_Controller::get_user() returns either a WP_User object or WP_Error and we already know it's not a WP_Error and an instantiated WP_User object will never evaluate to falsey.

As the WP_Error being thrown _within_ the condition refers to an "invalid user id", I believe the _intention_ of the code was to check that $id is not falsey, which would make more sense, as even though it would be unlikely that a WP_User object would not have a valid ID, that condition _could_ potential match and trigger the WP_Error.

There are already tests in place which (try to) cover this code, but as the WP_REST_Users_Controller::get_user() method throws the same error, it went unnoticed that this condition is incorrect.

In all honesty, IMO, this whole condition can be removed as it is already handled by the call to WP_REST_Users_Controller::get_user()....

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

ocean90 commented on PR #3186:


3 years ago
#23

The plan is actually to remove the incorrect default value as suggested in https://github.com/WordPress/wordpress-develop/pull/2801.

#24 @jrf
3 years ago

The builds against PHP 8.1 are still allowed to "continue on error" as there are still (known) incompatibilities with PHP 8.1 which impact the tests.

As PHP 8.2 is on the horizon and fixes are being created to make WP compatible with PHP 8.2, the above would mean that builds against PHP 8.2 would also need to "continue on error" due to the PHP 8.1 issues, even when there would be no PHP 8.2 issues.

All in all, the risk of new PHP 8.1 + 8.2 issues being introduced without anyone noticing grows exponentially this way.

With that in mind, a renewed effort has been made to fix (some of) the remaining PHP 8.1 issues which impact the test runs.

Ultimately, the goal is to get a passing build for PHP 8.1 (and 8.2), notwithstanding that we know that there are still additional issues to be solved which are currently not or insufficiently covered by tests.

So, with that in mind, I've opened six new PRs related to this ticket:

Reviews/merges appreciated ;-)

jrfnl commented on PR #3186:


3 years ago
#25

@ocean90 I wholeheartedly support that effort. Will leave a comment on the other PR though.

#26 @SergeyBiryukov
3 years ago

In 54072:

Build/Test Tools: Do not allow tests to fail for select PHP 8.1 test runs.

This affects the following test groups:

  • Ajax tests
  • ms-files tests
  • External HTTP tests
  • Xdebug tests

The tests being run in these particular test groups are passing on PHP 8.1, however, the test runs are still allowed to “continue on error”. This creates the risk that new PHP 8.1 incompatibilities will be introduced without anyone noticing.

By no longer allowing these test runs to “continue on error”, that risk is removed.

Follow-up to [51588], [51604], [53922].

Props jrf.
See #55656, #55652.

SergeyBiryukov commented on PR #3181:


3 years ago
#27

Thanks for the PR! Merged in r54072.

jrfnl commented on PR #3181:


3 years ago
#28

Thanks for the quick merge @SergeyBiryukov !

jrfnl commented on PR #3185:


3 years ago
#29

The docblock for the data_wp_localize_script_data_formats() provider also contains a param annotation for $warning on line 1454 which can also be removed.

Good catch! Fixed ;-)

#30 @SergeyBiryukov
3 years ago

In 54135:

Code Modernization: Pass correct value to parse_url() in WP_Customize_Manager::get_return_url().

This particular code block only makes sense to run when $this->return_url is not null. Previously, it caused a "passing null to non-nullable" deprecation notice on PHP 8.1.

By moving the code into the if ( $this->return_url ) condition block, the code will only be run when $this->return_url contains a non-falsey/non-null value.

No additional tests added as this issue was found via the existing tests for the function containing the bug.

This solves the following two PHP 8.1 test errors:

1) Tests_WP_Customize_Manager::test_return_url
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/tests/phpunit/tests/customize/manager.php:2975
/var/www/vendor/bin/phpunit:123

2) Tests_WP_Customize_Manager::test_customize_pane_settings
parse_url(): Passing null to parameter #1 ($url) of type string is deprecated

/var/www/src/wp-includes/class-wp-customize-manager.php:4696
/var/www/src/wp-includes/class-wp-customize-manager.php:4898
/var/www/tests/phpunit/tests/customize/manager.php:3085
/var/www/vendor/bin/phpunit:123

Follow-up to [46754].

Props jrf, costdev.
See #55656.

SergeyBiryukov commented on PR #3184:


3 years ago
#31

Thanks for the PR! Merged in r54135.

#32 @SergeyBiryukov
3 years ago

In 54142:

Code Modernization: Fix autovivification from false to array in WP_Scripts::localize().

This function was previously already problematic as it does not do proper input validation, and it has already received tweaks related to PHP 8.0 in [50408] / #52534, which also introduced a _doing_it_wrong() notice and added tests.

The short of it is:

  • The function expects to receive an array for the $l10n parameter;
  • ...but silently supported the parameter being passed as a string;
  • ...and would expect PHP to gracefully handle everything else or throw appropriate warnings/errors.

In the previous fix, a _doing_it_wrong() notice was added for any non-array inputs. The function would also cause a PHP native "Cannot use a scalar value as an array" warning (PHP < 8.0) or error (PHP 8.0+) for all scalar values, except false.

PHP 8.1 deprecated autovivification from false to array, so now false starts throwing an "Automatic conversion of false to array is deprecated" notice.

By rights, the function should just throw an exception when a non-array/string input is received, but that would be a backward compatibility break.

So the current change will maintain the previous behavior, but will prevent both the "Cannot use a scalar value as an array" warning/error as well as the "Automatic conversion of false to array" deprecation notice for invalid inputs.

Invalid inputs will still receive a _doing_it_wrong() notice, which is the reason this fix is considered acceptable.

Includes:

  • Adding a test passing an empty array.
  • Adding a test to the data provider for a null input to make sure that the function will not throw a PHP 8.1 "passing null to non-nullable" notice.

This solves the following PHP 8.1 test error:

Tests_Dependencies_Scripts::test_wp_localize_script_data_formats with data set #8 (false, '[""]')
Automatic conversion of false to array is deprecated

/var/www/src/wp-includes/class.wp-scripts.php:514
/var/www/src/wp-includes/functions.wp-scripts.php:221
/var/www/tests/phpunit/tests/dependencies/scripts.php:1447
/var/www/vendor/bin/phpunit:123

Reference: PHP Manual: PHP 8.1 Deprecations: Autovivification from false.

Follow-up to [7970], [18464], [18490], [19217], [50408].

Props jrf, costdev.
See #55656.

SergeyBiryukov commented on PR #3185:


3 years ago
#33

Thanks for the PR! Merged in r54142.

#35 @SergeyBiryukov
3 years ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

jrfnl commented on PR #3185:


3 years ago
#36

Thanks @SergeyBiryukov !

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


3 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

hellofromtonya commented on PR #3188:


3 years ago
#39

Self-assigning for commit review and prep.

#40 @hellofromTonya
3 years ago

In 54317:

Code Modernization: Fix null to non-nullable deprecation in WP_REST_Users_Controller::update_item().

Not all requests are accompanied by a $request['email']. This leads to a PHP 8.1 "passing null to non-nullable" deprecation notice when the WP_REST_Users_Controller::update_item() method passes a null email address onto email_exists(), which eventually reached the WP_User::get_data_by() method where things go wrong.

In the next condition in the code of the WP_REST_Users_Controller::update_item() method - if ( $owner_id && $owner_id !== $id ) - you can see that the code already takes this into account as it will not throw a WP_Error if $owner_id is falsey.

WP_User::get_data_by() returns false for a failed field request. The other functions through which the return value is passed through, do the same.

So, by setting a default value for $owner_id of false and only checking email_exists() when there is an email to check, the "passing null to non-nullable" deprecation notice is bypassed without breaking BC.

Fixes a whole slew of test errors along the lines of:

6) WP_Test_REST_Users_Controller::test_update_item_en_US_locale
trim(): Passing null to parameter https://github.com/WordPress/wordpress-develop/pull/1 ($string) of type string is deprecated

/var/www/src/wp-includes/class-wp-user.php:211
/var/www/src/wp-includes/pluggable.php:105
/var/www/src/wp-includes/user.php:1953
/var/www/src/wp-includes/rest-api/endpoints/class-wp-rest-users-controller.php:728
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:1143
/var/www/src/wp-includes/rest-api/class-wp-rest-server.php:990
/var/www/tests/phpunit/includes/spy-rest-server.php:67
/var/www/tests/phpunit/tests/rest-api/rest-users-controller.php:1719
/var/www/vendor/bin/phpunit:123

Follow-up to [44641], [38832].

Props jrf.
See #55656.

#41 @SergeyBiryukov
3 years ago

In 54320:

Code Modernization: Fix null to non-nullable deprecations in wp_xmlrpc_server::mw_newPost().

The wp_xmlrpc_server::mw_newPost() method creates a new post via wp_insert_post(), but the default/fallback values used in the function were not in line with the default/fallback values used in the wp_insert_post() function.

The wp_insert_post() function does a wp_parse_args() (array merge) of the received arguments with the defaults. If any of the received arguments are null, this would overwrite the default value, as seen in array_merge() example, and lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

This commit:

  • Ensures that all arguments are defined before they are compact()'ed together to the arguments array.
  • Verifies that the default/fallback value of the arguments as set within the wp_xmlrpc_server::mw_newPost() method are the same as the default/fallback values used in the wp_insert_post() function.
  • Verifies that arguments which do not have a default/fallback value defined in the wp_insert_post() function are handled correctly.
    • This was not the case for $post_name, which would previously already get an empty string default value in the wp_xmlrpc_server::mw_newPost() function, but then in the wp_insert_post() function, this would prevent the slug generation from being activated. Fixed now by setting the default in the wp_xmlrpc_server::mw_newPost() function to null.
    • The page_template argument was handled, but not documented in the wp_insert_post() function. The argument is now documented in the wp_insert_post() function DocBlock. Note: There are more than likely several other potential arguments missing from that list, but verifying the whole list is outside the scope of this particular commit.

Includes minor simplifications, such as:

  • Setting a default ahead of an if, instead of in an else clause (as long as no function call is needed to set the default).
  • Removing the unnecessary logic duplication in the $post_status switch.
  • Using a combined concatenation + assignment operator for adding $post_more.

Fixes various errors along the lines of:

1) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2497
/var/www/src/wp-includes/class-wp-hook.php:308
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5616
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315

...

23) Tests_XMLRPC_mw_editPost::test_draft_not_prematurely_published
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:5615
/var/www/tests/phpunit/tests/xmlrpc/mw/editPost.php:315
/var/www/vendor/bin/phpunit:123

Follow-up to [1563], [4793], [7900], [16824], [19848], [40677], [51968].

Props jrf.
See #55656.

#42 @SergeyBiryukov
3 years ago

In 54321:

Code Modernization: Fix null to non-nullable deprecations in wp_xmlrpc_server::_insert_post().

The wp_xmlrpc_server::_insert_post() method creates a new post via wp_insert_post() or updates an existing one via wp_update_post(), which subsequently calls wp_insert_post(). However, the default/fallback values used in the function were not in line with the default/fallback values used in the wp_insert_post() function.

The wp_insert_post() function does a wp_parse_args() (array merge) of the received arguments with the defaults. If any of the received arguments are null, this would overwrite the default value, as seen in array_merge() example, and lead to "passing null to non-nullable" deprecation notices on PHP 8.1 for certain arguments.

Unfortunately, the conditional logic within the wp_xmlrpc_server::_insert_post() function itself often uses an isset() to trigger certain code blocks, so syncing the defaults with those used in the wp_insert_post() function was not an option.

This commit:

  • Updates the default/fallback values in the $defaults array only for those values where this would not lead to a change in the behavior of the function.
  • Adds a safeguard function, filtering out all remaining null values from the $post_data array before it is passed on to the wp_insert_post() or wp_update_post() functions. Removing those values is safe as this means that these array keys will now:
    • either be set to the default/fallback value as defined in wp_insert_post().
    • or not be set and for those values which don't have a default/fallback value in wp_insert_post(), the function does an ! empty() or isset() check anyway and those array keys not being defined means that the result of those checks will remain the same.

Includes

  • Removing a couple of conditions which are now redundant.
  • Removing an expectDeprecation() in the Tests_Date_XMLRPC test class, which is now no longer needed.

Fixes various errors along the lines of:

36) Tests_XMLRPC_wp_newPost::test_no_content
json_decode(): Passing null to parameter #1 ($json) of type string is deprecated

/var/www/src/wp-includes/kses.php:2074
/var/www/src/wp-includes/class-wp-hook.php:307
/var/www/src/wp-includes/plugin.php:205
/var/www/src/wp-includes/post.php:2835
/var/www/src/wp-includes/post.php:2720
/var/www/src/wp-includes/post.php:4066
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1683
/var/www/src/wp-includes/class-wp-xmlrpc-server.php:1347
/var/www/tests/phpunit/tests/xmlrpc/wp/newPost.php:25
/var/www/vendor/bin/phpunit:123

Follow-up to [1563], [4793], [7900], [16824], [19848], [19873], [20632], [40677], [51968], [54320].

Props jrf.
See #55656.

SergeyBiryukov commented on PR #3187:


3 years ago
#43

Thanks for the PR! Merged in r54320 and r54321.

SergeyBiryukov commented on PR #1658:


3 years ago
#44

Thanks for the PR! I believe this was superseded by #3187.

jrfnl commented on PR #3187:


3 years ago
#45

Thanks @SergeyBiryukov !

jrfnl commented on PR #1658:


3 years ago
#46

I believe this was superseded by #3187.

Correct. Thanks for getting the improved version merged!

jrfnl commented on PR #3188:


3 years ago
#47

Closing this PR.

The first commit has been merged into Core. The second commit has been moved to a separate PR: https://github.com/WordPress/wordpress-develop/pull/3344

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

#49 @SergeyBiryukov
3 years ago

In 54349:

Code Modernization: Use correct default value for JavaScript translations path.

The $path parameter of load_script_textdomain() had a default value of null, but would be passed onto the untrailingslashit() function without any input validation, even though the latter explicitly only expects/supports a string input.

This commit changes the default value for $path to an empty string, and adds an is_string() check before passing the value to untrailingslashit() to fix the issue at the point where the invalid input is incorrectly (not) validated.

Note: Changing the untrailingslashit() function is outside the scope of this commit.

Includes:

  • Adding a dedicated unit test for this issue.
  • Correcting the default value for $path from null to an empty string in a few related methods and functions:
    • WP_Dependency::set_translations()
    • WP_Scripts::set_translations()
    • wp_set_script_translations()
    • load_script_textdomain()

This fix also allows to remove a couple of calls to expectDeprecation() in unrelated tests.

Fixes an error when running the test suite:

4) Tests_Dependencies_Scripts::test_wp_external_wp_i18n_print_order
rtrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2782
/var/www/src/wp-includes/l10n.php:1068
/var/www/src/wp-includes/class.wp-scripts.php:605
/var/www/src/wp-includes/class.wp-scripts.php:320
/var/www/src/wp-includes/class.wp-dependencies.php:136
/var/www/src/wp-includes/functions.wp-scripts.php:109
/var/www/tests/phpunit/tests/dependencies/scripts.php:1505
/var/www/tests/phpunit/includes/utils.php:436
/var/www/tests/phpunit/tests/dependencies/scripts.php:1507
/var/www/vendor/bin/phpunit:123

Follow-up to [44169], [44607], [51968].

Props jrf, ocean90, Chouby, swissspidy, lovor, iviweb, meysamnorouzi, DarkoG, oneearth27, SergeyBiryukov.
Fixes #55967. See #55656.

SergeyBiryukov commented on PR #3186:


3 years ago
#50

Thanks for the PR! Merged in r54349.

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


3 years ago

#52 @SergeyBiryukov
3 years ago

In 54351:

I18N: Use correct default value for JavaScript translations path.

The $path parameter of some script translation functions had a default value of null, even though the parameter is documented as a string.

This commit corrects the default value for $path in:

  • WP_Dependency::set_translations()
  • WP_Scripts::set_translations()
  • wp_set_script_translations()

Additionally, this commit removes an is_string() check for $path in load_script_textdomain(). Now that the default value for $path in that function has also been corrected to an empty string instead of null, that check is no longer necessary, as it would hide an error which should be fixed (at the source of the problem) instead.

Follow-up to [54349].

Props jrf, johnjamesjacoby.
See #55967, #55656.

This ticket was mentioned in Slack in #hosting-community by jrf. View the logs.


3 years ago

#54 @tillkruess
3 years ago

Return type of Requests_Cookie_Jar::offsetExists($key) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Cookie/Jar.php:63

Return type of Requests_Cookie_Jar::offsetGet($key) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Cookie/Jar.php:73

Return type of Requests_Cookie_Jar::offsetSet($key, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Cookie/Jar.php:89

Return type of Requests_Cookie_Jar::offsetUnset($key) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Cookie/Jar.php:102

Return type of Requests_Cookie_Jar::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Cookie/Jar.php:111

#55 @tillkruess
3 years ago

Return type of Requests_Utility_CaseInsensitiveDictionary::offsetExists($key) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Utility/CaseInsensitiveDictionary.php:40

Return type of Requests_Utility_CaseInsensitiveDictionary::offsetGet($key) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Utility/CaseInsensitiveDictionary.php:51

Return type of Requests_Utility_CaseInsensitiveDictionary::offsetSet($key, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Utility/CaseInsensitiveDictionary.php:68

Return type of Requests_Utility_CaseInsensitiveDictionary::offsetUnset($key) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Utility/CaseInsensitiveDictionary.php:82

Return type of Requests_Utility_CaseInsensitiveDictionary::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
wp-includes/Requests/Utility/CaseInsensitiveDictionary.php:91

#56 @tillkruess
3 years ago

rtrim(): Passing null to parameter #1 ($string) of type string is deprecated
wp-includes/formatting.php:2772

#57 @tillkruess
3 years ago

http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated
wp-includes/Requests/Transport/cURL.php:345

#58 @tillkruess
3 years ago

I run into these deprecation notices frequently and it's breaking HTTP redirects.

It would make sense to search for all getIterator() instances and make sure they have the new #[\ReturnTypeWillChange].

#59 @jrf
3 years ago

@tillkruess Thanks for reporting these.

Regarding the notices you reported in comment:54, comment:55 and comment:57: these are all related to the external dependency Requests for which a new version is available which fixes all that.
There is already a ticket open to upgrade the dependency - see #54504. You may want to subscribe to that ticket to track progress.

Regarding the issue reported in comment:56:

rtrim(): Passing null to parameter #1 ($string) of type string is deprecated
wp-includes/formatting.php:2772

This needs a backtrace to find out what is causing the notice. This should be fixed in the originating function call and without backtrace that cannot be determined.

Last edited 3 years ago by jrf (previous) (diff)

#60 follow-up: @tillkruess
3 years ago

I don't have a backtrace for:

rtrim(): Passing null to parameter #1 ($string) of type string is deprecated in wp-includes/formatting.php:2772

However it appears to be the load_script_textdomain() method calling untrailingslashit(), which calls rtrim().

https://github.com/WordPress/wordpress-develop/blob/62360f5f4b222a4db53e99cdf56df04b66421cf0/src/wp-includes/l10n.php#L1068

#61 in reply to: ↑ 60 @jrf
3 years ago

Replying to tillkruess:

I don't have a backtrace for:

rtrim(): Passing null to parameter #1 ($string) of type string is deprecated in wp-includes/formatting.php:2772

However it appears to be the load_script_textdomain() method calling untrailingslashit(), which calls rtrim().

https://github.com/WordPress/wordpress-develop/blob/62360f5f4b222a4db53e99cdf56df04b66421cf0/src/wp-includes/l10n.php#L1068

In that case, that's already been fixed and the fix will be included in WP 6.1.
Relevant commits: [54349] and [54351]

#62 @SergeyBiryukov
3 years ago

In 54364:

Code Modernization: Check the return type of parse_url() in url_to_postid().

As per the PHP manual:

If the component parameter is omitted, an associative array is returned.
If the component parameter is specified, parse_url() returns a string (or an int, in the case of PHP_URL_PORT) instead of an array. If the requested component doesn't exist within the given URL, null will be returned.

Reference: PHP Manual: parse_url(): Return Values

In this case, parse_url() is called with PHP_URL_HOST as $component, which returns null if the URL only has a path. The return value of parse_url() was then passed to str_replace(), leading to a notice on PHP 8.1:

str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated

Adding validation for the return type value of parse_url() prevents that.

This commit addresses a few errors in the test suite along the lines of:

5) Tests_Rewrite::test_url_to_postid_home_has_path
str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated

/var/www/src/wp-includes/rewrite.php:503
/var/www/tests/phpunit/tests/rewrite.php:271
/var/www/vendor/bin/phpunit:123

Includes adding a dedicated unit test for a URL that only has a path.

Follow-up to [41786], [51606], [51622], [51626], [51629], [51630], [52799].

Props jrf, aristath, poena, justinahinon, SergeyBiryukov.
See #55656.

#63 @SergeyBiryukov
3 years ago

In 54365:

Tests: Ensure prerequisites are met for draft length tests in Tests_L10n.

These three tests for wp_dashboard_recent_drafts() would run into a PHP 8.1 "passing null to non-nullable" deprecation for the call to ltrim() when the result of get_edit_post_link() is passed to esc_url().

Setting a deprecation expectation would not solve this as the returned value would still not match the expected value(s).

The recent drafts list is only displayed on the Dashboard screen for users with the edit_posts capability. By setting the current user to Editor, the prerequisites for wp_dashboard_recent_drafts() are met, which means the deprecation notice is avoided and the assertions will succeed.

This commit addresses a few errors in the test suite along the lines of:

1) Tests_L10n::test_length_of_draft_should_be_counted_by_words
ltrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:4376
/var/www/src/wp-admin/includes/dashboard.php:657
/var/www/tests/phpunit/tests/l10n.php:449
/var/www/vendor/bin/phpunit:123

Follow-up to [45505], [52253], [52259].

Props jrf, desrosj, SergeyBiryukov.
See #56681, #55652, #55656.

#64 @SergeyBiryukov
3 years ago

In 54368:

Code Modernization: Correct default values in wp_handle_comment_submission().

This affects the following parameters subsequently passed to wp_new_comment():

  • comment_author
  • comment_author_email
  • comment_author_url
  • comment_content

The default values for these parameters were previously set to null, causing PHP 8.1 "null to non-nullable" deprecation notices when running sanitization filters on them via wp_filter_comment().

While the deprecation notices were temporarily silenced in the unit test suite, that caused an unexpected issue in a test for submitting a comment to a password protected post, where the $_COOKIE[ 'wp-postpass_' . COOKIEHASH ] value was no longer unset, as the test stopped any further execution once the deprecation notice was triggered.

Due to how WordPress handles password protected posts, once that value is set, it affects all posts protected with the same password, so this resulted in unintentionally affecting another test which happened to use the same password.

These values are all documented to be a string in various related filters, and core also expects them to be a string, so there is no reason for these defaults to be set to null. Setting them to an empty string instead resolves the issues.

This commit includes:

  • Setting the defaults in wp_handle_comment_submission() to an empty string.
  • Adding a dedicated unit test to verify the type of these default values.
  • Removing the deprecation notice silencing as no longer needed.

Follow-up to [34799], [34801], [51968].

Props jrf, desrosj, mukesh27, SergeyBiryukov.
Fixes #56712. See #56681, #55656.

#65 @SergeyBiryukov
3 years ago

In 54369:

Build/Test Tools: Remove PHP 8.1 and 8.2 from allowed failures.

With all known unit test failures now addressed, WordPress 6.1 aims to support PHP 8.1 and 8.2 as much as possible.

While full compatibility with PHP 8.1 and 8.2 is still a work in progress, this commit aims to actively prevent new PHP issues from being introduced in WordPress core.

All remaining known issues are deprecation notices. Please note, a deprecation notice is not an error, but rather an indicator of where additional work is needed for compatibility before PHP 9 (i.e. when the notices become fatal errors). With a deprecation notice, the PHP code will continue to work and nothing is broken.

Follow-up to [49077], [49162], [50299], [51588], [51604], [53922], [54072].

Props jrf, desrosj.
See #55652, #55656, #56009, #56681.

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


3 years ago

#67 @SergeyBiryukov
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

With 6.1 RC1 today, closing this ticket. Ongoing compatibility fixes will continue in #56790 during the 6.2 cycle.

Thank you everyone for your contributions!

This ticket was mentioned in Slack in #hosting-community by bernardzijlstra. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


3 years ago

@SergeyBiryukov commented on PR #2804:


2 years ago
#70

Thanks for the PR! Merged in r55301.

Note: See TracTickets for help on using tickets.