esi: Implement <esi:include onerror="continue"/>

This changes the default behavior of includes, matching the ESI language
specification. The onerror attribute seems to only accept one value, so
in its absence a failed ESI fragment delivery aborts the top request
delivery.

This new behavior requires the esi_include_onerror feature flag to be
raised to take effect.

In addition, it breaks the ESI object attribute format for persisted
caches.

bin/varnishd/cache/cache_esi.h

@@ -39,7 +39,8 @@
 #define	VEC_S1	(0x60 + 1)
 #define	VEC_S2	(0x60 + 2)
 #define	VEC_S8	(0x60 + 8)
-#define	VEC_INCL	'I'
+#define	VEC_INCL_ABRT	'I'
+#define	VEC_INCL_CONT	'i'
 
 typedef ssize_t vep_callback_t(struct vfp_ctx *, void *priv, ssize_t l,
     enum vgz_flag flg);

bin/varnishd/cache/cache_esi_deliver.c

@@ -65,6 +65,7 @@ struct ecx {
 	ssize_t		l;
 	int		isgzip;
 	int		woken;
+	int		abrt;
 
 	struct req	*preq;
 	struct ecx	*pecx;
@@ -378,7 +379,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_INCL:
+			case VEC_INCL_ABRT:
+				ecx->abrt =
+				    FEATURE(FEATURE_ESI_INCLUDE_ONERROR);
+				/* FALLTHROUGH */
+			case VEC_INCL_CONT:
 				ecx->p++;
 				q = (void*)strchr((const char*)ecx->p, '\0');
 				AN(q);
@@ -905,4 +910,9 @@ ved_deliver(struct req *req, struct boc *boc, int wantbody)
 		req->doclose = SC_REM_CLOSE;
 
 	req->acct.resp_bodybytes += VDP_Close(req->vdc);
+
+	if (i && ecx->abrt) {
+		req->top->topreq->vdc->retval = -1;
+		req->top->topreq->doclose = req->doclose;
+	}
 }

bin/varnishd/cache/cache_esi_parse.c

@@ -114,6 +114,7 @@ struct vep_state {
 	dostuff_f		*dostuff;
 
 	struct vsb		*include_src;
+	unsigned		include_continue;
 
 	unsigned		nm_skip;
 	unsigned		nm_verbatim;
@@ -181,6 +182,7 @@ static struct vep_match vep_match_esi[] = {
 
 static struct vep_match vep_match_attr_include[] = {
 	{ "src=",	&VEP_ATTRGETVAL },
+	{ "onerror=",	&VEP_ATTRGETVAL },
 	{ NULL,		&VEP_SKIPATTR }
 };
 
@@ -460,11 +462,20 @@ include_attr_src(struct vep_state *vep)
 	vep->attr_vsb = NULL;
 }
 
+static void
+include_attr_onerror(struct vep_state *vep)
+{
+
+	vep->include_continue = !strcmp("continue", VSB_data(vep->attr_vsb));
+	VSB_destroy(&vep->attr_vsb);
+}
+
 static void v_matchproto_()
 vep_do_include(struct vep_state *vep, enum dowhat what)
 {
 	const char *p, *q, *h;
 	ssize_t l;
+	char incl;
 
 	Debug("DO_INCLUDE(%d)\n", what);
 	if (what == DO_ATTR) {
@@ -474,6 +485,10 @@ vep_do_include(struct vep_state *vep, enum dowhat what)
 			include_attr_src(vep);
 			return;
 		}
+		if (!strcmp("onerror=", vep->match_hit->match)) {
+			include_attr_onerror(vep);
+			return;
+		}
 		WRONG("Unhandled <esi:include> attribute");
 	}
 	assert(what == DO_TAG);
@@ -499,6 +514,8 @@ vep_do_include(struct vep_state *vep, enum dowhat what)
 	l = VSB_len(vep->include_src);
 	h = 0;
 
+	incl = vep->include_continue ? VEC_INCL_CONT : VEC_INCL_ABRT;
+
 	if (l > 7 && !memcmp(p, "http://", 7)) {
 		h = p + 7;
 		p = strchr(h, '/');
@@ -511,7 +528,7 @@ vep_do_include(struct vep_state *vep, enum dowhat what)
 			return;
 		}
 		Debug("HOST <%.*s> PATH <%s>\n", (int)(p-h),h, p);
-		VSB_printf(vep->vsb, "%c", VEC_INCL);
+		VSB_printf(vep->vsb, "%c", incl);
 		VSB_printf(vep->vsb, "Host: %.*s%c", (int)(p-h), h, 0);
 	} else if (l > 8 && !memcmp(p, "https://", 8)) {
 		if (!FEATURE(FEATURE_ESI_IGNORE_HTTPS)) {
@@ -534,13 +551,13 @@ vep_do_include(struct vep_state *vep, enum dowhat what)
 			VSB_destroy(&vep->include_src);
 			return;
 		}
-		VSB_printf(vep->vsb, "%c", VEC_INCL);
+		VSB_printf(vep->vsb, "%c", incl);
 		VSB_printf(vep->vsb, "Host: %.*s%c", (int)(p-h), h, 0);
 	} else if (*p == '/') {
-		VSB_printf(vep->vsb, "%c", VEC_INCL);
+		VSB_printf(vep->vsb, "%c", incl);
 		VSB_printf(vep->vsb, "%c", 0);
 	} else {
-		VSB_printf(vep->vsb, "%c", VEC_INCL);
+		VSB_printf(vep->vsb, "%c", incl);
 		VSB_printf(vep->vsb, "%c", 0);
 		/* Look for the last / before a '?' */
 		h = NULL;
@@ -574,6 +591,7 @@ vep_do_include(struct vep_state *vep, enum dowhat what)
 #undef R
 	VSB_printf(vep->vsb, "%c", 0);
 	VSB_destroy(&vep->include_src);
+	vep->include_continue = 0;
 }
 
 /*---------------------------------------------------------------------

bin/varnishtest/tests/e00035.vtc

+varnishtest "ESI fragment fetch fail"
+
+server s1 {
+	rxreq
+	expect req.url == "/abort"
+	txresp -hdr {surrogate-control: content="ESI/1.0"} \
+	    -body {before <esi:include src="/fail"/> after}
+
+	rxreq
+	expect req.url == "/fail"
+	txresp -hdr "content-length: 100" -nolen
+	delay 0.1
+} -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_response {
+		set beresp.do_esi = beresp.http.surrogate-control ~ "ESI/1.0";
+		unset beresp.http.surrogate-control;
+	}
+} -start
+
+client c1 {
+	txreq -url "/abort"
+	non_fatal
+	rxresp
+	expect resp.body == "before "
+} -run
+
+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}
+
+	rxreq
+	expect req.url == "/fail"
+	txresp -hdr "content-length: 100" -nolen
+	delay 0.1
+} -start
+
+client c1 {
+	fatal
+	txreq -url "/continue"
+	rxresp
+	expect resp.body == "before  after"
+} -run

include/tbl/feature_bits.h

@@ -70,6 +70,10 @@ FEATURE_BIT(ESI_REMOVE_BOM,		esi_remove_bom,
     "Ignore UTF-8 BOM in ESI bodies."
 )
 
+FEATURE_BIT(ESI_INCLUDE_ONERROR,	esi_include_onerror,
+    "Parse the onerror attribute of <esi:include> tags."
+)
+
 FEATURE_BIT(WAIT_SILO,			wait_silo,
     "Wait for persistent silos to completely load before serving requests."
 )