Mailing List Archive

commit d2f603c5bc944fde221c28393e37606cf0136301
Author: Dridi Boukelmoune <dridi.boukelmoune@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 ?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit