[master] 6e4e013 Overhaul the detection and reporting of fetch errors, to properly catch trouble that materializes only when we destroy the VGZ instance.

Poul-Henning Kamp phk at varnish-cache.org
Mon Oct 31 15:22:23 CET 2011


commit 6e4e013f407c6685241c7a4c88a72dbd671102ba
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Oct 31 14:20:34 2011 +0000

    Overhaul the detection and reporting of fetch errors, to properly
    catch trouble that materializes only when we destroy the VGZ instance.
    
    Fixes	#1037
    Fixes	#1043
    Fixes	#1044

diff --git a/bin/varnishd/cache.h b/bin/varnishd/cache.h
index bce7377..53e8ae8 100644
--- a/bin/varnishd/cache.h
+++ b/bin/varnishd/cache.h
@@ -345,6 +345,7 @@ struct worker {
 	struct vfp		*vfp;
 	struct vgz		*vgz_rx;
 	struct vef_priv		*vef_priv;
+	unsigned		fetch_failed;
 	unsigned		do_stream;
 	unsigned		do_esi;
 	unsigned		do_gzip;
@@ -703,6 +704,8 @@ int EXP_NukeOne(struct worker *w, struct lru *lru);
 
 /* cache_fetch.c */
 struct storage *FetchStorage(struct worker *w, ssize_t sz);
+int FetchError(struct worker *w, const char *error);
+int FetchError2(struct worker *w, const char *error, const char *more);
 int FetchHdr(struct sess *sp);
 int FetchBody(struct worker *w, struct object *obj);
 int FetchReqBody(struct sess *sp);
@@ -721,7 +724,7 @@ int VGZ_ObufFull(const struct vgz *vg);
 int VGZ_ObufStorage(struct worker *w, struct vgz *vg);
 int VGZ_Gzip(struct vgz *, const void **, size_t *len, enum vgz_flag);
 int VGZ_Gunzip(struct vgz *, const void **, size_t *len);
-void VGZ_Destroy(struct vgz **, int vsl_id);
+int VGZ_Destroy(struct vgz **, int vsl_id);
 void VGZ_UpdateObj(const struct vgz*, struct object *);
 int VGZ_WrwGunzip(struct worker *w, struct vgz *, const void *ibuf,
     ssize_t ibufl, char *obuf, ssize_t obufl, ssize_t *obufp);
diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
index fe7dae6..37c4b7b 100644
--- a/bin/varnishd/cache_center.c
+++ b/bin/varnishd/cache_center.c
@@ -934,7 +934,7 @@ cnt_streambody(struct sess *sp)
 
 	RES_StreamEnd(sp);
 	if (sp->wrk->res_mode & RES_GUNZIP)
-		VGZ_Destroy(&sctx.vgz, sp->vsl_id);
+		(void)VGZ_Destroy(&sctx.vgz, sp->vsl_id);
 
 	sp->wrk->sctx = NULL;
 	assert(WRW_IsReleased(sp->wrk));
diff --git a/bin/varnishd/cache_esi_deliver.c b/bin/varnishd/cache_esi_deliver.c
index a06bbb8..2c62bb7 100644
--- a/bin/varnishd/cache_esi_deliver.c
+++ b/bin/varnishd/cache_esi_deliver.c
@@ -406,7 +406,7 @@ ESI_Deliver(struct sess *sp)
 	if (vgz != NULL) {
 		if (obufl > 0)
 			(void)WRW_Write(sp->wrk, obuf, obufl);
-		VGZ_Destroy(&vgz, sp->vsl_id);
+		(void)VGZ_Destroy(&vgz, sp->vsl_id);
 	}
 	if (sp->wrk->gzip_resp && sp->esi_level == 0) {
 		/* Emit a gzip literal block with finish bit set */
diff --git a/bin/varnishd/cache_esi_fetch.c b/bin/varnishd/cache_esi_fetch.c
index 69896aa..1c75054 100644
--- a/bin/varnishd/cache_esi_fetch.c
+++ b/bin/varnishd/cache_esi_fetch.c
@@ -112,7 +112,7 @@ vfp_esi_bytes_gu(struct worker *w, struct http_conn *htc, ssize_t bytes)
 			bytes -= wl;
 		}
 		if (VGZ_ObufStorage(w, vg))
-			return (-1);
+			return(FetchError(w, "Could not get storage"));
 		i = VGZ_Gunzip(vg, &dp, &dl);
 		xxxassert(i == VGZ_OK || i == VGZ_END);
 		VEP_Parse(w, dp, dl);
@@ -297,7 +297,6 @@ vfp_esi_begin(struct worker *w, size_t estimate)
 	struct vef_priv *vef;
 
 	CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
-	/* XXX: snapshot WS's ? We'll need the space */
 
 	AZ(w->vgz_rx);
 	if (w->is_gzip && w->do_gunzip) {
@@ -306,9 +305,6 @@ vfp_esi_begin(struct worker *w, size_t estimate)
 	} else if (w->is_gunzip && w->do_gzip) {
 		ALLOC_OBJ(vef, VEF_MAGIC);
 		AN(vef);
-		//vef = (void*)WS_Alloc(sp->ws, sizeof *vef);
-		//memset(vef, 0, sizeof *vef);
-		//vef->magic = VEF_MAGIC;
 		vef->vgz = VGZ_NewGzip(w, "G F E");
 		AZ(w->vef_priv);
 		w->vef_priv = vef;
@@ -317,9 +313,6 @@ vfp_esi_begin(struct worker *w, size_t estimate)
 		w->vgz_rx = VGZ_NewUngzip(w, "U F E");
 		ALLOC_OBJ(vef, VEF_MAGIC);
 		AN(vef);
-		//vef = (void*)WS_Alloc(sp->ws, sizeof *vef);
-		//memset(vef, 0, sizeof *vef);
-		//vef->magic = VEF_MAGIC;
 		vef->vgz = VGZ_NewGzip(w, "G F E");
 		AZ(w->vef_priv);
 		w->vef_priv = vef;
@@ -339,6 +332,7 @@ vfp_esi_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 	int i;
 
 	CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
+	AZ(w->fetch_failed);
 	AN(w->vep);
 	if (w->is_gzip && w->do_gunzip)
 		i = vfp_esi_bytes_gu(w, htc, bytes);
@@ -358,35 +352,48 @@ vfp_esi_end(struct worker *w)
 	struct vsb *vsb;
 	struct vef_priv *vef;
 	ssize_t l;
+	int retval;
 
 	CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
 	AN(w->vep);
 
+	retval = w->fetch_failed;
+
+	if (w->vgz_rx != NULL && VGZ_Destroy(&w->vgz_rx, -1) != VGZ_END)
+		retval = FetchError(w,
+		    "Gunzip+ESI Failed at the very end");
+
 	vsb = VEP_Finish(w);
 
 	if (vsb != NULL) {
-		l = VSB_len(vsb);
-		assert(l > 0);
-		/* XXX: This is a huge waste of storage... */
-		w->fetch_obj->esidata = STV_alloc(w, l);
-		XXXAN(w->fetch_obj->esidata);
-		memcpy(w->fetch_obj->esidata->ptr, VSB_data(vsb), l);
-		w->fetch_obj->esidata->len = l;
+		if (!retval) {
+			l = VSB_len(vsb);
+			assert(l > 0);
+			/* XXX: This is a huge waste of storage... */
+			w->fetch_obj->esidata = STV_alloc(w, l);
+			if (w->fetch_obj->esidata != NULL) {
+				memcpy(w->fetch_obj->esidata->ptr,
+				    VSB_data(vsb), l);
+				w->fetch_obj->esidata->len = l;
+			} else {
+				retval = FetchError(w,
+				    "Could not allocate storage for esidata");
+			}
+		}
 		VSB_delete(vsb);
 	}
-	if (w->vgz_rx != NULL)
-		VGZ_Destroy(&w->vgz_rx, -1);
 
 	if (w->vef_priv != NULL) {
 		vef = w->vef_priv;
 		CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);
 		w->vef_priv = NULL;
 		VGZ_UpdateObj(vef->vgz, w->fetch_obj);
-		VGZ_Destroy(&vef->vgz,  -1);
-		XXXAZ(vef->error);
+		if (VGZ_Destroy(&vef->vgz,  -1) != VGZ_END)
+			retval = FetchError(w, 
+			    "ESI+Gzip Failed at the very end");
 		FREE_OBJ(vef);
 	}
-	return (0);
+	return (retval);
 }
 
 struct vfp vfp_esi = {
diff --git a/bin/varnishd/cache_fetch.c b/bin/varnishd/cache_fetch.c
index 6e92963..dd3a1c7 100644
--- a/bin/varnishd/cache_fetch.c
+++ b/bin/varnishd/cache_fetch.c
@@ -43,6 +43,36 @@
 static unsigned fetchfrag;
 
 /*--------------------------------------------------------------------
+ * We want to issue the first error we encounter on fetching and
+ * supress the rest.  This function does that.
+ *
+ * Other code is allowed to look at w->fetch_failed to bail out
+ *
+ * For convenience, always return -1
+ */
+
+int
+FetchError2(struct worker *w, const char *error, const char *more)
+{
+
+	CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
+	if (!w->fetch_failed) {
+		if (more == NULL)
+			WSLB(w, SLT_FetchError, "%s", error);
+		else
+			WSLB(w, SLT_FetchError, "%s: %s", error, more);
+	}
+	w->fetch_failed = 1;
+	return (-1);
+}
+
+int
+FetchError(struct worker *w, const char *error)
+{
+	return(FetchError2(w, error, NULL));
+}
+
+/*--------------------------------------------------------------------
  * VFP_NOP
  *
  * This fetch-processor does nothing but store the object.
@@ -74,7 +104,8 @@ vfp_nop_begin(struct worker *w, size_t estimate)
  *
  * Process (up to) 'bytes' from the socket.
  *
- * Return -1 on error
+ * Return -1 on error, issue FetchError()
+ *	will not be called again, once error happens.
  * Return 0 on EOF on socket even if bytes not reached.
  * Return 1 when 'bytes' have been processed.
  */
@@ -85,17 +116,18 @@ vfp_nop_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 	ssize_t l, wl;
 	struct storage *st;
 
+	AZ(w->fetch_failed);
 	while (bytes > 0) {
 		st = FetchStorage(w, 0);
-		if (st == NULL) {
-			htc->error = "Could not get storage";
-			return (-1);
-		}
+		if (st == NULL)
+			return(FetchError(w, "Could not get storage"));
 		l = st->space - st->len;
 		if (l > bytes)
 			l = bytes;
 		wl = HTC_Read(htc, st->ptr + st->len, l);
-		if (wl <= 0)
+		if (wl < 0)
+			return(FetchError(w, htc->error));
+		if (wl == 0)
 			return (wl);
 		st->len += wl;
 		w->fetch_obj->len += wl;
@@ -205,17 +237,13 @@ fetch_straight(struct worker *w, struct http_conn *htc, ssize_t cl)
 	assert(w->body_status == BS_LENGTH);
 
 	if (cl < 0) {
-		WSLB(w, SLT_FetchError, "straight length field bogus");
-		return (-1);
+		return (FetchError(w, "straight length field bogus"));
 	} else if (cl == 0)
 		return (0);
 
 	i = w->vfp->bytes(w, htc, cl);
-	if (i <= 0) {
-		WSLB(w, SLT_FetchError, "straight read_error: %d %d (%s)",
-		    i, errno, htc->error);
-		return (-1);
-	}
+	if (i <= 0)
+		return (FetchError(w, "straight insufficient bytes"));
 	return (0);
 }
 
@@ -257,10 +285,8 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
 				break;
 		}
 
-		if (u >= sizeof buf) {
-			WSLB(w, SLT_FetchError,	"chunked header too long");
-			return (-1);
-		}
+		if (u >= sizeof buf) 
+			return (FetchError(w,"chunked header too long"));
 
 		/* Skip trailing white space */
 		while(vct_islws(buf[u]) && buf[u] != '\n') {
@@ -268,19 +294,17 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
 			CERR();
 		}
 
-		if (buf[u] != '\n') {
-			WSLB(w, SLT_FetchError,	"chunked header char syntax");
-			return (-1);
-		}
+		if (buf[u] != '\n') 
+			return (FetchError(w,"chunked header no NL"));
 		buf[u] = '\0';
 
 		cl = fetch_number(buf, 16);
 		if (cl < 0) {
-			WSLB(w, SLT_FetchError,	"chunked header nbr syntax");
-			return (-1);
+			return (FetchError(w,"chunked header number syntax"));
 		} else if (cl > 0) {
 			i = w->vfp->bytes(w, htc, cl);
-			CERR();
+			if (i <= 0)
+				return (-1);
 		}
 		i = HTC_Read(htc, buf, 1);
 		CERR();
@@ -288,10 +312,8 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
 			i = HTC_Read(htc, buf, 1);
 			CERR();
 		}
-		if (buf[0] != '\n') {
-			WSLB(w, SLT_FetchError,	"chunked tail syntax");
-			return (-1);
-		}
+		if (buf[0] != '\n')
+			return (FetchError(w,"chunked tail no NL"));
 	} while (cl > 0);
 	return (0);
 }
@@ -307,11 +329,8 @@ fetch_eof(struct worker *w, struct http_conn *htc)
 
 	assert(w->body_status == BS_EOF);
 	i = w->vfp->bytes(w, htc, SSIZE_MAX);
-	if (i < 0) {
-		WSLB(w, SLT_FetchError, "eof read_error: %d (%s)",
-		    errno, htc->error);
+	if (i < 0) 
 		return (-1);
-	}
 	return (0);
 }
 
@@ -497,6 +516,7 @@ FetchBody(struct worker *w, struct object *obj)
 	AZ(VTAILQ_FIRST(&obj->store));
 
 	w->fetch_obj = obj;
+	w->fetch_failed = 0;
 
 	/* XXX: pick up estimate from objdr ? */
 	cl = 0;
@@ -572,6 +592,7 @@ FetchBody(struct worker *w, struct object *obj)
 		obj->len = 0;
 		return (__LINE__);
 	}
+	AZ(w->fetch_failed);
 
 	if (cls == 0 && w->do_close)
 		cls = 1;
@@ -588,6 +609,7 @@ FetchBody(struct worker *w, struct object *obj)
 		if (w->do_stream)
 			/* Streaming might have started freeing stuff */
 			assert (uu <= obj->len);
+
 		else
 			assert(uu == obj->len);
 	}
diff --git a/bin/varnishd/cache_gzip.c b/bin/varnishd/cache_gzip.c
index 7df69cd..73f4bb9 100644
--- a/bin/varnishd/cache_gzip.c
+++ b/bin/varnishd/cache_gzip.c
@@ -81,7 +81,7 @@ struct vgz {
 	const char		*id;
 	struct ws		*tmp;
 	char			*tmp_snapshot;
-	const char		*error;
+	int			last_i;
 
 	struct storage		*obuf;
 
@@ -201,8 +201,6 @@ VGZ_NewGzip(struct worker *wrk, const char *id)
 	    16 + params->gzip_window,	/* Window bits (16=gzip + 15) */
 	    params->gzip_memlevel,	/* memLevel */
 	    Z_DEFAULT_STRATEGY);
-	if (i != Z_OK)
-		printf("deflateInit2() = %d\n", i);
 	assert(Z_OK == i);
 	return (vg);
 }
@@ -259,10 +257,8 @@ VGZ_ObufStorage(struct worker *w, struct vgz *vg)
 	struct storage *st;
 
 	st = FetchStorage(w, 0);
-	if (st == NULL) {
-		vg->error = "Could not get ObufStorage";
+	if (st == NULL) 
 		return (-1);
-	}
 
 	vg->obuf = st;
 	VGZ_Obuf(vg, st->ptr + st->len, st->space - st->len);
@@ -294,6 +290,7 @@ VGZ_Gunzip(struct vgz *vg, const void **pptr, size_t *plen)
 		if (vg->obuf != NULL)
 			vg->obuf->len += l;
 	}
+	vg->last_i = i;
 	if (i == Z_OK)
 		return (VGZ_OK);
 	if (i == Z_STREAM_END)
