Skip to content
This repository has been archived by the owner on Mar 9, 2024. It is now read-only.

🐞 Issues reported in debug.log #123

Closed
KTS915 opened this issue Jun 19, 2023 · 10 comments · Fixed by #161
Closed

🐞 Issues reported in debug.log #123

KTS915 opened this issue Jun 19, 2023 · 10 comments · Fixed by #161
Labels
status: discussion type: bug Something isn't working

Comments

@KTS915
Copy link
Member

KTS915 commented Jun 19, 2023

Expected behavior

Empty log

Current behavior

[19-Jun-2023 14:15:36 UTC] PHP Deprecated:  ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in /wp-includes/formatting.php on line 4414

[19-Jun-2023 14:15:36 UTC] PHP Deprecated:  ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in /wp-includes/formatting.php on line 4414

[19-Jun-2023 14:15:43 UTC] PHP Fatal error:  Uncaught TypeError: Cannot access offset of type string on string in /wp-admin/includes/class-wp-site-health.php:3053

Stack trace:
#0 /wp-includes/class-wp-hook.php(308): WP_Site_Health->wp_cron_scheduled_check()
#1 /wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters()
#2 /wp-includes/plugin.php(565): WP_Hook->do_action()
#3 /wp-cron.php(188): do_action_ref_array()
#4 {main}
  thrown in /wp-admin/includes/class-wp-site-health.php on line 3053

[19-Jun-2023 14:15:43 UTC] PHP Fatal error:  Uncaught TypeError: call_user_func(): Argument #1 ($callback) must be a valid callback, no array or string given in /wp-includes/functions.php:3722

Stack trace:
#0 /wp-includes/class-wp-fatal-error-handler.php(239): wp_die()
#1 /wp-includes/class-wp-fatal-error-handler.php(154): WP_Fatal_Error_Handler->display_default_error_template()
#2 /wp-includes/class-wp-fatal-error-handler.php(58): WP_Fatal_Error_Handler->display_error_template()
#3 [internal function]: WP_Fatal_Error_Handler->handle()
#4 {main}
  thrown in /wp-includes/functions.php on line 3722

Possible solution

No response

Steps to reproduce bug

Load a page

Context

No response

ClassicPress version

2.0

PHP version

8.1

Can you help?

I can help test a solution

@KTS915 KTS915 added status: needs triage This issue needs revision, splitting, or other gardening work type: bug Something isn't working labels Jun 19, 2023
@mattyrob
Copy link
Collaborator

@KTS915 - what theme and plugins are you using? I'm not seeing any of the ltrim() issues logged on my sites.

There is a fix for the Site Health section in #121

@KTS915
Copy link
Member Author

KTS915 commented Jun 19, 2023

I'm running no plugins and the default 2015 theme. But I have now identified that the ltrim() issues seem to be caused when a new page (on front-end or back-end) is visited immediately after my APCu object cache is cleared, so it's extremely hard to pin down what the real cause is.

I can see that ltrim() on line 4414 of /wp-includes/formatting.php is being run within the esc_url() function, so obviously what's happening somewhere is that there's an attempt to escape a URL that doesn't exist. But who knows where that non-existent URL is coming from ?!

@xxsimoxx
Copy link
Member

Seem two different issues.

  • One is fixed in Fix version check in Site Health #121
  • The other is a PHP compatibility issue that seems to be in many places, mainly in wp-includes/formatting.php. For this one maybe we can wait a fix for WP, even if fixing this is easy, so we handle those cases the same way.

@viktorix viktorix added status: discussion and removed status: needs triage This issue needs revision, splitting, or other gardening work labels Jul 6, 2023
@mattyrob
Copy link
Collaborator

A potential fix for the ltrim() issue is to replace the early check in the function from:

if ( '' === $url ) {

To:
if ( empty( $url ) ) {

@KTS915
Copy link
Member Author

KTS915 commented Jul 11, 2023

So that solution suggests that what's happening is that somewhere core is trying to escape something that's an object (or array, or integer, etc) and not a string. In that case, @mattyrob's "potential fix" is the right way to go.

@xxsimoxx
Copy link
Member

Here we used the null coalesce operator.

@KTS915
Copy link
Member Author

KTS915 commented Jul 11, 2023

But do we know it's an object here and not an array? If it's an array, null isn't correct.

@mattyrob
Copy link
Collaborator

According to upstream documentation esc_url() should be passed a string. The fucntion checks for and returns early is passed an empty string. I honestly don't know what happens if you pass an array or object (but it is an incorrect use of the function).

The errors we are seeing is because passing null to the function - so perhaps passing a variable that is undefined will pass the first conditional check because null is not the same as an empty string. But, when null is passed to ltrim PHP now flags a deprecation notice.

Using empty() is a good way to check for null and an empty string. But perhaps you are correct and the function should handle incorrect usage much better - maybe a check for is_string() and bail out with _doing_it_wrong() if that check fails.

@xxsimoxx
Copy link
Member

I think we should return the same values as before, as an example:

$x = wp_kses( null, array() );
var_dump( $x );
string(0) ""

So return an empty string on null.

@Soenke2
Copy link

Soenke2 commented Jul 19, 2023

... I have already reported about the same error (deprecation) with formatting.php, line 4414, and it still exists (php 8.1 and 8.2) ...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: discussion type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants