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

kkmuffme commented 3 weeks ago

[x] I've read and understood the contribution guidelines.
[x] I've searched for any related issues and avoided creating a duplicate issue.

Please give us a description of what happened

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

wordpress-seo/src/helpers/first-time-configuration-notice-helper.php (line 127)

To Reproduce

Step-by-step reproduction instructions

open WooCommerce product csv import (direct url: /wp-admin/admin.php?import=woocommerce_product_csv)
get PHP notice

Expected results

no PHP notice

Actual results

PHP notice

filter_input will return null|false if the GET parameter is not set or in PHP CLI (even if it's set). Possibly use static analysis tools like psalm to prevent errors like those in the future

Used versions

PHP version: PHP 8.3
WordPress version: 6.5.3
WordPress Theme: storefront
Yoast SEO version: 22.8
Relevant plugins in case of a bug: woocommerce. However this happens with other plugins that have urls without the checked GET parameter specified too.

josevarghese commented 1 week ago

Hi @kkmuffme

Thanks for using the Yoast SEO plugin and for creating the issue.

I tried reproducing the issue by installing the Storefront theme, Yoast SEO 22.9. However, I could not notice the error you mentioned when I tried to access the Import product page within the Tools > Import > WooCommerce products (CSV). I also noticed that the import URL on my lab site is /wp-admin/edit.php?post_type=product&page=product_importer instead of the one you mentioned /wp-admin/admin.php?import=woocommerce_product_csv. So, please let us know whether you are using any plugins for importing the WooCommerce Product import other than the one provided by WooCommerce by default.

We look forward to hearing from you.

kkmuffme commented 1 week ago

Hi

possibly. In any case, this is purely a type/static analysis issue and doesn't need a reproducer. Trying to get a reproducer takes more time than to actually create a PR and fix this, so I don't think it's time well spent.

josevarghese commented 1 week ago

Hi @kkmuffme

We want to reproduce the issue before escalating it to the appropriate team, as line 127 within the file https://github.com/Yoast/wordpress-seo/blob/trunk/src/helpers/first-time-configuration-notice-helper.php#L127 is to check whether the page is a Yoast SEO admin page.

If you are interested in contributing to our plugin, please send a PR - see more info here: https://github.com/Yoast/wordpress-seo/blob/trunk/.github/CONTRIBUTING.md

Yoast / wordpress-seo