[master] 671b93b74 condfetch: Test retrying a bereq on a dying stale object

[Nils Goroll]

commit 671b93b74b2bbf1af4e668b8a6215b40fa1292af
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Mar 12 17:41:54 2024 +0100

    condfetch: Test retrying a bereq on a dying stale object
    
    The built-in vcl_refresh_valid returns a 503 error if the stale object used for
    revalidation gets invalidated while the backend fetch is in progress.
    
    One alternative is to not dismiss the stale object, but that can lead to
    invalidated objects being re-instantiated through concurrent refreshes (#4082).
    
    This test case covers the other alternative, which is to retry the fetch. We
    could use
    
           sub vcl_refresh_valid {
                   if (!obj_stale.is_valid) {
                           # core code removes conditional headers INM/IMS
                           return (retry);
                   }
           }
    
    but more realisticly, real world VCL will retry in v_b_e, so we take that route
    and keep the built-in vcl_refresh_valid {}
    
    The plain retry works because core code detects the invalidated stale object and
    removes the conditional headers.
    
    Committer edit: Removed code change from original commit, edited test case and,
    rewrote commit message. See
    https://github.com/varnishcache/varnish-cache/pull/4400#discussion_r2379454734

diff --git a/bin/varnishtest/tests/c00129.vtc b/bin/varnishtest/tests/c00129.vtc
new file mode 100644
index 000000000..073e8a469
--- /dev/null
+++ b/bin/varnishtest/tests/c00129.vtc
@@ -0,0 +1,68 @@
+varnishtest "304 on invalidated stale object"
+
+barrier b1 cond 2
+barrier b2 cond 2
+
+server s1 {
+	rxreq
+	txresp -hdr {ETag: "foo"} -body corrupted
+
+	rxreq
+	expect req.http.If-None-Match == {"foo"}
+	barrier b1 sync
+	barrier b2 sync
+	txresp -status 304 -hdr {ETag: "foo"}
+
+	close
+	accept
+
+	rxreq
+	expect req.http.If-None-Match == <undef>
+	txresp -hdr {ETag: "foo"} -body valid
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		if (req.method == "PURGE") {
+			return (purge);
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.ttl = 1ms;
+		set beresp.grace = 0s;
+		set beresp.keep = 10s;
+	}
+	# rather than modifying vcl_refresh_valid, we retry in v_b_e, which is
+	# more generic and likely to be found in real-world configs.
+	sub vcl_backend_error {
+		if (bereq.retries == 0) {
+			return (retry);
+		}
+	}
+	sub vcl_deliver {
+		set resp.http.obj-hits = obj.hits;
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.body == corrupted
+	expect resp.http.obj-hits == 0
+
+	delay 0.1
+
+	txreq
+	rxresp
+	expect resp.body == valid
+	expect resp.http.obj-hits == 0
+} -start
+
+barrier b1 sync
+client c2 {
+	txreq -req PURGE
+	rxresp
+} -run
+barrier b2 sync
+
+client c1 -wait