[master] 9e399f38e Consistently abort ESI transactions also for empty includes

Nils Goroll nils.goroll at uplex.de
Fri Mar 1 18:29:05 UTC 2024


commit 9e399f38e7588c2c1593bce5747dd37b40ebc457
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Fri Mar 1 17:38:29 2024 +0100

    Consistently abort ESI transactions also for empty includes
    
    When we introduced checking ESI includes for status 200 or 204, we
    would only (potentially, depending on the onerror attribute and
    esi_onerror_continue parameter) abort ESI processing for includes
    with a non-zero length.
    
    This patch makes behavior consistent to not depend on whether or not an
    include is empty.
    
    Fixes #4070

diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index f9a97b085..2c6318039 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -879,11 +879,6 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 
 	CAST_OBJ_NOTNULL(ecx, req->transport_priv, ECX_MAGIC);
 
-	if (wantbody == 0) {
-		ved_close(req, boc, 0);
-		return;
-	}
-
 	status = req->resp->status % 1000;
 
 	if (!ecx->incl_cont && status != 200 && status != 204) {
@@ -891,6 +886,11 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 		return;
 	}
 
+	if (wantbody == 0) {
+		ved_close(req, boc, 0);
+		return;
+	}
+
 	if (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0) {
 		ved_close(req, boc, 0);
 		return;
diff --git a/bin/varnishtest/tests/r03865.vtc b/bin/varnishtest/tests/r03865.vtc
index aba495f27..b36e23f66 100644
--- a/bin/varnishtest/tests/r03865.vtc
+++ b/bin/varnishtest/tests/r03865.vtc
@@ -6,6 +6,11 @@ server s1 {
 	txresp -hdr {surrogate-control: content="ESI/1.0"} \
 	    -body {before <esi:include src="/fail" onerror="abort"/> after}
 
+	rxreq
+	expect req.url == "/abort0"
+	txresp -hdr {surrogate-control: content="ESI/1.0"} \
+	    -body {before <esi:include src="/fail0" onerror="abort"/> after}
+
 } -start
 
 varnish v1 -cliok "param.set feature +esi_disable_xml_check"
@@ -15,6 +20,9 @@ varnish v1 -vcl+backend {
 		if (bereq.url == "/fail") {
 			return (error(604));
 		}
+		if (bereq.url == "/fail0") {
+			return (error(605));
+		}
 	}
 	sub vcl_backend_response {
 		set beresp.do_esi = beresp.http.surrogate-control ~ "ESI/1.0";
@@ -25,6 +33,10 @@ varnish v1 -vcl+backend {
 			set beresp.body = "FOOBAR";
 			return(deliver);
 		}
+		if (beresp.status == 605) {
+			set beresp.body = "";
+			return(deliver);
+		}
 	}
 } -start
 
@@ -39,6 +51,18 @@ client c1 {
 	expect resp.body == "before "
 } -run
 
+client c1 {
+	# #4070
+	txreq -url "/abort0"
+	non_fatal
+	rxresphdrs
+	expect resp.status == 200
+	rxchunk
+	rxchunk
+	expect_close
+	expect resp.body == "before "
+} -run
+
 varnish v1 -cliok "param.set max_esi_depth 0"
 
 client c1 {


More information about the varnish-commit mailing list