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

Argument 2 ($url) passed to exclude_rocket_from_wp_updates() must be of the type string #7042

Closed
joejoe04 opened this issue Oct 17, 2024 · 15 comments · Fixed by #7059
Closed
Assignees
Labels
priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value
Milestone

Comments

@joejoe04
Copy link

joejoe04 commented Oct 17, 2024

Describe the bug
User reported the following error with version 3.17.1 (incomplete stack trace was provided):

Se ha producido un error del tipo E_ERROR en la línea 135 del archivo /home/customer/www/example.net/public_html/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php. Mensaje de error: Uncaught TypeError: Argument 2 passed to WP_Rocket\Engine\Plugin\UpdaterSubscriber::exclude_rocket_from_wp_updates() must be of the type string, null given, called in /home/customer/www/example.net/public_html/wp-includes/class-wp-hook.php on line 324 and defined in /home/customer/www/example.net/public_html/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php:135 Stack trace: #0 /home/customer/www/example.net/public_html/wp-includes/class-wp-hook.php(324): WP_Rocket\Engine\Plugin\UpdaterSubscriber->exclude_rocket_from_wp_updates(Array, NULL) #1 /home/customer/www/example.net/public_html/wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array) #2 /home/customer/www/example.net/public_html/wp-includes/class-wp-http.php(234): apply_filters('http_request_ar...', Array, NULL) #3 /home/customer/www/example.net/public_html/wp-includes/class-wp-http.php(638): WP_Http->request(NULL, Array) #4 /home/customer/www/example.net/public_html/wp-includes/http.php(184): WP_Http->get(NULL, Array) #5 /h

It relates to this line:

