[master] 582ded6a2 By default do not deliver failed 200 ESI:includes.
Poul-Henning Kamp
phk at FreeBSD.org
Tue Jan 31 14:45:08 UTC 2023
commit 582ded6a2d6ae1a4467b1eb500f2725b42888016
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Tue Jan 31 14:43:48 2023 +0000
By default do not deliver failed 200 ESI:includes.
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 117e710c6..56f4b82d2 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 abrt;
+ int incl_cont;
struct req *preq;
struct ecx *pecx;
@@ -382,11 +382,11 @@ ved_vdp_esi_bytes(struct vdp_ctx *vdx, enum vdp_action act, void **priv,
Debug("SKIP1(%d)\n", (int)ecx->l);
ecx->state = 4;
break;
- case VEC_IA:
- ecx->abrt =
+ case VEC_IC:
+ ecx->incl_cont =
FEATURE(FEATURE_ESI_INCLUDE_ONERROR);
/* FALLTHROUGH */
- case VEC_IC:
+ case VEC_IA:
ecx->p++;
q = (void*)strchr((const char*)ecx->p, '\0');
AN(q);
@@ -869,6 +869,14 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
if (wantbody == 0)
return;
+ if (!ecx->incl_cont &&
+ req->resp->status != 200 &&
+ req->resp->status != 204) {
+ req->top->topreq->vdc->retval = -1;
+ req->top->topreq->doclose = req->doclose;
+ return;
+ }
+
if (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0)
return;
@@ -918,7 +926,7 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
req->acct.resp_bodybytes += VDP_Close(req->vdc);
- if (i && ecx->abrt) {
+ if (i && !ecx->incl_cont) {
req->top->topreq->vdc->retval = -1;
req->top->topreq->doclose = req->doclose;
}
diff --git a/bin/varnishtest/tests/r03241.vtc b/bin/varnishtest/tests/r03241.vtc
index ac7fab446..54a485743 100644
--- a/bin/varnishtest/tests/r03241.vtc
+++ b/bin/varnishtest/tests/r03241.vtc
@@ -4,12 +4,8 @@ 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"
@@ -47,10 +43,11 @@ logexpect l1 -v v1 -g raw {
client c1 {
txreq -hdr "Host: foo"
- rxresp
- # XXX this is actually wrong (missed include)
- expect resp.bodylen == 57
+ rxresphdrs
expect resp.status == 200
+ rxchunk
+ expect_close
+ expect resp.body == {<html>Before include}
} -run
logexpect l1 -wait
diff --git a/bin/varnishtest/tests/r03865.vtc b/bin/varnishtest/tests/r03865.vtc
new file mode 100644
index 000000000..98f706b5b
--- /dev/null
+++ b/bin/varnishtest/tests/r03865.vtc
@@ -0,0 +1,57 @@
+varnishtest "ESI onerror"
+
+server s1 {
+ rxreq
+ expect req.url == "/abort"
+ txresp -hdr {surrogate-control: content="ESI/1.0"} \
+ -body {before <esi:include src="/fail" onerror="abort"/> after}
+
+} -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_backend_fetch {
+ if (bereq.url == "/fail") {
+ return (error(604));
+ }
+ }
+ sub vcl_backend_response {
+ set beresp.do_esi = beresp.http.surrogate-control ~ "ESI/1.0";
+ unset beresp.http.surrogate-control;
+ }
+ sub vcl_backend_error {
+ if (beresp.status == 604) {
+ set beresp.body = "FOOBAR";
+ return(deliver);
+ }
+ }
+} -start
+
+client c1 {
+ txreq -url "/abort"
+ non_fatal
+ rxresphdrs
+ expect resp.status == 200
+ rxchunk
+ expect_close
+ expect resp.body == "before "
+} -run
+
+varnish v1 -vsl_catchup
+
+server s1 -wait
+
+server s1 {
+ rxreq
+ expect req.url == "/continue"
+ txresp -hdr {surrogate-control: content="ESI/1.0"} \
+ -body {before <esi:include src="/fail" onerror="continue"/> after}
+} -start
+
+client c1 {
+ fatal
+ txreq -url "/continue"
+ rxresp
+ expect resp.body == "before FOOBAR after"
+} -run
diff --git a/doc/changes.rst b/doc/changes.rst
index 8d16a3988..979857f08 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -35,6 +35,25 @@ release process.
Varnish Cache NEXT (2023-03-15)
===============================
+* Do not ESI:include failed objects unless instructed to.
+
+ Previously, any ESI:include object would be included, no matter
+ what the status of it were, 200, 503, didn't matter.
+
+ From now on, by default, only objects with 200 and 204 status
+ will be included and any other status code will fail the parent
+ ESI request.
+
+ If objects with other status should be delivered, they should
+ have their status changed to 200 in VCL, for instance in
+ ``sub vcl_backend_error{}``, ``vcl_synth{}`` or ``vcl_deliver{}``.
+
+ If ``param.set feature +esi_include_onerror`` is used, and the
+ ``<esi:include …>`` tag has a ``onerror="continue"`` attribute,
+ any and all ESI:include objects will be delivered, no matter what
+ their status might be, and not even a partial delivery of them
+ will fail the parent ESI request. To be used with great caution.
+
* VXIDs are 64 bit now and the binary format of SHM and raw saved
VSL files has changed as a consequence.
More information about the varnish-commit
mailing list