@@ -336,6 +333,7 @@ VGZ_Gzip(struct vgz *vg, const void **pptr, size_t *plen, enum vgz_flag flags)
 		if (vg->obuf != NULL)
 			vg->obuf->len += l;
 	}
+	vg->last_i = i;
 	if (i == Z_OK)
 		return (0);
 	if (i == Z_STREAM_END)
@@ -405,11 +403,11 @@ VGZ_UpdateObj(const struct vgz *vg, struct object *obj)
  * Passing a vsl_id of -1 means "use w->vbc->vsl_id"
  */
 
-void
+int
 VGZ_Destroy(struct vgz **vgp, int vsl_id)
 {
 	struct vgz *vg;
-	const char *err;
+	int i;
 
 	vg = *vgp;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
@@ -431,14 +429,22 @@ VGZ_Destroy(struct vgz **vgp, int vsl_id)
 		    (intmax_t)vg->vz.start_bit,
 		    (intmax_t)vg->vz.last_bit,
 		    (intmax_t)vg->vz.stop_bit);
-	err = vg->error;
 	if (vg->tmp != NULL)
 		WS_Reset(vg->tmp, vg->tmp_snapshot);
 	if (vg->dir == VGZ_GZ)
-		assert(deflateEnd(&vg->vz) == 0 || err != NULL);
+		i = deflateEnd(&vg->vz);
 	else
-		assert(inflateEnd(&vg->vz) == 0 || err != NULL);
+		i = inflateEnd(&vg->vz);
+	if (vg->last_i == Z_STREAM_END && i == Z_OK)
+		i = Z_STREAM_END;
 	FREE_OBJ(vg);
