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

Query string caching with ignored parameter #1234

Closed
maximejobin opened this issue Oct 8, 2018 · 4 comments
Closed

Query string caching with ignored parameter #1234

maximejobin opened this issue Oct 8, 2018 · 4 comments
Assignees
Labels
module: cache priority: low Issues that can wait type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@maximejobin
Copy link

Based on this page, there are parameters that are ignore by default. For instance, gclid is one of them. According to that same page, it is also possible to cache query strings. The example on that page show country as an example.

From what I understand, the following URL:

http://www.mysite.com/?country=canada&gclid=1234

... should ignore the gclidparameter and serve the cache file from country=canada. This file shoud be located in:

wp-content/cache/wp-rocket/www.mysite.com/?country=canada/index.html

Instead, it is located in:

wp-content/cache/wp-rocket/www.mysite.com/?country=canada&gclid=1234/index.html

This means that gclid=1234 is not ignored when combined with a cached query string like country.

Am I missing something? I'm coding the rules for Rocket-Nginx and want to make sure I'm understanding the logic correctly. From what I understand, this is a bug. Is it?

@Tabrisrp
Copy link
Contributor

Tabrisrp commented Oct 9, 2018

It's ignored if it's the only query string parameter in the URL. In that case, the process continues and the default cache version is used.

If there is another query string parameter that should be cached, the URL used for the cache will still be the full one, including all query string parameters.

It's confusing and not ideal I agree. Ignored parameters should probably be removed from the cache filename in all cases to prevent that.

@maximejobin
Copy link
Author

maximejobin commented Oct 10, 2018

I totally agree that it is confusing and not ideal. The way you describe what is done is exactly how I understood it.

It makes cache useless in most cases. This should be flag as a bug and fixed, IMHO.

Also, why do all 3 utm_* parameters are required to be considered ?

@maximejobin maximejobin changed the title Query string caching with ignore parameter Query string caching with ignored parameter Oct 10, 2018
@Tabrisrp Tabrisrp added type: bug Indicates an unexpected problem or unintended behavior Severity: minor labels Oct 10, 2018
@Tabrisrp
Copy link
Contributor

I believe because if the 3 are set, we can know for sure that it's for campaigns and we can ignore them safely. But now there is other parameters existing too, so this also needs to be improved.

@ernee
Copy link

ernee commented Oct 26, 2018

Looks to be a similar case, however, the user was not caching any query strings. Caching was unexpected and caused some issues:

https://secure.helpscout.net/conversation/691607800/83647?folderId=1391600

@Tabrisrp Tabrisrp added this to the 3.2.1 milestone Oct 26, 2018
@Tabrisrp Tabrisrp self-assigned this Oct 26, 2018
maximejobin referenced this issue in SatelliteWP/rocket-nginx Oct 26, 2018
Facebook added a new tracking parameter fbclid, ignore them
@arunbasillal arunbasillal added the priority: low Issues that can wait label Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: cache priority: low Issues that can wait type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

4 participants