wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
687 stars 215 forks source link

WP Rocket | Regex Exclusions helper not working due to recent code change #6151

Open joejoe04 opened 1 year ago

joejoe04 commented 1 year ago

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

Describe the bug Per the discussion here, the helper plugin to exclude from cache using regex does not seem to be currently working.

We think it's due to this new update in the code: https://github.com/wp-media/wp-rocket/commit/1e7145067e7946bb9fed70d2331a7c1aa78dcb25

When a regex is added to the helper like this one for example:

$uri[] = '(.*)not-a-test(.*)';

The output is like this:

$rocket_cache_reject_uri = '/(?:.+/)?feed(?:/(?:.+/?)?)?$|/(?:.+/)?embed/|http://(.*)not-a-test(.*)|/(index.php/)?(.*)wp-json(/.*|$)';

A protocol of http: is added like this: http://(.*)not-a-test(.*)

And it prevents the exclusion from cache from working (the page is still cached). The above should exclude this page:

https://i.joedisalvo.com/my-category/not-a-test/

But it does not unless I manually edit the config file so the exclusion is like this: (.*)not-a-test(.*)

Expected behavior When an exclusion regex is added via the helper plugin, the exclusion should be added to $rocket_cache_reject_uri without a protocol so it will work as intended.

Additional context Related Ticket: https://secure.helpscout.net/conversation/2346081976/439933/

Acceptance Criteria (for WP Media team use only)

  1. Exclude URLs filter should accept wildcards at the beginning
  2. WooCommerce URLs should be sanitized not to accept other values than paths
mostafa-hisham commented 1 year ago

Reproduce the problem

Yes following the same steps in the issue

Identify the root cause

the esc_url_raw function append http://

If the URL doesn't appear to contain a scheme, we presume

  • it needs http:// prepended (unless it's a relative link
  • starting with /, # or ?, or a PHP file)./

https://developer.wordpress.org/reference/functions/esc_url/#source

I don't if there is a similar function that do the same job with out the append part

Scope a solution

OR

Estimate the effort

[XS] This grooming still needs a review

engahmeds3ed commented 1 year ago

Exactly as @mostafa-hisham mentioned, the problem is that the Core function esc_url_raw is adding this http:// before the url.

https://github.com/WordPress/wordpress-develop/blob/c676279a1415524893c3921a0510641e972e8918/src/wp-includes/formatting.php#L4496-L4505

As u can see, it adds this http:// only when the sent string doesn't contain : AND doesn't start with one of those characters '/', '#', '?' AND is not a .php file

So what if we sanitized the string to only add the slash at the start of the string, for example

(.*)not-a-test(.*)

will be

/(.*)not-a-test(.*)

this will skip adding this http:// what do u think? @joejoe04 can u plz confirm if adding this slash solves the issue without affecting the base functionality?

Tabrisrp commented 1 year ago

I think it's a good idea to make sure the provided path starts with a slash

engahmeds3ed commented 12 months ago

@mostafa-hisham would you mind updating your grooming based on our last comments?

engahmeds3ed commented 12 months ago

After the discussion with @mostafa-hisham

Scope a solution

Here https://github.com/wp-media/wp-rocket/blob/56895a819512af1244920ffd5e6415efe796768c/inc/functions/options.php#L252 Before calling esc_url_raw function, we need to do the following check:

if ( empty( filter_var( $uri, FILTER_VALIDATE_URL ) ) ) {
    $uri = '/' . ltrim( $uri, '/' );
}

and adjust test.

Estimate the effort

[XS]

Miraeld commented 11 months ago

Hello, I can't reproduce the issue, seems like it's already checking if it starts with a /. On my end, if I don't start my regex with a /, it won't save anything.

vmanthos commented 11 months ago

@Miraeld I was able to reproduce the issue with the following steps:

  1. Edit $uri[] in the helper plugin to match this: $uri[] = '(.*)hello-world(.*)';
  2. Activate the helper plugin.
  3. Deactivate/reactivate WP Rocket - this will regenerate the configuration file and add the new exclusion there.
  4. Clear WP Rocket's cache.
  5. Visit https://yoursite.tld/hello-world. It will be cached/optimized.

@wp-media/tech-support It would be a good idea to update the helper plugin and run a function that will flush the htaccess file and regenerate the config file when activating/reactivating the helper: https://github.com/wp-media/wp-rocket-helpers/blob/96f9993ab414453fb63f3525c60fa1418fb32868/_wp-rocket-helper-plugin/wp-rocket-helper-plugin.php#L50-L63