Performance Hints error: null passed to delete_by_url() when string expected

joejoe04 commented 3 weeks ago

Before submitting an issue please check that you’ve completed the following steps:

Made sure you’re on the latest version ✅
Used the search feature to ensure that the bug hasn’t been reported before ✅

Describe the bug Users on 3.16.4 reporting the following error being logged:

[14-Aug-2024 17:34:32 UTC] PHP Fatal error: Uncaught TypeError: Argument 1 passed to WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller::delete_by_url() must be of the type string, null given, called in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 81 and defined in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php:177 Stack trace: #0 /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php(81): WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller->delete_by_url(NULL) #1 /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Subscriber.php(64): WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller->delete_post(187146) #2 /home/customer/www/mabelkatz.com/public_html/espanol/wp-includes/class-wp-hook.php(326 in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 177

It seems some plugins are returning null for attachment $urls. The code above only returns if $url is exactly false, allowing null to get past.

Could instead do something like this:

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

To Reproduce Steps to reproduce the behavior:

Be on version 3.16.4 and use any plugin that returns null for attachment URLs

We're not sure exactly which plugins are doing this, but we have 3 cases reported so far that use a variety of different plugins.

@wordpressfan said it can be reproduced with this:

add_filter( 'attachment_link', function ( $permalink, $post_id ) {
    if ( 'attachment' === get_post_type( $post_id ) ) {
        return;
    }
    return $permalink;
}, 10, 2 );

Expected behavior Any time the $url value is not a string, we should return to stop processing.

Additional context Slack: https://wp-media.slack.com/archives/C43T1AYMQ/p1723658183566389 Case1: https://secure.helpscout.net/conversation/2679645797/507641/ Case2: https://secure.helpscout.net/conversation/2684619765/508416/ Case3: https://secure.helpscout.net/conversation/2685184550/508504/

Acceptance Criteria (for WP Media team use only) Clear instructions for developers, to be added before the grooming

webtrainingwheels commented 3 weeks ago

Another case: https://secure.helpscout.net/conversation/2691146803/509495?folderId=377611

sandyfigueroa commented 2 weeks ago

Another case: https://secure.helpscout.net/conversation/2696774398/510478/

kskonovalov commented 1 week ago

Faced the same issue preventing upload any media or plugins, error Uncaught TypeError: WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller::delete_by_url(): Argument #1 ($url) must be of type string, null given, called in wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php

Uploading media work again if deactivate WPRocket

Another way is a hotfix until it's fixed by plugin's authors

in wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php

replace the delete_by_url method with

private function delete_by_url( $url ) {
    if(is_string($url)) {
      foreach ($this->factories as $factory) {
        $factory->queries()->delete_by_url(untrailingslashit($url));
      }
    }
}

alfonso100 commented 5 days ago

another case here https://secure.helpscout.net/conversation/2706213063/512091?folderId=273766

Adrianadla commented 1 day ago

wp-media / wp-rocket

Performance Hints error: null passed to delete_by_url() when string expected #6887