#55656 closed task (blessed) (fixed)
PHP 8.x: various compatibility fixes for WordPress 6.1
Reported by: |
|
Owned by: |
|
---|---|---|---|
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:
- PHP 8.2: keyword is
php82
with its report https://core.trac.wordpress.org/query?keywords=~php82 - PHP 8.1: keyword is
php81
with its report https://core.trac.wordpress.org/query?keywords=~php81 - PHP 8.0: keyword is
php8
with its report https://core.trac.wordpress.org/query?keywords=~php8
Attachments (2)
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
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.
This ticket was mentioned in PR #2680 on WordPress/wordpress-develop by SergeyBiryukov.
3 years ago
#4
Trac ticket: https://core.trac.wordpress.org/ticket/55656
@
3 years ago
Deprecated notices in #wp-auth-check interstitial – PHP 8.1.6 here – preg_replace(), trim(), rtrim()
#6
@
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).
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
#9
@
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
@
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
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 thenull
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 thewp_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 thewp_xmlrpc_server::mw_newPost()
function, but then in thewp_insert_post()
function, this would prevent the slug generation from being activated.
- This was not the case for
Fixed now by setting the default in the
wp_xmlrpc_server::mw_newPost()
function tonull
..
- The
page_template
argument was handled, but not documented in thewp_insert_post()
function. I've now documented the argument in thewp_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 anelse
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 thewp_insert_post()
orwp_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()
orisset()
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
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
@
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:
- GH PR #3181 - GH Actions vs PHP 8.1 tweaks
- GH PR #3184 - PHP 8.1 | WP_Customize_Manager::get_return_url(): bug fix
- GH PR #3185 - PHP 8.1 | WP_Scripts::localize(): fix autovivification from false to array bug
- GH PR #3186 - PHP 8.1 | load_script_textdomain(): fix passing null to non-nullable
- GH PR #3187 - PHP 8.1 | wp_xmlrpc_server::(mw_newPost|_insert_post)(): fix passing null to non-nullable and more
- GH PR #3188 - PHP 8.1 | WP_REST_Users_Controller::update_item(): fix passing null to non-nullable
Reviews/merges appreciated ;-)
3 years ago
#25
@ocean90 I wholeheartedly support that effort. Will leave a comment on the other PR though.
SergeyBiryukov commented on PR #3181:
3 years ago
#27
Thanks for the PR! Merged in r54072.
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 ;-)
SergeyBiryukov commented on PR #3184:
3 years ago
#31
Thanks for the PR! Merged in r54135.
SergeyBiryukov commented on PR #3185:
3 years ago
#33
Thanks for the PR! Merged in r54142.
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.
SergeyBiryukov commented on PR #3187:
3 years ago
#43
SergeyBiryukov commented on PR #1658:
3 years ago
#44
Thanks for the PR! I believe this was superseded by #3187.
3 years ago
#46
I believe this was superseded by #3187.
Correct. Thanks for getting the improved version merged!
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
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
This ticket was mentioned in Slack in #hosting-community by jrf. View the logs.
3 years ago
#54
@
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
@
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
@
3 years ago
rtrim(): Passing null to parameter #1 ($string) of type string is deprecated wp-includes/formatting.php:2772
#57
@
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
@
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
@
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.
#60
follow-up:
↓ 61
@
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()
.
#61
in reply to:
↑ 60
@
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:2772However it appears to be the
load_script_textdomain()
method callinguntrailingslashit()
, which callsrtrim()
.
In that case, that's already been fixed and the fix will be included in WP 6.1.
Relevant commits: [54349] and [54351]
This ticket was mentioned in Slack in #core by sergey. View the logs.
3 years ago
#67
@
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.
The PHP native
trim()
function expects to be passed a string and it is not a nullable parameter. Passingnull
to it will result in atrim(): 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 returningfalse
. Whyis_scalar()
? For backwards-compatible, the native behavior oftrim()
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: