[master] cffda50c8 esi: Restore the original onerror behavior

Tue Mar 5 06:28:07 UTC 2024

commit cffda50c8bb65206b1709a16ae6138264907e0a8
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Fri Mar 1 18:06:49 2024 +0100

    esi: Restore the original onerror behavior
    
    This is a partial revert of 582ded6a2d6ae1a4467b1eb500f2725b42888016 to
    restore the assumed onerror=continue behavior for ESI includes, unless
    the feature flag esi_include_onerror is raised.
    
    The part of the change that considers all status codes besides 200 and
    204 to be errors for ESI includes remains. A test case covers VCL's
    ability to "bless" error responses by overriding resp.status, allowing
    ESI delivery to continue on this criterion.
    
    Fixes #4053

diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 2c6318039..24180e6d3 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -65,7 +65,7 @@ struct ecx {
 	ssize_t		l;
 	int		isgzip;
 	int		woken;
-	int		incl_cont;
+	int		abrt;
 
 	struct req	*preq;
 	struct ecx	*pecx;
@@ -127,7 +127,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 		VSLb(preq->vsl, SLT_VCL_Error,
 		    "ESI depth limit reached (param max_esi_depth = %u)",
 		    cache_param->max_esi_depth);
-		if (!ecx->incl_cont)
+		if (ecx->abrt)
 			preq->top->topreq->vdc->retval = -1;
 		return;
 	}
@@ -384,11 +384,11 @@ ved_vdp_esi_bytes(struct vdp_ctx *vdc, enum vdp_action act, void **priv,
 				Debug("SKIP1(%d)\n", (int)ecx->l);
 				ecx->state = 4;
 				break;
-			case VEC_IC:
-				ecx->incl_cont =
+			case VEC_IA:
+				ecx->abrt =
 				    FEATURE(FEATURE_ESI_INCLUDE_ONERROR);
 				/* FALLTHROUGH */
-			case VEC_IA:
+			case VEC_IC:
 				ecx->p++;
 				q = (void*)strchr((const char*)ecx->p, '\0');
 				AN(q);
@@ -881,7 +881,7 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 
 	status = req->resp->status % 1000;
 
-	if (!ecx->incl_cont && status != 200 && status != 204) {
+	if (ecx->abrt && status != 200 && status != 204) {
 		ved_close(req, boc, 1);
 		return;
 	}
@@ -943,5 +943,5 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 	if (i && req->doclose == SC_NULL)
 		req->doclose = SC_REM_CLOSE;
 
-	ved_close(req, boc, i && !ecx->incl_cont ? 1 : 0);
+	ved_close(req, boc, i && ecx->abrt);
 }
diff --git a/bin/varnishtest/tests/r03241.vtc b/bin/varnishtest/tests/r03241.vtc
index 54a485743..ac7fab446 100644
--- a/bin/varnishtest/tests/r03241.vtc
+++ b/bin/varnishtest/tests/r03241.vtc
@@ -4,8 +4,12 @@ varnishtest "ESI include out of workspace"
 server s1 {
 	rxreq
 	expect req.http.esi0 == "foo"
-	txresp -body {<html>Before include<esi:include
-		src="/body" sr="foo"/>After include</html>
+	txresp -body {
+		<html>
+		Before include
+		<esi:include src="/body" sr="foo"/>
+		After include
+		</html>
 	}
 	rxreq
 	expect req.url == "/body1"
@@ -43,11 +47,10 @@ logexpect l1 -v v1 -g raw {
 
 client c1 {
 	txreq -hdr "Host: foo"
-	rxresphdrs
+	rxresp
+	# XXX this is actually wrong (missed include)
+	expect resp.bodylen == 57
 	expect resp.status == 200
-	rxchunk
-	expect_close
-	expect resp.body == {<html>Before include}
 }  -run
 
 logexpect l1 -wait
diff --git a/bin/varnishtest/tests/r04053.vtc b/bin/varnishtest/tests/r04053.vtc
new file mode 100644
index 000000000..443d624a6
--- /dev/null
+++ b/bin/varnishtest/tests/r04053.vtc
@@ -0,0 +1,37 @@
+varnishtest "Override ESI status check for onerror=abort"
+
+server s1 {
+        rxreq
+        expect req.http.esi-level == 0
+        txresp -body {before <esi:include src="/fail"/> after}
+
+        rxreq
+        expect req.http.esi-level == 1
+        txresp -status 500 -hdr "transfer-encoding: chunked"
+        delay 0.1
+        chunked 500
+	chunkedlen 0
+} -start
+
+varnish v1 -cliok "param.set feature +esi_disable_xml_check"
+varnish v1 -cliok "param.set feature +esi_include_onerror"
+
+varnish v1 -vcl+backend {
+        sub vcl_recv {
+                set req.http.esi-level = req.esi_level;
+        }
+        sub vcl_backend_response {
+                set beresp.do_esi = bereq.http.esi-level == "0";
+        }
+        sub vcl_deliver {
+                if (req.esi_level > 0 && resp.status != 200) {
+			set resp.status = 200;
+                }
+        }
+} -start
+
+client c1 {
+        txreq
+        rxresp
+        expect resp.body == "before 500 after"
+} -run
