Fix object access in ESI VFP when there is no object

We set up fetch filters (VFPs) before initializing a storage object to
fetch into. If that fails, we close the filters again.

The esi_gzip VFP interacts with the storage object and was missing
error handling for the case of a teardown with no storage object.

Implementation:

We make sure that for the VFP_ERROR case, we do not interact with the
storage object (which makes no sense if that is going to be C-4'ed any
moment).

vfp_vep_callback() keeps its hands off if (struct vef_priv).error is
set, so we use that to avoid complications in the code.

bin/varnishd/cache/cache_esi_fetch.c

@@ -120,6 +120,11 @@ vfp_esi_end(struct vfp_ctx *vc, struct vef_priv *vef,
 	CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);
 
+	if (retval == VFP_ERROR)
+		vef->error = errno ? errno : EINVAL;
+	else
+		assert(retval == VFP_END);
+
 	vsb = VEP_Finish(vef->vep);
 
 	if (vsb != NULL) {
@@ -137,7 +142,8 @@ vfp_esi_end(struct vfp_ctx *vc, struct vef_priv *vef,
 	}
 
 	if (vef->vgz != NULL) {
-		VGZ_UpdateObj(vc, vef->vgz, VUA_END_GZIP);
+		if (retval == VFP_END)
+			VGZ_UpdateObj(vc, vef->vgz, VUA_END_GZIP);
 		if (VGZ_Destroy(&vef->vgz) != VGZ_END)
 			retval = VFP_Error(vc,
 			    "ESI+Gzip Failed at the very end");
@@ -292,8 +298,13 @@ vfp_esi_fini(struct vfp_ctx *vc, struct vfp_entry *vfe)
 	CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);
 	CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);
 
-	if (vfe->priv1 != NULL)
-		(void)vfp_esi_end(vc, vfe->priv1, VFP_ERROR);
+	if (vfe->priv1 == NULL)
+		return;
+
+	if (vc->oc->stobj->stevedore == NULL)
+		errno = ENOMEM;
+
+	(void)vfp_esi_end(vc, vfe->priv1, VFP_ERROR);
 	vfe->priv1 = NULL;
 }
 

bin/varnishtest/tests/r03502.vtc

+varnishtest "#3502 Panic in VEP_Finish() for out-of-storage in vbf_beresp2obj()"
+
+server s1 {
+	# First consume (almost) all of the storage - the value
+	# is brittle, see l1 fail
+	rxreq
+	expect req.url == /url1
+	txresp -bodylen 1048336
+
+	rxreq
+	expect req.http.accept-encoding == gzip
+	txresp -bodylen 1
+} -start
+
+varnish v1 -arg "-sTransient=default,1M -p debug=+syncvsl -p nuke_limit=0" -vcl+backend {
+	sub vcl_recv {
+		if (req.url == "/") {
+			return (pass);
+		}
+	}
+	sub vcl_backend_response {
+		set beresp.http.free = storage.Transient.free_space;
+		if (bereq.url == "/") {
+			set beresp.do_gzip = true;
+			set beresp.do_esi = true;
+		}
+	}
+} -start
+
+logexpect l1 -v v1 -g vxid -q "vxid == 1004" {
+	expect 25 1004	VCL_call        {^BACKEND_RESPONSE}
+	expect 0  =	BerespHeader    {^free:}
+	expect 0  =	VCL_return      {^deliver}
+	expect 0  =	Timestamp       {^Process}
+	expect 0  =	Filters         {^ esi_gzip}
+	expect 0  =	BerespUnset     {^Content-Length: 1}
+	expect 0  =	BerespHeader    {^Content-Encoding: gzip}
+	expect 0  =	BerespHeader    {^Vary: Accept-Encoding}
+	# Ensure the FetchError is in vbf_beresp2obj()
+	# not later in the VFP. Otherwise we have too much free_space
+	fail add  =	Storage
+	expect 0  =	FetchError      {^Could not get storage}
+	fail clear
+} -start
+
+client c1 {
+	txreq -url /url1
+	rxresp
+	expect resp.status == 200
+
+	txreq -hdr "Accept-Encoding: gzip"
+	# no storage for synth either
+	expect_close
+} -run
+
+logexpect l1 -wait