[experimental-ims] 7c784d5 Add long time missing error handling of gunzip'ing fetched objects for ESI processing.

Poul-Henning Kamp phk at FreeBSD.org
Thu Dec 18 10:27:53 CET 2014


commit 7c784d5c9d2dd959a5ea1a1bea5f7bbb4173437d
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Aug 21 08:19:44 2012 +0000

    Add long time missing error handling of gunzip'ing fetched objects
    for ESI processing.
    
    Polish the VGZ code a bit while here anyway.
    
    Fixes	#1184

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 2441bd6..43c104e 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -803,6 +803,13 @@ void Fetch_Init(void);
 /* cache_gzip.c */
 struct vgz;
 
+enum vgzret_e {
+	VGZ_ERROR = -1,
+	VGZ_OK = 0,
+	VGZ_END = 1,
+	VGZ_STUCK = 2,
+};
+
 enum vgz_flag { VGZ_NORMAL, VGZ_ALIGN, VGZ_RESET, VGZ_FINISH };
 struct vgz *VGZ_NewUngzip(struct vsl_log *vsl, const char *id);
 struct vgz *VGZ_NewGzip(struct vsl_log *vsl, const char *id);
@@ -811,22 +818,16 @@ int VGZ_IbufEmpty(const struct vgz *vg);
 void VGZ_Obuf(struct vgz *, void *, ssize_t len);
 int VGZ_ObufFull(const struct vgz *vg);
 int VGZ_ObufStorage(struct busyobj *, 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);
-int VGZ_Destroy(struct vgz **);
+enum vgzret_e VGZ_Gzip(struct vgz *, const void **, size_t *len, enum vgz_flag);
+enum vgzret_e VGZ_Gunzip(struct vgz *, const void **, size_t *len);
+enum vgzret_e VGZ_Destroy(struct vgz **);
 void VGZ_UpdateObj(const struct vgz*, struct object *);
 
 int VGZ_WrwInit(struct vgz *vg);
