Handle non-200 status

src/vmod_zipflow.c

@@ -546,6 +546,21 @@ vdp_zipflow_log(void *vsl, char *msg)
 	free(msg);
 }
 
+static struct zipflow_top *
+zipsub_take(const struct zipflow_request *zfr)
+{
+	struct zipflow_top *zft;
+
+	zft = zfr->top;
+	CHECK_OBJ_NOTNULL(zft, ZIPFLOW_TOP_MAGIC);
+
+	assert(zfr == VSTAILQ_FIRST(&zft->head));
+	VSTAILQ_REMOVE_HEAD(&zft->head, list);
+
+	AN(zft->zip);
+	return (zft);
+}
+
 static int
 vdp_zipsub_init(VRT_CTX, struct vdp_ctx *vdc, void **priv, struct objcore *oc)
 {
@@ -561,13 +576,7 @@ vdp_zipsub_init(VRT_CTX, struct vdp_ctx *vdc, void **priv, struct objcore *oc)
 	CAST_OBJ_NOTNULL(zfr, *priv, ZIPFLOW_REQUEST_MAGIC);
 	(void) oc;
 
-	zft = zfr->top;
-	CHECK_OBJ_NOTNULL(zft, ZIPFLOW_TOP_MAGIC);
-
-	assert(zfr == VSTAILQ_FIRST(&zft->head));
-	VSTAILQ_REMOVE_HEAD(&zft->head, list);
-
-	AN(zft->zip);
+	zft = zipsub_take(zfr);
 
 	if (zfr->bundle) {
 		fill_meta(ctx, zfr);
@@ -633,6 +642,13 @@ vdp_zipflow_init(VRT_CTX, struct vdp_ctx *vdc, void **priv, struct objcore *oc)
 	return (vdp_zipsub_init(ctx, vdc, priv, oc));
 }
 
+static void
+zipsub_fini(struct zipflow_request *zfr)
+{
+	CHECK_OBJ_NOTNULL(zfr, ZIPFLOW_REQUEST_MAGIC);
+	memset(zfr, 0, sizeof *zfr);
+}
+
 static int
 vdp_zipsub_fini(struct vdp_ctx *vdc, void **priv)
 {
@@ -643,10 +659,8 @@ vdp_zipsub_fini(struct vdp_ctx *vdc, void **priv)
 	AN(priv);
 	zfr = *priv;
 	*priv = NULL;
-	if (zfr != NULL) {
-		CHECK_OBJ(zfr, ZIPFLOW_REQUEST_MAGIC);
-		memset(zfr, 0, sizeof *zfr);
-	}
+	if (zfr != NULL)
+		zipsub_fini(zfr);
 	return (0);
 }
 
@@ -926,16 +940,15 @@ zfr_deliver(struct req *req, struct boc *boc, int wantbody)
 	CHECK_OBJ_ORNULL(boc, BOC_MAGIC);
 	CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 
-	if (wantbody == 0)
-		return;
-
 	status = req->resp->status % 1000;
-	// XXX should we trigger an error like ESI does?
-	if (status != 200)
-		return;
 
-	if (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0)
+	if (wantbody == 0 ||
+	    status != 200 ||
+	    (boc == NULL && ObjGetLen(req->wrk, req->objcore) == 0)) {
+		(void) zipsub_take(req->transport_priv);
+		zipsub_fini(req->transport_priv);
 		return;
+	}
 
 	AZ(http_GetHdr(req->resp, H_Content_Encoding, &p));
 

src/vmod_zipflow.vcc

@@ -107,6 +107,9 @@ The order of subrequests is:
 
   * each subreq followed by any `zipflow.subreq()`_ initiated from it
 
+Only sub requests with reponse status 200 will be included in the
+resulting zip file.
+
 $Function BOOL is_subreq()
 
 $Restrict client

src/vtc/sub-body.vtc

@@ -15,6 +15,7 @@ varnish v1 -vcl {
 
 	sub synth_top {
 		zipflow.subreq("/fromvcl");
+		zipflow.subreq("/404");
 		synthetic(" /FIRST/FROM/RESP/file /file1");
 		zipflow.subreqs_from_body(resp_body);
 		zipflow.subreqs_from_body(req_body);
@@ -26,7 +27,11 @@ varnish v1 -vcl {
 	}
 
 	sub vcl_synth {
-		if (zipflow.is_subreq()) {
+		if (req.url == "/404") {
+			set resp.status = 404;
+			set resp.body = "404";
+		}
+		else if (zipflow.is_subreq()) {
 			call synth_sub;
 			zipflow.meta(name=req.url);
 			if (req.url ~ "/file1") {
@@ -36,7 +41,8 @@ varnish v1 -vcl {
 			if (req.url ~ "/file3") {
 				zipflow.subreq("/file4");
 			}
-		} else {
+		}
+		else {
 			call synth_top;
 		}
 		return (deliver);
@@ -46,6 +52,7 @@ varnish v1 -vcl {
 logexpect l1 -v v1 -g request -q "ReqURL ~ \"/REQ/first/file\"" {
 	fail add *	ReqURL	"valid"
 	expect	 * *	ReqURL	"/fromvcl"
+	expect	 * *	ReqURL	"/404"
 	expect	 * *	ReqURL	"/FIRST/FROM/RESP/file"
 	expect	 * *	ReqURL	"/file1"
 	expect	 * *	ReqURL	"/file3"