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

Check for null string before escaping #848

Closed
wants to merge 1 commit into from

Conversation

iandunn
Copy link

@iandunn iandunn commented Dec 7, 2023

Running a CLI command like wp cron event list on PHP 8.1 produces a deprecation notice:

[07-Dec-2023 15:29:29 UTC] PHP Deprecated:  ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in wp-includes/formatting.php on line 4494
[07-Dec-2023 15:29:29 UTC] PHP Stack trace:
[07-Dec-2023 15:29:29 UTC] PHP   1. {main}() /usr/local/bin/wp:0
[07-Dec-2023 15:29:29 UTC] PHP   2. include() /usr/local/bin/wp:4
[07-Dec-2023 15:29:29 UTC] PHP   3. include() phar:///usr/local/bin/wp/php/boot-phar.php:20
[07-Dec-2023 15:29:29 UTC] PHP   4. WP_CLI\bootstrap() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php:32
[07-Dec-2023 15:29:29 UTC] PHP   5. WP_CLI\Bootstrap\LaunchRunner->process($state = class WP_CLI\Bootstrap\BootstrapState { private $state = ['context_manager' => class WP_CLI\ContextManager { private $contexts = [...]; private $current_context = 'cli' }] }) phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/bootstrap.php:83
[07-Dec-2023 15:29:29 UTC] PHP   6. WP_CLI\Runner->start() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Bootstrap/LaunchRunner.php:28
[07-Dec-2023 15:29:29 UTC] PHP   7. WP_CLI\Runner->load_wordpress() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1282
[07-Dec-2023 15:29:29 UTC] PHP   8. require() phar:///usr/local/bin/wp/vendor/wp-cli/wp-cli/php/WP_CLI/Runner.php:1363
[07-Dec-2023 15:29:29 UTC] PHP   9. wp_cache_postload() wp-settings.php:496
[07-Dec-2023 15:29:29 UTC] PHP  10. esc_url_raw($url = NULL, $protocols = *uninitialized*) wp-content/plugins/wp-super-cache/wp-cache-phase2.php:394
[07-Dec-2023 15:29:29 UTC] PHP  11. sanitize_url($url = NULL, $protocols = NULL) wp-includes/formatting.php:4602
[07-Dec-2023 15:29:29 UTC] PHP  12. esc_url($url = NULL, $protocols = NULL, $_context = 'db') wp-includes/formatting.php:4620
[07-Dec-2023 15:29:29 UTC] PHP  13. ltrim($string = NULL) wp-includes/formatting.php:4494

This PR detects if it's null and passes an empty string instead. I didn't use ?? because that requires PHP 7.

This avoids a `ltrim(): Passing null to parameter #1 ($string) of type string is deprecated` notice on PHP 8.1.
Copy link

github-actions bot commented Dec 7, 2023

Thank you for your interest!

Pull requests should be made against the monorepo at https://github.com/Automattic/jetpack.

@github-actions github-actions bot closed this Dec 7, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
@iandunn
Copy link
Author

iandunn commented Dec 7, 2023

@anomiex , @jeherve What do you think about marking this repo as archived? That would make it more obvious that it's no longer the canonical source.

@jeherve
Copy link
Member

jeherve commented Dec 7, 2023

@iandunn The repo must remain, since the plugin is distributed from here to SVN, and available to folks consuming it via Composer from here.

Maybe we could add a mention to the readme though, making that clearer?

@anomiex
Copy link
Contributor

anomiex commented Dec 7, 2023

I note the description in the upper right already notes that it's read-only and "This repository is a mirror, for issue tracking and development head to: https://github.com/automattic/jetpack", like all of our mirror repos do.

Note that, as things are now, any change to the readme will also be present in the plugin itself (e.g. https://plugins.trac.wordpress.org/browser/wp-super-cache/trunk/README.md), so keep that in mind when editing it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants