Conditional requests for cached 404 responses

[Mark Slater]

Thanks for this.

I changed my sample VCL to be the following, and it seems to work as
intended:

vcl 4.1;

import std;

backend default {
    .host = "localhost";
    .port = "8081";
}

sub vcl_backend_response {
     set beresp.keep = 5m;
     set beresp.http.actual-status = beresp.status;
     set beresp.status = 200;
}

sub vcl_deliver {
     set resp.status = std.integer(resp.http.actual-status, 0);
     unset resp.http.actual-status;
}

However, on further investigation, I'm not sure it's a good idea.  Here's
the commit that introduced the current behaviour:
https://github.com/varnishcache/varnish-cache/commit/e99b5cfd886ec38a7f883e23ba516063cf4c16f8.
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:
https://datatracker.ietf.org/doc/html/rfc7232#section-4.1, specifically:

>  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.

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.

Regards,

Mark


On Sat, 15 Jul 2023 at 10:30, Dridi Boukelmoune <dridi at varni.sh> wrote:

> On Sat, Jul 15, 2023 at 5:09 AM Guillaume Quintard
> <guillaume.quintard at gmail.com> wrote:
> >
> > Hi Mark,
> >
> > You are correct:
> https://github.com/varnishcache/varnish-cache/blob/varnish-7.3.0/bin/varnishd/cache/cache_fetch.c#L699-L703
> >
> > We only set the OF_IMSCAND flag (that we use to say that we can
> conditional download) if:
> > - the object is not a Hit-For-Miss (HFM)
> > - if the status is 200
> > - we either have a convincing Last-modified, or an Etag header
> >
> > You can also test it with this VTC:
> > varnishtest "conditional requests"
> >
> > server s1 {
> >         rxreq
> >         txresp -status 200 -hdr "ETag: 1234" -hdr "Last-Modified: Wed,
> 21 Oct 2015 07:28:00 GMT" -body "dad"
> >
> >         rxreq
> >         expect req.http.if-none-match == "1234"
> >         expect req.http.if-modified-since == "Wed, 21 Oct 2015 07:28:00
> GMT"
> >         txresp
> > } -start
> >
> > varnish v1 -vcl+backend {
> >         sub vcl_backend_response {
> >                 set beresp.ttl = 0.1s;
> >                 set beresp.grace = 0s;
> >                 set beresp.keep = 1y;
> >                 return (deliver);
> >         }
> > } -start
> >
> > client c1 {
> >         txreq
> >         rxresp
> >
> >         delay 0.2
> >
> >         txreq
> >         rxresp
> > } -run
> >
> > Change the 200 to a 404 and the test will now fail.
> >
> > 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.
>
> I was writing a very similar test case but haven't spent time on the
> RFC but their is also the concern of not breaking existing setups.
>
> Similarly to how how we handle request cookies with extra caution, we
> could imagine something like this in the built-in VCL for
> vcl_backend_request:
>
>     sub bereq_revalidate {
>             if (bereq.uncacheable || obj_stale.status == 200) {
>                     return;
>             }
>             unset bereq.http.If-None-Match;
>             unset bereq.http.If-Modified-Since;
>     }
>
> Then enabling revalidation for 404 is only a matter of adding this:
>
>     sub bereq_revalidate {
>             if (obj_stale.status == 404) {
>                     return;
>             }
>     }
>
> We briefly discussed accessing the stale object during last VDD and
> extensively discussed other aspects of (re)validation.
>
> https://github.com/varnishcache/varnish-cache/wiki/VDD23Q1#compliance
>
> Right now the workaround would be to store the actual beresp.status in
> a header when you wish to enable revalidation, change it to 200 and
> restore the original in vcl_deliver.
>
> Dridi
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-misc/attachments/20230721/61ec711b/attachment.html>