wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

From v3.13.1, conflict between WP Rocket and membership plugins like ultimate member, wpforo, and WishList Member

Open engahmeds3ed opened this issue 1 year ago • 9 comments

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

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

Describe the bug Starting from 3.13.1, and to be more specific in #5680 we received some tickets talking about wrong redirection and not valid behavior when having ultimate member and wpforo plugins active also we got a case with Wishlist Member plugin.

In those cases we managed to get the root cause in the following lines: https://github.com/wp-media/wp-rocket/blob/50dcab488d8594a6bbeea3bbd11ccb4a8df6d537/inc/functions/files.php#L590-L594

The problem in this WP function is that it's calling WP_Query class here which is calling the query method in its constructor here and this is calling get_posts method here that applies a filter called the_posts here this filter is used inside UM plugin to add the callback for the method filter_protected_posts inside their class um\core\Access there they are trying to filter the posts based on the attached restriction from their dashboard and if the restriction for a post/page is to redirect to another post/page it will redirect exactly there and that's weird (u can find it here) so it'll redirect and exit without continue in registration!

To Reproduce Steps to reproduce the behavior:

  1. Install and activate ultimate member plugin and continue the activation with creating the requested pages automatically with that button in admin notice
  2. Install and activate wpforo plugin (free forum plugin)
  3. go to the created page forum and make UM settings for this page as attached
  4. Visit the register page as incognito and complete the registration, you will be redirected to the url that u added in forum page and it should redirect you to the user profile page

image

Expected behavior the redirection should happen as it was before 3.13.1

Backlog Grooming (for WP Media dev team use only)

  • [ ] Reproduce the problem
  • [ ] Identify the root cause
  • [ ] Scope a solution
  • [ ] Estimate the effort

engahmeds3ed avatar Jun 10 '23 11:06 engahmeds3ed

Scope a solution

A solution would be to remove theses lines: https://github.com/wp-media/wp-rocket/blob/50dcab488d8594a6bbeea3bbd11ccb4a8df6d537/inc/functions/files.php#L590-L594 and the method rocket_clean_post_cache_on_slug_change inside files.php.

Then to add back the same logic that we deleted we will have to add trash status to the array on that line: https://github.com/wp-media/wp-rocket/blob/73ab0c312ad29b15dfb9970974da0a67bf19aad9/inc/common/purge.php#L615

Thanks @engahmeds3ed !

Estimate the effort

Effort XS

CrochetFeve0251 avatar Jun 21 '23 14:06 CrochetFeve0251

Just wanted to add a note here that if lines 590-594 are present in files.php, you may experience problems with people placing orders in Woocommerce. We found that a huge query was being generated by WP Rocket but only when a product had inventory enabled, and even then, orders would sometimes go through normally and generate errors other times. We think the query builds up over time and becomes more of an issue as it grows; the amount of time it takes to cause orders to fail may be dependent on server resources. The orders would actually go through in Woo, but the customer wouldn't see the thank you page due to an error and would attempt to place the order over and over. After quite a long back and forth with WP Rocket support, they pointed us to this issue, and commenting out lines 590-594 in files.php solved the problem. Hope to see this fix applied to the live plugin soon.

sc456a avatar Jul 03 '23 15:07 sc456a

Reopening this one per Remy's comments in the escalation here. https://wp-media.slack.com/archives/C056ZJMHG0P/p1701101281011479

I see that the original bug was related to the usage of url_to_postid(), and in 3.15.4 we added a modified copy of this function in our code for the preload https://github.com/wp-media/wp-rocket/compare/v3.15.3...v3.15.4#diff-13bb6ae74d8bc004dd6fbdb9a3d705ceb0c263bb9ffa6e4df10ebc19190589afR536

alfonso100 avatar Nov 29 '23 21:11 alfonso100

The issue was reintroduced because of the exclusion of private posts from the preload: https://github.com/wp-media/wp-rocket/blob/8b64400a9c36ec5c0a26df238946cea389fc2a34/inc/Engine/Preload/Subscriber.php#L552

piotrbak avatar Jan 15 '24 10:01 piotrbak

Access to dev environment where we can clearly see the issue.

piotrbak avatar Jan 17 '24 09:01 piotrbak

I don't know how to handle this one, @engahmeds3ed do you have an idea?

remyperona avatar Jan 22 '24 21:01 remyperona

I don't know how to handle this one, @engahmeds3ed do you have an idea?

Checking that now

wordpressfan avatar Jan 23 '24 13:01 wordpressfan

I validated now the following idea and seems like it's working properly:

What if we added a new argument here: https://github.com/wp-media/wp-rocket/blob/ae76c04ce384ba9077abb15e043cb770e4474b7c/inc/functions/posts.php#L82

something like:

function rocket_url_to_postid( string $url, array $search_in_post_statuses = [ 'publish', 'private' ] ) {

then here: https://github.com/wp-media/wp-rocket/blob/ae76c04ce384ba9077abb15e043cb770e4474b7c/inc/functions/posts.php#L242

change it to:

$query['post_status'] = $search_in_post_statuses;

Then in preload subscriber (that's where we use the function rocket_url_to_postid) here: https://github.com/wp-media/wp-rocket/blob/786ca3c926f843c1e8ce3133515cf699d18ba6b8/inc/Engine/Preload/Subscriber.php#L547-L559

replace it with:

	public function exclude_private_url( $excluded, string $url ): bool {
		if ( $excluded ) {
			return true;
		}

		$post_id = rocket_url_to_postid( $url, [ 'private' ] );

		return ! empty( $post_id );
	}

so here we will only search in private posts only and this will save a lot of time.

Also, what if we adjusted the wp_query args a bit to return only IDs and do some enhancements there.

@Tabrisrp what do u think?

wordpressfan avatar Jan 24 '24 09:01 wordpressfan

@engahmeds3ed looks good to me

remyperona avatar Feb 12 '24 20:02 remyperona

Moving back to in-progress to assess @hanna-meda 's findings https://wp-media.slack.com/archives/CUT7FLHF1/p1709125646503859

MathieuLamiot avatar Feb 28 '24 14:02 MathieuLamiot