public function exclude_rocket_from_wp_updates( $request, string $url ) {

I can see a recent update to the related method here:
7348869
https://imageshack.com/a/img922/4788/YNsWPZ.png

The change removed the check to make sure $url is a string, and it opens us up to errors like these.

To Reproduce
Steps to reproduce the behavior:

  1. Update to version 3.17.1
  2. Not sure exactly what other condition is required. Could be a conflict with another plugin or theme.
  3. Anything that could cause the $url parameter to not be of type string.

Expected behavior
We should check to confirm the $url parameter is a string to prevent this error from happening.

Additional context
Ticket - https://secure.helpscout.net/conversation/2737001255/518317
Slack - https://wp-media.slack.com/archives/C43T1AYMQ/p1729176825682779

@joejoe04
Copy link
Author

@piotrbak piotrbak added priority: medium Issues which are important, but no one will go out of business. needs: grooming severity: moderate Feature isn't working as expected but has work around to get same value labels Oct 18, 2024
@piotrbak
Copy link
Contributor

The complete stack trace is in the ticket above

@alfonso100
Copy link
Contributor

alfonso100 commented Oct 21, 2024

@alfonso100
Copy link
Contributor

another one: https://secure.helpscout.net/conversation/2740067797/518850?folderId=2008123

error stack

/public/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php on line 135
[21-Oct-2024 08:37:27 UTC] PHP Fatal error: Uncaught TypeError: WP_Rocket\Engine\Plugin\UpdaterSubscriber::exclude_rocket_from_wp_updates(): Argument #2 ($url) must be of type string, null given, called in /public/wp-includes/class-wp-hook.php on line 324 and defined in /public/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php:135
Stack trace:
#0 /public/wp-includes/class-wp-hook.php(324): WP_Rocket\Engine\Plugin\UpdaterSubscriber->exclude_rocket_from_wp_updates()
#1 /public/wp-includes/plugin.php(205): WP_Hook->apply_filters()
#2 /public/wp-includes/class-wp-http.php(234): apply_filters()
#3 /public/wp-includes/class-wp-http.php(638): WP_Http->request()
#4 /public/wp-includes/http.php(184): WP_Http->get()
**#5 /public/wp-content/plugins/bizreview-pro/inc/google-review/google-api-config.php(50): wp_remote_get()**
#6 /public/wp-content/plugins/bizreview-pro/inc/functions.php(28): BizReview_Google_API->get_place_data()
#7 /public/wp-includes/class-wp-hook.php(324): bizreview_google_api()

@UpworksTeam
Copy link

UpworksTeam commented Oct 21, 2024

The same thing here on 3 websites after updating from 3.17.0.2 to 3.17.1

@viobru
Copy link
Contributor

viobru commented Oct 22, 2024

@alfonso100
Copy link
Contributor

alfonso100 commented Oct 23, 2024

@piotrbak piotrbak added this to the 3.17.3 milestone Oct 23, 2024
@Khadreal
Copy link
Contributor

I think we have few instances where $url is not set.

Scope a solution ✅

We can add a nullable type hint to the ?string $url here and a condition to check if $url is not string

if( is_null( $url ) ) {
    return $request;
}

Estimate the effort ✅

[XS]

@remyperona
Copy link
Contributor

IMO, we should revert to what was there before: no typehint in the method declaration, and a check for is_string() inside the method. We can't know if a plugin will be modifying the URL value to a specific type, some plugins use null, others might use false.

@joejoe04
Copy link
Author

@joejoe04
Copy link
Author

joejoe04 commented Oct 30, 2024

Khadreal added a commit that referenced this issue Nov 1, 2024
@hanna-meda hanna-meda self-assigned this Nov 4, 2024
@salvia34
Copy link

salvia34 commented Nov 5, 2024

I'm still experiencing this issue in the latest WP Rocket version (v3.17.2).

Steps to reproduce the behavior: Just load the WP plugins page (/wp-admin/plugins.php)

I'm getting the following error:

Fatal error: Uncaught TypeError: WP_Rocket\Engine\Plugin\UpdaterSubscriber::exclude_rocket_from_wp_updates(): Argument #2 ($url) must be of type string, null given, called in /home/terroir/public_html/wp-includes/class-wp-hook.php on line 324 and defined in /home/terroir/public_html/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php:135 Stack trace: #0 /home/terroir/public_html/wp-includes/class-wp-hook.php(324): WP_Rocket\Engine\Plugin\UpdaterSubscriber->exclude_rocket_from_wp_updates(Array, NULL) #1 /home/terroir/public_html/wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array) #2 /home/terroir/public_html/wp-includes/class-wp-http.php(234): apply_filters('http_request_ar...', Array, NULL) #3 /home/terroir/public_html/wp-includes/class-wp-http.php(638): WP_Http->request(NULL, Array) #4 /home/terroir/public_html/wp-includes/http.php(184): WP_Http->get(NULL, Array) #5 /home/terroir/public_html/wp-content/plugins/hmenu/classes/core/checkin.class.php(42): wp_remote_get(NULL) #6 /home/terroir/public_html/wp-includes/class-wp-hook.php(326): hmenu_checkin->hmenu_check_in(Object(stdClass)) #7 /home/terroir/public_html/wp-includes/plugin.php(205): WP_Hook->apply_filters(Object(stdClass), Array) #8 /home/terroir/public_html/wp-includes/option.php(2576): apply_filters('pre_set_site_tr...', Object(stdClass), 'update_plugins') #9 /home/terroir/public_html/wp-includes/update.php(394): set_site_transient('update_plugins', Object(stdClass)) #10 /home/terroir/public_html/wp-includes/class-wp-hook.php(324): wp_update_plugins('') #11 /home/terroir/public_html/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array) #12 /home/terroir/public_html/wp-includes/plugin.php(517): WP_Hook->do_action(Array) #13 /home/terroir/public_html/wp-admin/admin.php(385): do_action('load-plugins.ph...') #14 /home/terroir/public_html/wp-admin/plugins.php(10): require_once('/home/terroir/p...') #15 {main} thrown in /home/terroir/public_html/wp-content/plugins/wp-rocket/inc/Engine/Plugin/UpdaterSubscriber.php on line 135

As a temporary workaround, I've edited the maybe_set_rocket_user_agent() function in UpdaterApiCommonSubscriber.php to check if $url is a string:

if ( ! is_string( $url ) ) {
    return $request;
}

However, after applying this fix, I encountered another fatal error, so I had to apply the same fix to the exclude_rocket_from_wp_updates() function in UpdaterSubscriber.php.

I hope this information is helpful in resolving this issue.

@piotrbak
Copy link
Contributor

piotrbak commented Nov 5, 2024

Hello @salvia34 the issue is will be fixed in the 3.17.3 version of WP Rocket. The proposed fix can be seen here:
https://github.com/wp-media/wp-rocket/pull/7059/files

github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
@jekayode
Copy link

jekayode commented Nov 7, 2024

@joejoe04
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value
Projects
None yet
10 participants