-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Build query string array from url stripped of marketing params instead of original request uri #40349
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
base: 2.4-develop
Are you sure you want to change the base?
Conversation
…d of original request uri.
|
Hi @NateSwanson7. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
1 similar comment
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
4 similar comments
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run all tests |
Description (*)
This pull request fixes an issue in the built-in Full Page Cache (FPC) where
marketing/tracking query parameters such as utm_*, gclid, fbclid, dclid, etc.
were unintentionally being added back into the FPC cache identifier.
Magento 2.4-develop introduced logic in Identifier::getValue() to strip these
parameters from the request URI before generating the cache key. However,
reconstructUrl() rebuilt the query string using the original request's
query parameters via $this->request->getUri()->getQueryAsArray(), which
still contained all marketing parameters.
As a result, although the sanitized URL correctly removed marketing parameters,
the reconstructed query string reintroduced them, causing the built-in FPC to
generate separate cache entries for URLs that differed only by marketing params.
This PR corrects reconstructUrl() so that it parses the query string
from the sanitized $url passed into the method. The sanitized query is
then normalized (ksort + http_build_query) and used in the cache key, ensuring
consistent behavior and preventing unnecessary cache fragmentation.
Fixed Issues (if relevant)
Manual testing scenarios (*)
Enable built-in Full Page Cache.
Visit a storefront page with marketing parameters in the URL, e.g.:
https://example.com/?utm_source=test&utm_medium=cpc&gclid=ABC123&foo=bar
Enable cache debug tools, or temporarily add logging in
Magento\Framework\App\PageCache\Identifier::getValue() to inspect the
generated identifier value.
Observe the following behavior before this fix:
reconstructUrl() rebuilds the query from the original request query array.
Apply this patch and reload the same URL.
Verify that:
Confirm that non-marketing query parameters (e.g. ?foo=bar) remain part of
the FPC key as expected.
Questions or comments
If maintainers prefer this logic to live outside reconstructUrl(), or want
a different mechanism for parameter stripping prior to FPC key generation,
I am happy to adjust the implementation.
If unit test coverage is desired for this method, I can add tests to assert
that marketing parameters are excluded and that query ordering is normalized.
Contribution checklist (*)