+	if (i == Z_OK)
+		return (VGZ_OK);
+	if (i == Z_STREAM_END)
+		return (VGZ_END);
+	if (i == Z_BUF_ERROR)
+		return (VGZ_STUCK);
+	return (VGZ_ERROR);
 }
 
 /*--------------------------------------------------------------------
@@ -465,6 +471,7 @@ vfp_gunzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 	size_t dl;
 	const void *dp;
 
+	AZ(w->fetch_failed);
 	vg = w->vgz_rx;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 	AZ(vg->vz.avail_in);
@@ -474,27 +481,25 @@ vfp_gunzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 			if (l > bytes)
 				l = bytes;
 			wl = HTC_Read(htc, ibuf, l);
-			if (wl <= 0)
+			if (wl < 0)
+				return(FetchError(w, htc->error));
+			if (wl == 0)
 				return (wl);
 			VGZ_Ibuf(vg, ibuf, wl);
 			bytes -= wl;
 		}
 
-		if (VGZ_ObufStorage(w, vg)) {
-			htc->error = "Could not get storage";
-			return (-1);
-		}
+		if (VGZ_ObufStorage(w, vg)) 
+			return(FetchError(w, "Could not get storage"));
 		i = VGZ_Gunzip(vg, &dp, &dl);
-		assert(i == VGZ_OK || i == VGZ_END);
+		if (i != VGZ_OK && i != VGZ_END) 
+			return(FetchError(w, "Gunzip data error"));
 		w->fetch_obj->len += dl;
 		if (w->do_stream)
 			RES_StreamPoll(w);
 	}
-	if (i == Z_OK || i == Z_STREAM_END)
-		return (1);
-	htc->error = "See other message";
-	WSLB(w, SLT_FetchError, "Gunzip trouble (%d)", i);
-	return (-1);
+	assert(i == Z_OK || i == Z_STREAM_END);
+	return (1);
 }
 
 static int __match_proto__()
@@ -505,7 +510,12 @@ vfp_gunzip_end(struct worker *w)
 	vg = w->vgz_rx;
 	w->vgz_rx = NULL;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
-	VGZ_Destroy(&vg, -1);
+	if (w->fetch_failed) {
+		(void)VGZ_Destroy(&vg, -1);
+		return(0);
+	}
+	if (VGZ_Destroy(&vg, -1) != VGZ_END)
+		return(FetchError(w, "Gunzip error at the very end"));
 	return (0);
 }
 
@@ -541,6 +551,7 @@ vfp_gzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 	size_t dl;
 	const void *dp;
 
+	AZ(w->fetch_failed);
 	vg = w->vgz_rx;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 	AZ(vg->vz.avail_in);
@@ -550,15 +561,15 @@ vfp_gzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 			if (l > bytes)
 				l = bytes;
 			wl = HTC_Read(htc, ibuf, l);
-			if (wl <= 0)
+			if (wl < 0)
+				return(FetchError(w, htc->error));
+			if (wl == 0)
 				return (wl);
 			VGZ_Ibuf(vg, ibuf, wl);
 			bytes -= wl;
 		}
-		if (VGZ_ObufStorage(w, vg)) {
-			htc->error = "Could not get storage";
-			return (-1);
-		}
+		if (VGZ_ObufStorage(w, vg)) 
+			return(FetchError(w, "Could not get storage"));
 		i = VGZ_Gzip(vg, &dp, &dl, VGZ_NORMAL);
 		assert(i == Z_OK);
 		w->fetch_obj->len += dl;
@@ -579,19 +590,22 @@ vfp_gzip_end(struct worker *w)
 	vg = w->vgz_rx;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 	w->vgz_rx = NULL;
-	if (vg->error == NULL) {
-		do {
-			VGZ_Ibuf(vg, "", 0);
-			if (VGZ_ObufStorage(w, vg))
-				return (-1);
-			i = VGZ_Gzip(vg, &dp, &dl, VGZ_FINISH);
-			w->fetch_obj->len += dl;
-		} while (i != Z_STREAM_END);
-		if (w->do_stream)
-			RES_StreamPoll(w);
-		VGZ_UpdateObj(vg, w->fetch_obj);
+	if (w->fetch_failed) {
+		(void)VGZ_Destroy(&vg, -1);
+		return(0);
 	}
-	VGZ_Destroy(&vg, -1);
+	do {
+		VGZ_Ibuf(vg, "", 0);
+		if (VGZ_ObufStorage(w, vg))
+			return(FetchError(w, "Could not get storage"));
+		i = VGZ_Gzip(vg, &dp, &dl, VGZ_FINISH);
+		w->fetch_obj->len += dl;
+	} while (i != Z_STREAM_END);
+	if (w->do_stream)
+		RES_StreamPoll(w);
+	VGZ_UpdateObj(vg, w->fetch_obj);
+	if (VGZ_Destroy(&vg, -1) != VGZ_END)
+		return(FetchError(w, "Gzip error at the very end"));
 	return (0);
 }
 
@@ -627,21 +641,21 @@ vfp_testgzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 	const void *dp;
 	struct storage *st;
 
+	AZ(w->fetch_failed);
 	vg = w->vgz_rx;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
 	AZ(vg->vz.avail_in);
 	while (bytes > 0) {
 		st = FetchStorage(w, 0);
-		if (st == NULL) {
-			htc->error = "Could not get storage";
-			vg->error = htc->error;
-			return (-1);
-		}
+		if (st == NULL)
+			return(FetchError(w, "Could not get storage"));
 		l = st->space - st->len;
 		if (l > bytes)
 			l = bytes;
 		wl = HTC_Read(htc, st->ptr + st->len, l);
-		if (wl <= 0)
+		if (wl < 0)
+			return(FetchError(w, htc->error));
+		if (wl == 0)
 			return (wl);
 		bytes -= wl;
 		VGZ_Ibuf(vg, st->ptr + st->len, wl);
@@ -653,23 +667,15 @@ vfp_testgzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
 		while (!VGZ_IbufEmpty(vg)) {
 			VGZ_Obuf(vg, obuf, sizeof obuf);
 			i = VGZ_Gunzip(vg, &dp, &dl);
-			if (i == VGZ_END && !VGZ_IbufEmpty(vg)) {
-				htc->error = "Junk after gzip data";
-				return (-1);
-			}
-			if (i != VGZ_OK && i != VGZ_END) {
-				htc->error = "See other message";
-				WSLB(w, SLT_FetchError,
-				    "Invalid Gzip data: %s", vg->vz.msg);
-				return (-1);
-			}
+			if (i == VGZ_END && !VGZ_IbufEmpty(vg)) 
+				return(FetchError(w, "Junk after gzip data"));
+			if (i != VGZ_OK && i != VGZ_END)
+				return(FetchError2(w,
+				    "Invalid Gzip data", vg->vz.msg));
 		}
 	}
-	if (i == VGZ_OK || i == VGZ_END)
-		return (1);
-	htc->error = "See other message";
-	WSLB(w, SLT_FetchError, "Gunzip trouble (%d)", i);
-	return (-1);
+	assert(i == VGZ_OK || i == VGZ_END);
+	return (1);
 }
 
 static int __match_proto__()
@@ -680,8 +686,13 @@ vfp_testgzip_end(struct worker *w)
 	vg = w->vgz_rx;
 	w->vgz_rx = NULL;
 	CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
+	if (w->fetch_failed) {
+		(void)VGZ_Destroy(&vg, -1);
+		return(0);
+	}
 	VGZ_UpdateObj(vg, w->fetch_obj);
-	VGZ_Destroy(&vg, -1);
+	if (VGZ_Destroy(&vg, -1) != VGZ_END)
+		return(FetchError(w, "TestGunzip error at the very end"));
 	return (0);
 }
 
diff --git a/bin/varnishd/cache_response.c b/bin/varnishd/cache_response.c
index 84606b6..04d2da5 100644
--- a/bin/varnishd/cache_response.c
+++ b/bin/varnishd/cache_response.c
@@ -182,7 +182,7 @@ res_WriteGunzipObj(const struct sess *sp)
 		(void)WRW_Write(sp->wrk, obuf, obufl);
 		(void)WRW_Flush(sp->wrk);
 	}
-	VGZ_Destroy(&vg, sp->vsl_id);
+	(void)VGZ_Destroy(&vg, sp->vsl_id);
 	assert(u == sp->obj->len);
 }
 
diff --git a/bin/varnishtest/tests/g00004.vtc b/bin/varnishtest/tests/g00004.vtc
new file mode 100644
index 0000000..5d518e0
--- /dev/null
+++ b/bin/varnishtest/tests/g00004.vtc
@@ -0,0 +1,43 @@
+varnishtest "truncated gzip from backend"
+
+server s1 -repeat 2 {
+	rxreq
+	txresp -nolen \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Transfer-Encoding: Chunked"
+	send "18\r\n"
+		# A truncate gzip file
+		sendhex "1f8b"
+		sendhex "08"
+		sendhex "00"
+		sendhex "f5 64 ae 4e  02 03 f3 cd cf 53 f0 4f"
+		sendhex "2e 51 30 36 54 30 b0 b4"
+	send "\r\n"
+	chunkedlen 0
+
+} -start
+
+varnish v1 \
+	-arg {-p diag_bitmap=0x00010000} \
+	-vcl+backend {
+	sub vcl_fetch {
+		if (req.url == "/gunzip") {
+			set beresp.do_gunzip = true;
+		} 
+	}
+} 
+
+varnish v1 -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 503
+} -run
+
+client c1 {
+	txreq -url /gunzip
+	rxresp
+	expect resp.status == 503
+} -run
+
diff --git a/bin/varnishtest/tests/r01037.vtc b/bin/varnishtest/tests/r01037.vtc
new file mode 100644
index 0000000..acb77d3
--- /dev/null
+++ b/bin/varnishtest/tests/r01037.vtc
@@ -0,0 +1,29 @@
+varnishtest "Test case for #1037"
+
+server s1 {
+	rxreq
+	# Send a bodylen of 1,5M, which will exceed cache space of 1M
+	non-fatal
+	txresp -nolen -hdr "Transfer-encoding: chunked"
+	chunked {<HTML>}
+	chunkedlen 500000
+	chunked {<esi:remove>}
+	chunkedlen 500000
+	chunked {</esi:remove>}
+	chunkedlen 500000
+	chunkedlen 0
+} -start
+
+varnish v1 -arg "-smalloc,1M" -arg "-pgzip_level=0" -vcl+backend {
+	sub vcl_fetch {
+		set beresp.do_esi = true;
+		set beresp.do_gzip = true;
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == "503"
+} -run
+



More information about the varnish-commit mailing list