-int VGZ_WrwGunzip(struct req *, struct vgz *, const void *ibuf,
+enum vgzret_e VGZ_WrwGunzip(struct req *, struct vgz *, const void *ibuf,
     ssize_t ibufl);
 void VGZ_WrwFlush(struct req *, struct vgz *vg);
 
-/* Return values */
-#define VGZ_ERROR	-1
-#define VGZ_OK		0
-#define VGZ_END		1
-#define VGZ_STUCK	2
-
 /* cache_http.c */
 unsigned HTTP_estimate(unsigned nhttp);
 void HTTP_Copy(struct http *to, const struct http * const fm);
diff --git a/bin/varnishd/cache/cache_esi_fetch.c b/bin/varnishd/cache/cache_esi_fetch.c
index c9cc689..7dc74ae 100644
--- a/bin/varnishd/cache/cache_esi_fetch.c
+++ b/bin/varnishd/cache/cache_esi_fetch.c
@@ -119,7 +119,7 @@ vfp_esi_bytes_gu(struct busyobj *bo, const struct vef_priv *vef,
 {
 	struct vgz *vg;
 	ssize_t wl;
-	int i;
+	enum vgzret_e vr;
 	size_t dl;
 	const void *dp;
 
@@ -137,10 +137,13 @@ vfp_esi_bytes_gu(struct busyobj *bo, const struct vef_priv *vef,
 		}
 		if (VGZ_ObufStorage(bo, vg))
 			return(-1);
-		i = VGZ_Gunzip(vg, &dp, &dl);
-		xxxassert(i == VGZ_OK || i == VGZ_END);
-		VEP_Parse(bo, dp, dl);
-		VFP_update_length(bo, dl);
+		vr = VGZ_Gunzip(vg, &dp, &dl);
+		if (vr < VGZ_OK)
+			return (-1);
+		if (dl > 0) {
+			VEP_Parse(bo, dp, dl);
+			VFP_update_length(bo, dl);
+		}
 	}
 	return (1);
 }
@@ -265,7 +268,7 @@ vfp_esi_bytes_gg(const struct busyobj *bo, struct vef_priv *vef,
 	ssize_t wl;
 	size_t dl;
 	const void *dp;
-	int i;
+	enum vgzret_e vr;
 
 	CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -280,9 +283,9 @@ vfp_esi_bytes_gg(const struct busyobj *bo, struct vef_priv *vef,
 		do {
 			wl = vef->ibuf_sz - (vef->ibuf_i - vef->ibuf);
 			VGZ_Obuf(bo->vgz_rx, vef->ibuf_i, wl);
-			i = VGZ_Gunzip(bo->vgz_rx, &dp, &dl);
-			/* XXX: check i */
-			assert(i >= VGZ_OK);
+			vr = VGZ_Gunzip(bo->vgz_rx, &dp, &dl);
+			if (vr < VGZ_OK)
+				return (-1);
 			if (dl > 0 && vfp_vep_inject(bo, vef, dl))
 				return (-1);
 		} while (!VGZ_IbufEmpty(bo->vgz_rx));
diff --git a/bin/varnishd/cache/cache_gzip.c b/bin/varnishd/cache/cache_gzip.c
index 80149c6..5a53cd8 100644
--- a/bin/varnishd/cache/cache_gzip.c
+++ b/bin/varnishd/cache/cache_gzip.c
@@ -218,7 +218,7 @@ VGZ_ObufStorage(struct busyobj *bo, struct vgz *vg)
 
 /*--------------------------------------------------------------------*/
 
-int
+enum vgzret_e
 VGZ_Gunzip(struct vgz *vg, const void **pptr, size_t *plen)
 {
 	int i;
@@ -247,13 +247,13 @@ VGZ_Gunzip(struct vgz *vg, const void **pptr, size_t *plen)
 		return (VGZ_END);
 	if (i == Z_BUF_ERROR)
 		return (VGZ_STUCK);
-	VSL(SLT_Debug, 0, "Unknown INFLATE=%d (%s)\n", i, vg->vz.msg);
+	VSLb(vg->vsl, SLT_Gzip, "Gunzip error: %d (%s)", i, vg->vz.msg);
 	return (VGZ_ERROR);
 }
 
 /*--------------------------------------------------------------------*/
 
-int
+enum vgzret_e
 VGZ_Gzip(struct vgz *vg, const void **pptr, size_t *plen, enum vgz_flag flags)
 {
 	int i;
@@ -285,12 +285,13 @@ VGZ_Gzip(struct vgz *vg, const void **pptr, size_t *plen, enum vgz_flag flags)
 	}
 	vg->last_i = i;
 	if (i == Z_OK)
-		return (0);
+		return (VGZ_OK);
 	if (i == Z_STREAM_END)
-		return (1);
+		return (VGZ_END);
 	if (i == Z_BUF_ERROR)
-		return (2);
-	return (-1);
+		return (VGZ_STUCK);
+	VSLb(vg->vsl, SLT_Gzip, "Gzip error: %d (%s)", i, vg->vz.msg);
+	return (VGZ_ERROR);
 }
 
 /*--------------------------------------------------------------------
@@ -314,11 +315,11 @@ VGZ_WrwInit(struct vgz *vg)
  * Leave flushing to caller, more data may be coming.
  */
 
-int
+enum vgzret_e
 VGZ_WrwGunzip(struct req *req, struct vgz *vg, const void *ibuf,
     ssize_t ibufl)
 {
-	int i;
+	enum vgzret_e vr;
 	size_t dl;
 	const void *dp;
 	struct worker *wrk;
@@ -333,16 +334,14 @@ VGZ_WrwGunzip(struct req *req, struct vgz *vg, const void *ibuf,
 		return (VGZ_OK);
 	do {
 		if (vg->m_len == vg->m_sz)
-			i = VGZ_STUCK;
+			vr = VGZ_STUCK;
 		else {
-			i = VGZ_Gunzip(vg, &dp, &dl);
+			vr = VGZ_Gunzip(vg, &dp, &dl);
 			vg->m_len += dl;
 		}
-		if (i < VGZ_OK) {
-			/* XXX: VSL ? */
-			return (-1);
-		}
-		if (vg->m_len == vg->m_sz || i == VGZ_STUCK) {
+		if (vr < VGZ_OK)
+			return (vr);
+		if (vg->m_len == vg->m_sz || vr == VGZ_STUCK) {
 			req->acct_req.bodybytes += vg->m_len;
 			(void)WRW_Write(wrk, vg->m_buf, vg->m_len);
 			(void)WRW_Flush(wrk);
@@ -350,9 +349,9 @@ VGZ_WrwGunzip(struct req *req, struct vgz *vg, const void *ibuf,
 			VGZ_Obuf(vg, vg->m_buf, vg->m_sz);
 		}
 	} while (!VGZ_IbufEmpty(vg));
-	if (i == VGZ_STUCK)
-		i = VGZ_OK;
-	return (i);
+	if (vr == VGZ_STUCK)
+		vr = VGZ_OK;
+	return (vr);
 }
 
 /*--------------------------------------------------------------------*/
@@ -393,10 +392,11 @@ VGZ_UpdateObj(const struct vgz *vg, struct object *obj)
 /*--------------------------------------------------------------------
  */
 
-int
+enum vgzret_e
 VGZ_Destroy(struct vgz **vgp)
 {
 	struct vgz *vg;
+	enum vgzret_e vr;
 	int i;
 
 	vg = *vgp;
@@ -420,14 +420,18 @@ VGZ_Destroy(struct vgz **vgp)
 		i = Z_STREAM_END;
 	if (vg->m_buf)
 		free(vg->m_buf);
-	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);
+		vr = VGZ_OK;
+	else if (i == Z_STREAM_END)
+		vr = VGZ_END;
+	else if (i == Z_BUF_ERROR)
+		vr = VGZ_STUCK;
+	else {
+		VSLb(vg->vsl, SLT_Gzip, "G(un)zip error: %d (%s)", i, vg->vz.msg);
+		vr = VGZ_ERROR;
+	}
+	FREE_OBJ(vg);
+	return (vr);
 }
 
 /*--------------------------------------------------------------------
diff --git a/bin/varnishtest/tests/r01184.vtc b/bin/varnishtest/tests/r01184.vtc
new file mode 100644
index 0000000..9a74f28
--- /dev/null
+++ b/bin/varnishtest/tests/r01184.vtc
@@ -0,0 +1,126 @@
+varnishtest "Corrupt gzip'ed ESI objects"
+
+# First, check that our data is actually good
+
+server s1 {
+	rxreq
+	expect req.url == "/"
+	txresp -nolen \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Transfer-Encoding: Chunked"
+	send "ed\r\n"
+                sendhex " 1f 8b 08 00 c2 39 33 50  02 03 45 90 4d 6a 03 31"
+                sendhex " 0c 85 f7 73 8a 47 2e 10  d9 f2 9f ca 34 d0 c2 64"
+                sendhex " 5b 08 bd 80 2d cb 10 28  5d 34 f4 fe f5 30 d0 68"
+                sendhex " 25 89 a7 f7 3e b4 be 6f  d7 8f db 76 59 e8 28 d8"
+                sendhex " 10 45 f3 a9 83 b8 18 1c  7b c2 30 55 04 17 13 c4"
+                sendhex " 0f 07 5f 7a 38 f4 8e 50  b3 37 d4 3a 32 4a 34 07"
+                sendhex " 8a 9c d1 92 77 48 d4 0a  72 ea 06 5f b3 1c fa dd"
+                sendhex " 2b b9 88 20 99 e6 9a 3c  84 7c 85 8e 50 e0 59 2a"
+                sendhex " 42 b0 8a 34 0f 96 d5 1e  f7 97 fb b7 7e fd 4e 87"
+                sendhex " c7 8f be 9e ce fb 74 3a  3f 51 89 a3 9b 7e b2 43"
+                sendhex " a4 86 a2 55 90 b9 29 4c  4b 83 b8 99 5f b5 bb 27"
+                sendhex " 6a d4 86 18 22 83 8a 26  f4 11 1a 5c eb 34 3b ca"
+                sendhex " 20 31 9e 12 29 ff a8 92  78 a2 e6 ec 61 55 12 fc"
+                sendhex " 68 84 6c 12 41 b9 cf 2f  30 3b f0 10 5e d6 b7 eb"
+                sendhex " e7 76 bb 2c 7f 8c 90 4a  14 4c 01 00 00"
+	send "\r\n"
+	chunkedlen 0
+	rxreq
+	expect req.url == "/incl"
+	txresp -body "INCLUDED\n"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_fetch {
+		set beresp.do_esi = true;
+	}
+} -start
+
+client c1 {
+	txreq -url "/"
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 315
+} -run
+
+# Second, corrupt them, while using the g-g path
+
+server s1 {
+	rxreq
+	expect req.url == "/c"
+	txresp -nolen \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Transfer-Encoding: Chunked"
+	send "ed\r\n"
+                sendhex " 1f 8b 08 00 c2 39 33 50  02 03 45 90 4d 6a 03 31"
+                sendhex " 0c 85 f7 73 8a 47 2e 10  d9 f2 9f ca 34 d0 c2 64"
+                sendhex " 5b 08 bd 80 2d cb 10 28  5d 34 f4 fe f5 30 d0 68"
+                sendhex " 25 89 a7 f7 3e b4 be 6f  d7 8f db 76 59 e8 28 d8"
+                sendhex " 10 45 f3 a9 83 b8 18 1c  7b c2 30 55 04 17 13 c4"
+                sendhex " 0f 07 5f 7a 38 f4 8e 50  b3 37 d4 3a 32 4a 34 07"
+                sendhex " FF FF FF FF FF FF FF FF  72 ea 06 5f b3 1c fa dd"
+                sendhex " 2b b9 88 20 99 e6 9a 3c  84 7c 85 8e 50 e0 59 2a"
+                sendhex " 42 b0 8a 34 0f 96 d5 1e  f7 97 fb b7 7e fd 4e 87"
+                sendhex " c7 8f be 9e ce fb 74 3a  3f 51 89 a3 9b 7e b2 43"
+                sendhex " a4 86 a2 55 90 b9 29 4c  4b 83 b8 99 5f b5 bb 27"
+                sendhex " 6a d4 86 18 22 83 8a 26  f4 11 1a 5c eb 34 3b ca"
+                sendhex " 20 31 9e 12 29 ff a8 92  78 a2 e6 ec 61 55 12 fc"
+                sendhex " 68 84 6c 12 41 b9 cf 2f  30 3b f0 10 5e d6 b7 eb"
+                sendhex " e7 76 bb 2c 7f 8c 90 4a  14 4c 01 00 00"
+	send "\r\n"
+	chunkedlen 0
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_fetch {
+		set beresp.do_esi = true;
+	}
+}
+
+client c1 {
+	txreq -url "/c"
+	rxresp
+	expect resp.status == 503
+} -run
+
+# Third, corrupt them, while using the g-u path
+
+server s1 {
+	rxreq
+	expect req.url == "/d"
+	txresp -nolen \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Transfer-Encoding: Chunked"
+	send "ed\r\n"
+                sendhex " 1f 8b 08 00 c2 39 33 50  02 03 45 90 4d 6a 03 31"
+                sendhex " 0c 85 f7 73 8a 47 2e 10  d9 f2 9f ca 34 d0 c2 64"
+                sendhex " 5b 08 bd 80 2d cb 10 28  5d 34 f4 fe f5 30 d0 68"
+                sendhex " 25 89 a7 f7 3e b4 be 6f  d7 8f db 76 59 e8 28 d8"
+                sendhex " 10 45 f3 a9 83 b8 18 1c  7b c2 30 55 04 17 13 c4"
+                sendhex " 0f 07 5f 7a 38 f4 8e 50  b3 37 d4 3a 32 4a 34 07"
+                sendhex " FF FF FF FF FF FF FF FF  72 ea 06 5f b3 1c fa dd"
+                sendhex " 2b b9 88 20 99 e6 9a 3c  84 7c 85 8e 50 e0 59 2a"
+                sendhex " 42 b0 8a 34 0f 96 d5 1e  f7 97 fb b7 7e fd 4e 87"
+                sendhex " c7 8f be 9e ce fb 74 3a  3f 51 89 a3 9b 7e b2 43"
+                sendhex " a4 86 a2 55 90 b9 29 4c  4b 83 b8 99 5f b5 bb 27"
+                sendhex " 6a d4 86 18 22 83 8a 26  f4 11 1a 5c eb 34 3b ca"
+                sendhex " 20 31 9e 12 29 ff a8 92  78 a2 e6 ec 61 55 12 fc"
+                sendhex " 68 84 6c 12 41 b9 cf 2f  30 3b f0 10 5e d6 b7 eb"
+                sendhex " e7 76 bb 2c 7f 8c 90 4a  14 4c 01 00 00"
+	send "\r\n"
+	chunkedlen 0
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_fetch {
+		set beresp.do_esi = true;
+		set beresp.do_gunzip = true;
+	}
+}
+
+client c1 {
+	txreq -url "/d"
+	rxresp
+	expect resp.status == 503
+} -run



More information about the varnish-commit mailing list