[master] d2f603c5b esi: Skip the response body for onerror="continue"

[Dridi Boukelmoune]

commit d2f603c5bc944fde221c28393e37606cf0136301
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Fri Mar 1 19:11:21 2024 +0100

    esi: Skip the response body for onerror="continue"
    
    That is to say, when we can actually do that.

diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 24180e6d3..43c9c660f 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -881,8 +881,9 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 
 	status = req->resp->status % 1000;
 
-	if (ecx->abrt && status != 200 && status != 204) {
-		ved_close(req, boc, 1);
+	if (FEATURE(FEATURE_ESI_INCLUDE_ONERROR) &&
+	    status != 200 && status != 204) {
+		ved_close(req, boc, ecx->abrt);
 		return;
 	}
 
diff --git a/bin/varnishtest/tests/e00035.vtc b/bin/varnishtest/tests/e00035.vtc
index 0fc59d749..164c5044f 100644
--- a/bin/varnishtest/tests/e00035.vtc
+++ b/bin/varnishtest/tests/e00035.vtc
@@ -8,7 +8,7 @@ server s1 {
 
 	rxreq
 	expect req.url == "/fail"
-	txresp -hdr "content-length: 100" -nolen
+	txresp -hdr "content-length: 100"
 	delay 0.1
 } -start
 
@@ -17,6 +17,7 @@ varnish v1 -cliok "param.set feature +esi_include_onerror"
 varnish v1 -vcl+backend {
 	sub vcl_backend_response {
 		set beresp.do_esi = beresp.http.surrogate-control ~ "ESI/1.0";
+		set beresp.do_stream = beresp.http.stream != "false";
 		unset beresp.http.surrogate-control;
 	}
 } -start
@@ -38,7 +39,7 @@ server s1 {
 
 	rxreq
 	expect req.url == "/fail"
-	txresp -hdr "content-length: 100" -nolen
+	txresp -hdr "content-length: 100"
 	delay 0.1
 } -start
 
@@ -48,3 +49,24 @@ client c1 {
 	rxresp
 	expect resp.body == "before  after"
 } -run
+
+server s1 -wait
+
+server s1 {
+	rxreq
+	expect req.url == "/continue-no-stream"
+	txresp -hdr {surrogate-control: content="ESI/1.0"} \
+	    -body {before <esi:include src="/err" onerror="continue"/> after}
+
+	rxreq
+	expect req.url == "/err"
+	txresp -hdr "content-encoding chunked" -hdr "stream: false"
+	chunked "incomplete"
+} -start
+
+client c1 {
+	fatal
+	txreq -url "/continue-no-stream"
+	rxresp
+	expect resp.body == "before  after"
+} -run
diff --git a/bin/varnishtest/tests/r03865.vtc b/bin/varnishtest/tests/r03865.vtc
index b36e23f66..bd3736de2 100644
--- a/bin/varnishtest/tests/r03865.vtc
+++ b/bin/varnishtest/tests/r03865.vtc
@@ -93,7 +93,7 @@ client c1 {
 	fatal
 	txreq -url "/continue"
 	rxresp
-	expect resp.body == "before FOOBAR after"
+	expect resp.body == "before  after"
 } -run
 
 varnish v1 -cliok "param.set max_esi_depth 0"
diff --git a/doc/sphinx/users-guide/esi.rst b/doc/sphinx/users-guide/esi.rst
index 0913f3eef..209ade289 100644
--- a/doc/sphinx/users-guide/esi.rst
+++ b/doc/sphinx/users-guide/esi.rst
@@ -128,7 +128,10 @@ continue in case of failures, by setting::
     param.set feature +esi_include_onerror
 
 Once this feature flag is enabled, a delivery failure can only continue
-if an ``onerror`` attribute said so.
+if an ``onerror`` attribute said so. The ESI specification states that
+in that case the failing fragment is not delivered, which is honored based
+on the status code, or based on the response body only when streaming is
+disabled (see ``beresp.do_stream``).
 
 Can an ESI fragment also use ESI-includes ?
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~