<div dir="ltr">Thanks for this.<div><br></div><div>I changed my sample VCL to be the following, and it seems to work as intended:</div><div><br></div><div><font face="monospace">vcl 4.1;<br><br>import std;<br><br>backend default {<br> .host = "localhost";<br> .port = "8081";<br>}<br><br>sub vcl_backend_response {<br> set beresp.keep = 5m;<br> set beresp.http.actual-status = beresp.status;<br> set beresp.status = 200;<br>}<br><br>sub vcl_deliver {<br> set resp.status = std.integer(resp.http.actual-status, 0);<br> unset resp.http.actual-status;<br>}<br></font></div><div><br></div><div>However, on further investigation, I'm not sure it's a good idea. Here's the commit that introduced the current behaviour: <a href="https://github.com/varnishcache/varnish-cache/commit/e99b5cfd886ec38a7f883e23ba516063cf4c16f8">https://github.com/varnishcache/varnish-cache/commit/e99b5cfd886ec38a7f883e23ba516063cf4c16f8</a>. The commit changes it from attempting conditional download of any cached object, to only those with status 200. I read through the RFC, and it appears it mandates this behaviour: <a href="https://datatracker.ietf.org/doc/html/rfc7232#section-4.1">https://datatracker.ietf.org/doc/html/rfc7232#section-4.1</a>, specifically:</div><div><br></div><div>> The 304 (Not Modified) status code indicates that a conditional GET or HEAD request has been received and would have resulted in a 200 (OK) response if it were not for the fact that the condition evaluated to false.</div><div><br></div><div>I.e. a conditional request that would have resulted in a 404 *cannot* respond with 304, so the cached 404 cannot be refreshed. This seems a bit of a shame, but I can't claim to know the RFC well enough to know if there's a strong reason for it to be this way.</div><div><br></div><div>Regards,</div><div><br></div><div>Mark</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, 15 Jul 2023 at 10:30, Dridi Boukelmoune <<a href="mailto:dridi@varni.sh">dridi@varni.sh</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sat, Jul 15, 2023 at 5:09 AM Guillaume Quintard<br>
<<a href="mailto:guillaume.quintard@gmail.com" target="_blank">guillaume.quintard@gmail.com</a>> wrote:<br>
><br>
> Hi Mark,<br>
><br>
> You are correct: <a href="https://github.com/varnishcache/varnish-cache/blob/varnish-7.3.0/bin/varnishd/cache/cache_fetch.c#L699-L703" rel="noreferrer" target="_blank">https://github.com/varnishcache/varnish-cache/blob/varnish-7.3.0/bin/varnishd/cache/cache_fetch.c#L699-L703</a><br>
><br>
> We only set the OF_IMSCAND flag (that we use to say that we can conditional download) if:<br>
> - the object is not a Hit-For-Miss (HFM)<br>
> - if the status is 200<br>
> - we either have a convincing Last-modified, or an Etag header<br>
><br>
> You can also test it with this VTC:<br>
> varnishtest "conditional requests"<br>
><br>
> server s1 {<br>
> rxreq<br>
> txresp -status 200 -hdr "ETag: 1234" -hdr "Last-Modified: Wed, 21 Oct 2015 07:28:00 GMT" -body "dad"<br>
><br>
> rxreq<br>
> expect req.http.if-none-match == "1234"<br>
> expect req.http.if-modified-since == "Wed, 21 Oct 2015 07:28:00 GMT"<br>
> txresp<br>
> } -start<br>
><br>
> varnish v1 -vcl+backend {<br>
> sub vcl_backend_response {<br>
> set beresp.ttl = 0.1s;<br>
> set beresp.grace = 0s;<br>
> set beresp.keep = 1y;<br>
> return (deliver);<br>
> }<br>
> } -start<br>
><br>
> client c1 {<br>
> txreq<br>
> rxresp<br>
><br>
> delay 0.2<br>
><br>
> txreq<br>
> rxresp<br>
> } -run<br>
><br>
> Change the 200 to a 404 and the test will now fail.<br>
><br>
> I quickly skimmed the HTTP spec and see no reason for us to actually check the status, but I'm sure somebody closer to the code will pop up to provide some light on the topic.<br>
<br>
I was writing a very similar test case but haven't spent time on the<br>
RFC but their is also the concern of not breaking existing setups.<br>
<br>
Similarly to how how we handle request cookies with extra caution, we<br>
could imagine something like this in the built-in VCL for<br>
vcl_backend_request:<br>
<br>
sub bereq_revalidate {<br>
if (bereq.uncacheable || obj_stale.status == 200) {<br>
return;<br>
}<br>
unset bereq.http.If-None-Match;<br>
unset bereq.http.If-Modified-Since;<br>
}<br>
<br>
Then enabling revalidation for 404 is only a matter of adding this:<br>
<br>
sub bereq_revalidate {<br>
if (obj_stale.status == 404) {<br>
return;<br>
}<br>
}<br>
<br>
We briefly discussed accessing the stale object during last VDD and<br>
extensively discussed other aspects of (re)validation.<br>
<br>
<a href="https://github.com/varnishcache/varnish-cache/wiki/VDD23Q1#compliance" rel="noreferrer" target="_blank">https://github.com/varnishcache/varnish-cache/wiki/VDD23Q1#compliance</a><br>
<br>
Right now the workaround would be to store the actual beresp.status in<br>
a header when you wish to enable revalidation, change it to 200 and<br>
restore the original in vcl_deliver.<br>
<br>
Dridi<br>
</blockquote></div>