From 7b5655e8e7c8d1f7c5a7a82f20a9244d0cbdc2ab Mon Sep 17 00:00:00 2001 From: Pieter Hoste Date: Wed, 23 Nov 2022 22:34:38 +0100 Subject: [PATCH 1/2] Don't cache private content in Varnish, only 200 and 404 responses. Also fixes some whitespace inconsistencies. --- app/code/Magento/PageCache/etc/varnish4.vcl | 24 +++++++++--------- app/code/Magento/PageCache/etc/varnish5.vcl | 28 ++++++++++----------- app/code/Magento/PageCache/etc/varnish6.vcl | 28 ++++++++++----------- 3 files changed, 38 insertions(+), 42 deletions(-) diff --git a/app/code/Magento/PageCache/etc/varnish4.vcl b/app/code/Magento/PageCache/etc/varnish4.vcl index 0f67aae1e6975..7cc1b49bd4a3c 100644 --- a/app/code/Magento/PageCache/etc/varnish4.vcl +++ b/app/code/Magento/PageCache/etc/varnish4.vcl @@ -183,22 +183,22 @@ sub vcl_backend_response { unset beresp.http.set-cookie; } - # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass - if (beresp.ttl <= 0s || - beresp.http.Surrogate-control ~ "no-store" || - (!beresp.http.Surrogate-Control && - beresp.http.Cache-Control ~ "no-cache|no-store") || - beresp.http.Vary == "*") { - # Mark as Hit-For-Pass for the next 2 minutes + # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass + if (beresp.ttl <= 0s || + beresp.http.Surrogate-control ~ "no-store" || + (!beresp.http.Surrogate-Control && + beresp.http.Cache-Control ~ "no-cache|no-store") || + beresp.http.Vary == "*") { + # Mark as Hit-For-Pass for the next 2 minutes set beresp.ttl = 120s; set beresp.uncacheable = true; } - # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key - if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { - set beresp.ttl = 0s; - set beresp.uncacheable = true; - } + # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key + if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { + set beresp.ttl = 0s; + set beresp.uncacheable = true; + } return (deliver); } diff --git a/app/code/Magento/PageCache/etc/varnish5.vcl b/app/code/Magento/PageCache/etc/varnish5.vcl index bd9e5c92f5077..392e45085774e 100644 --- a/app/code/Magento/PageCache/etc/varnish5.vcl +++ b/app/code/Magento/PageCache/etc/varnish5.vcl @@ -168,10 +168,8 @@ sub vcl_backend_response { set beresp.http.X-Magento-Cache-Control = beresp.http.Cache-Control; } - # cache only successfully responses and 404s - if (beresp.status != 200 && - beresp.status != 404 && - beresp.http.Cache-Control ~ "private") { + # cache only successfully responses and 404s that are not marked as private + if ((beresp.status != 200 && beresp.status != 404) || beresp.http.Cache-Control ~ "private") { set beresp.uncacheable = true; set beresp.ttl = 86400s; return (deliver); @@ -182,22 +180,22 @@ sub vcl_backend_response { unset beresp.http.set-cookie; } - # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass - if (beresp.ttl <= 0s || - beresp.http.Surrogate-control ~ "no-store" || - (!beresp.http.Surrogate-Control && - beresp.http.Cache-Control ~ "no-cache|no-store") || - beresp.http.Vary == "*") { + # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass + if (beresp.ttl <= 0s || + beresp.http.Surrogate-control ~ "no-store" || + (!beresp.http.Surrogate-Control && + beresp.http.Cache-Control ~ "no-cache|no-store") || + beresp.http.Vary == "*") { # Mark as Hit-For-Pass for the next 2 minutes set beresp.ttl = 120s; set beresp.uncacheable = true; } - # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key - if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { - set beresp.ttl = 0s; - set beresp.uncacheable = true; - } + # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key + if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { + set beresp.ttl = 0s; + set beresp.uncacheable = true; + } return (deliver); } diff --git a/app/code/Magento/PageCache/etc/varnish6.vcl b/app/code/Magento/PageCache/etc/varnish6.vcl index 16dd9505e834b..3c990b8dac05e 100644 --- a/app/code/Magento/PageCache/etc/varnish6.vcl +++ b/app/code/Magento/PageCache/etc/varnish6.vcl @@ -173,9 +173,7 @@ sub vcl_backend_response { } # cache only successfully responses and 404s that are not marked as private - if (beresp.status != 200 && - beresp.status != 404 && - beresp.http.Cache-Control ~ "private") { + if ((beresp.status != 200 && beresp.status != 404) || beresp.http.Cache-Control ~ "private") { set beresp.uncacheable = true; set beresp.ttl = 86400s; return (deliver); @@ -186,22 +184,22 @@ sub vcl_backend_response { unset beresp.http.set-cookie; } - # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass - if (beresp.ttl <= 0s || - beresp.http.Surrogate-control ~ "no-store" || - (!beresp.http.Surrogate-Control && - beresp.http.Cache-Control ~ "no-cache|no-store") || - beresp.http.Vary == "*") { + # If page is not cacheable then bypass varnish for 2 minutes as Hit-For-Pass + if (beresp.ttl <= 0s || + beresp.http.Surrogate-control ~ "no-store" || + (!beresp.http.Surrogate-Control && + beresp.http.Cache-Control ~ "no-cache|no-store") || + beresp.http.Vary == "*") { # Mark as Hit-For-Pass for the next 2 minutes set beresp.ttl = 120s; set beresp.uncacheable = true; - } + } - # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key - if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { - set beresp.ttl = 0s; - set beresp.uncacheable = true; - } + # If the cache key in the Magento response doesn't match the one that was sent in the request, don't cache under the request's key + if (bereq.url ~ "/graphql" && bereq.http.X-Magento-Cache-Id && bereq.http.X-Magento-Cache-Id != beresp.http.X-Magento-Cache-Id) { + set beresp.ttl = 0s; + set beresp.uncacheable = true; + } return (deliver); } From f4e3b542218d4c7f993b32c6bd08f4a711093109 Mon Sep 17 00:00:00 2001 From: Guillaume Quintard Date: Thu, 25 Jan 2024 11:57:05 -0800 Subject: [PATCH 2/2] [vcl] test uncacheability rules (cherry picked from commit ca32d5ad3d19563d864a0ce0e6d0313a48227bbc) --- dev/tests/varnish/no-caching.vtc | 95 ++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 dev/tests/varnish/no-caching.vtc diff --git a/dev/tests/varnish/no-caching.vtc b/dev/tests/varnish/no-caching.vtc new file mode 100644 index 0000000000000..1144c91a23478 --- /dev/null +++ b/dev/tests/varnish/no-caching.vtc @@ -0,0 +1,95 @@ +varnishtest "X-Magento-Cache-Debug header" + +server s1 { + # first request will be the probe, handle it and be on our way + rxreq + expect req.url == "/health_check.php" + txresp + + # the probe expects the connection to close + close + accept + + # the client will send 2 requests for each url, the server should only see one + # of each in the cacheable case, and two otherwise + + # 405, Cache-Control doesn't matter + loop 2 { + rxreq + expect req.url == "/405uncacheable" + txresp -status 405 + } + + # 200, private Cache-Control + loop 2 { + rxreq + expect req.url == "/200uncacheable" + txresp -status 200 -hdr "cache-control: private" + } + + # 404, private Cache-Control + loop 2 { + rxreq + expect req.url == "/404uncacheable" + txresp -status 200 -hdr "cache-control: private" + } + + # 200, no Cache-Control + rxreq + expect req.url == "/200cacheable" + txresp -status 200 + + # 404, but still no Cache-Control + rxreq + expect req.url == "/404cacheable" + txresp -status 404 +} -start + +# generate usable VCL pointing towards s1 +# mostly, we replace the place-holders, but we also jack up the probe +# interval to avoid further interference +shell { + # testdir is automatically set to the directory containing the present vtc + sed \ + -e 's@\.interval = 5s;@.interval = 5m; .initial = 10;@' \ + -e 's@/\* {{ host }} \*/@${s1_addr}@' \ + -e 's@/\* {{ port }} \*/@${s1_port}@' \ + -e 's@/\* {{ ssl_offloaded_header }} \*/@unused@' \ + -e 's@/\* {{ grace_period }} \*/@0@' \ + ${testdir}/../../../app/code/Magento/PageCache/etc/varnish6.vcl > ${tmpdir}/output.vcl +} + +varnish v1 -arg "-f" -arg "${tmpdir}/output.vcl" -start + +# make sure the probe request fired +delay 1 + +client c1 { + loop 2 { + txreq -url /405uncacheable + rxresp + } + + loop 2 { + txreq -url /200uncacheable + rxresp + } + + loop 2 { + txreq -url /404uncacheable + rxresp + } + + loop 2 { + txreq -url /200cacheable + rxresp + } + + loop 2 { + txreq -url /404cacheable + rxresp + } +} -run + +# make sure s1 saw all the requests it was expecting +server s1 -wait