Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecation warning in WP-CLI on PHP 8.2 #34960

Closed
hirasso opened this issue Jan 11, 2024 · 12 comments · Fixed by #34962
Closed

Deprecation warning in WP-CLI on PHP 8.2 #34960

hirasso opened this issue Jan 11, 2024 · 12 comments · Fixed by #34962
Assignees
Labels
[Plugin] Super Cache A fast caching plugin for WordPress. [Pri] Low [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@hirasso
Copy link
Contributor

hirasso commented Jan 11, 2024

Impacted plugin

Super Cache

Quick summary

Super Cache causes a deprecation warning in some WP-CLI commands on PHP 8.2 (see below)

Steps to reproduce

1. Have this setup:

PHP Version

❯ php --version
PHP 8.2.0 (cli) (built: Dec  8 2022 17:48:05) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.0, Copyright (c) Zend Technologies

WP-CLI Version

❯ wp --version
WP-CLI 2.9.0

WP Version

❯ wp core version
6.4.2

Active plugins

❯ wp plugin list --status=active
+----------------+--------+--------+---------+
| name           | status | update | version |
+----------------+--------+--------+---------+
| wp-super-cache | active | none   | 1.11.0  |
+----------------+--------+--------+---------+

wp-config.php

define('WP_DEBUG', true);

2. Run the wp command that lists all commands:

❯ wp

Result: I get the following warning:

Deprecated: ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in /[...]/wp-includes/formatting.php on line 4494

3. Run the same command again, without Super Cache:

❯ wp --skip-plugins=wp-super-cache

Result: I get no warning.

A clear and concise description of what you expected to happen.

I'd expect to not get a deprecation warning. Something in super cache seems to pass null to esc_url.

What actually happened

I got the deprecation warning

Impact

All

Available workarounds?

No but the platform is still usable

Platform (Simple and/or Atomic)

Self-hosted

Logs or notes

No response

@hirasso hirasso added [Type] Bug When a feature is broken and / or not performing as intended Needs triage Ticket needs to be triaged labels Jan 11, 2024
@github-actions github-actions bot added [Plugin] Super Cache A fast caching plugin for WordPress. [Pri] High [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack labels Jan 11, 2024
jeherve added a commit that referenced this issue Jan 11, 2024
Fixes #34960

Avoid trying to fetch specific links when the global is not set.
@jeherve jeherve added [Pri] Low Triaged and removed [Pri] High Needs triage Ticket needs to be triaged labels Jan 11, 2024
@jeherve jeherve moved this from Needs Triage to Triaged in Automattic Prioritization: The One Board ™ Jan 11, 2024
@jeherve
Copy link
Member

jeherve commented Jan 11, 2024

I wasn't able to reproduce the issue on my end, but I tried to be more defensive in one place where I think this may come up in some scenarios. If you feel comfortable editing PHP files, could you apply the changes suggested in #34962, and let me know if that fixes the problem for you?

Thank you!

@hirasso
Copy link
Contributor Author

hirasso commented Jan 11, 2024

Thanks for you quick reaction @jeherve ! #34962 did not fix the issue for me.

I was able to further drill down to the source of the issue by doing this in /wp-includes/formatting.php#4494:

if (is_null($url)) {
	var_dump(debug_backtrace());
	exit;
}
$url = str_replace( ' ', '%20', ltrim( $url ) );

The third entry in the dump points to this:

  [2]=>
  array(4) {
    ["file"]=>
    string(97) "/Users/rah/Sites/wp-caching-plugins/wwwroot/wp-content/plugins/wp-super-cache/wp-cache-phase2.php"
    ["line"]=>
    int(394)
    ["function"]=>
    string(11) "esc_url_raw"
    ["args"]=>
    array(1) {
      [0]=>
      NULL
    }
  }

Turns out $wp_cache_request_uri in wp_cache_postload() is null in my case, causing the warning. Maybe this helps?

@hirasso
Copy link
Contributor Author

hirasso commented Jan 11, 2024

Just found a merged PR that seems to be related: #32629

While it would be nice to get this fixed in Super Cache, I really feel like this kind of thing should be taken care of in WordPress core. A simple empty check should be enough I think:

function esc_url( $url, $protocols = null, $_context = 'display' ) {
	$original_url = $url;

-	if ( '' === $url ) {
+	if ( empty($url) ) {
		return $url;
	}

        // ....

I would love to submit a PR to WP for that, but wasn't very lucky the first time I tried (no response since), so I'll leave it to you to escalate if you think it makes sense :)

@jeherve
Copy link
Member

jeherve commented Jan 11, 2024

Thanks for the extra details! I'll cc @donnchawp on this, since he worked on #32629.

@donnchawp
Copy link
Contributor

donnchawp commented Jan 16, 2024

With the changes to wp-cache-phase1.php to define $wp_cache_request_uri, I'm surprised a null value crept through.

However, if that variable is still null, we should probably disable caching. If the plugin doesn't know the URL loading WordPress, then it's not simple to cache it.

@hirasso
Copy link
Contributor Author

hirasso commented Jan 16, 2024

Shouldn't caching always be disabled if running inside WP-CLI?

At least I can't think of a scenario where I would need it 🙂

jeherve added a commit that referenced this issue Jan 16, 2024
Fixes #34960

Avoid trying to fetch specific links when the global is not set.
@jeherve jeherve reopened this Jan 16, 2024
@hirasso
Copy link
Contributor Author

hirasso commented Jan 16, 2024

@donnchawp when I run wp commands, wp-cache-phase1.php doesn't seem to be executed, at all. I tested this by adding this to the very top of the file:

var_dump('wp-cache-phase1.php executed'); exit;

Output from a test command:

❯ wp plugin list
Deprecated: ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in /Users/rah/Sites/vanillawp/wwwroot/wp-includes/formatting.php on line 4497
+--------------------+----------+--------+---------+
| name               | status   | update | version |
+--------------------+----------+--------+---------+
| akismet            | inactive | none   | 5.3     |
| hello              | inactive | none   | 1.7.2   |
| wp-super-cache     | active   | none   | 1.11.0  |
| advanced-cache.php | dropin   | none   |         |
+--------------------+----------+--------+---------+

I'd expect the output to be this if wp-cache-phase1.php was being loaded:

❯ wp plugin list
string(28) "wp-cache-phase1.php executed"

As a side note: I'm testing this on a vanilla 6.4.2 WordPress install with the twentytwentyfour theme and only wp-super-cache activated.

@hirasso
Copy link
Contributor Author

hirasso commented Jan 16, 2024

I'm happy to provide further information about my environment if it helps 🙂

@donnchawp
Copy link
Contributor

Thanks for debugging that some more. I think what is happening here is that wp_cache_postload() is being called in wp-settings.php, regardless of whether wp-cache-phase1.php is loaded or not. I should probably move the logic out of that function in wp-cache-phase1.php as much as possible.

@donnchawp
Copy link
Contributor

I was able to replicate what you saw. When I added a null check to the postload function it disappeared.

@hirasso
Copy link
Contributor Author

hirasso commented Feb 2, 2024

@donnchawp are there any plans to release a fix for this sometime soon?

@donnchawp
Copy link
Contributor

donnchawp commented Feb 29, 2024

Fixed it in #36024.

Fix will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Super Cache A fast caching plugin for WordPress. [Pri] Low [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack Triaged [Type] Bug When a feature is broken and / or not performing as intended
Development

Successfully merging a pull request may close this issue.

3 participants