-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Magento_CacheInvalidate mis-handles very large tag patterns when doing a PURGE #26255
Comments
Hi @moloughlin. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. @moloughlin do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?
|
…purge batching, regardless of the size and content of provided tag pattern.
Proposed solution: #26256 |
This issue is duplicate of #8815, but as it have quite a good description I think it's good to keep it open. |
…orrect tag splitting Fix static tests
…orrect tag splitting Fix static tests
@engcom-Alfa Thank you for verifying the issue. Unfortunately, not enough information was provided to acknowledge ticket. Please consider adding the following:
Once all required information is added, please add label |
✅ Confirmed by @engcom-Alfa Issue Available: @engcom-Alfa, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself. |
Hi @moloughlin. Thank you for your report.
The fix will be available with the upcoming 2.4.1 release. |
…ix incorrect tag splitting #26256
Preconditions:
\Magento\CacheInvalidate\Model\PurgeCache::sendPurgeRequest is responsible for taking arbitrarily large tag patterns and chunking it into HTTP PURGE requests based on the maximum header length Varnish will accept.
Currently, \Magento\CacheInvalidate\Observer\InvalidateVarnishObserver::execute will join tags with '|' and call sendPurgeRequest. Those tags themselves may include '|' characters.
\Magento\CacheInvalidate\Model\PurgeCache::splitTags will then separate on '|' and re-join components until the length of those components reaches the max. header length. This is too naive, since it may now split an individual tag (one which itself included '|' characters) across two requests. Both of those requests now have an invalid pattern - this would mean that at the very least, some purges will be missed. In the worst-case scenario, I expect this could also lead to Varnish being completely purged, depending on the tag that was incorrectly split.
For the sake of brevity, the example below has been limited to a small subset of my real-world example; as such let's pretend the max header size is 256 bytes. Note that this is a typical pattern that would result from a product save event.
((^|,)cat_p_1468(,|$))|((^|,)cat_p_1361(,|$))|((^|,)cat_p_1473(,|$))|((^|,)cat_p_930(,|$))|((^|,)cat_p_933(,|$))|((^|,)cat_p_934(,|$))|((^|,)cat_p_664(,|$))|((^|,)cat_p_487(,|$))|((^|,)cat_p_490(,|$))|((^|,)cat_p_491(,|$))|((^|,)cat_p_153(,|$))|((^|,)cat_p_156(,|$))|((^|,)cat_p_157(,|$))|((^|,)cat_p_497(,|$))|((^|,)cat_p_498(,|$))|((^|,)cat_p_499(,|$))|((^|,)cat_p_227(,|$))|((^|,)cat_p_228(,|$))|((^|,)cat_p_229(,|$))|((^|,)cat_p_242(,|$))|((^|,)cat_p_244(,|$))|((^|,)cat_p_245(,|$))
Based on my desired max. header size, this will be chunked into two separate requests:
((^|,)cat_p_1468(,|$))|((^|,)cat_p_1361(,|$))|((^|,)cat_p_1473(,|$))|((^|,)cat_p_930(,|$))|((^|,)cat_p_933(,|$))|((^|,)cat_p_934(,|$))|((^|,)cat_p_664(,|$))|((^|,)cat_p_487(,|$))|((^|,)cat_p_490(,|$))|((^|,)cat_p_491(,|$))|((^|,)cat_p_153(,|$))|((^
,)cat_p_156(,|$))|((^|,)cat_p_157(,|$))|((^|,)cat_p_497(,|$))|((^|,)cat_p_498(,|$))|((^|,)cat_p_499(,|$))|((^|,)cat_p_227(,|$))|((^|,)cat_p_228(,|$))|((^|,)cat_p_229(,|$))|((^|,)cat_p_242(,|$))|((^|,)cat_p_244(,|$))|((^|,)cat_p_245(,|$))|((^|,)cat_p_245(,|$))
Expected split behaviour:
((^|,)cat_p_1468(,|$))|((^|,)cat_p_1361(,|$))|((^|,)cat_p_1473(,|$))|((^|,)cat_p_930(,|$))|((^|,)cat_p_933(,|$))|((^|,)cat_p_934(,|$))|((^|,)cat_p_664(,|$))|((^|,)cat_p_487(,|$))|((^|,)cat_p_490(,|$))|((^|,)cat_p_491(,|$))|((^|,)cat_p_153(,|$))
((^|,)cat_p_156(,|$))|((^|,)cat_p_157(,|$))|((^|,)cat_p_497(,|$))|((^|,)cat_p_498(,|$))|((^|,)cat_p_499(,|$))|((^|,)cat_p_227(,|$))|((^|,)cat_p_228(,|$))|((^|,)cat_p_229(,|$))|((^|,)cat_p_242(,|$))|((^|,)cat_p_244(,|$))|((^|,)cat_p_245(,|$))
Steps to reproduce:
Actual Result:
Observe two resulting PURGE requests sent to Varnish with invalid patterns (e.g. xdebug_break inside \Magento\CacheInvalidate\Model\PurgeCache::sendPurgeRequestToServers and observe the X-Magento-Tags-Pattern header)
Expected Result:
The text was updated successfully, but these errors were encountered: