Commit 856bd826 authored by Poul-Henning Kamp's avatar Poul-Henning Kamp Committed by Tollef Fog Heen

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

Conflicts:

	bin/varnishd/cache.h
	bin/varnishd/cache_center.c
	bin/varnishd/cache_esi_deliver.c
	bin/varnishd/cache_esi_fetch.c
	bin/varnishd/cache_fetch.c
	bin/varnishd/cache_gzip.c
	bin/varnishd/cache_response.c
parent 999d163d
......@@ -331,6 +331,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;
......@@ -718,6 +719,8 @@ int EXP_NukeOne(struct worker *w, struct lru *lru);
/* cache_fetch.c */
struct storage *FetchStorage(const struct sess *sp, ssize_t sz);
int FetchError(struct sess *sp, const char *error);
int FetchError2(struct sess *sp, const char *error, const char *more);
int FetchHdr(struct sess *sp);
int FetchBody(struct sess *sp);
int FetchReqBody(struct sess *sp);
......@@ -736,7 +739,7 @@ int VGZ_ObufFull(const struct vgz *vg);
int VGZ_ObufStorage(const struct sess *sp, 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 VGZ_Destroy(struct vgz **);
void VGZ_UpdateObj(const struct vgz*, struct object *);
int VGZ_WrwGunzip(const struct sess *, struct vgz *, const void *ibuf,
ssize_t ibufl, char *obuf, ssize_t obufl, ssize_t *obufp);
......
......@@ -935,7 +935,7 @@ cnt_streambody(struct sess *sp)
RES_StreamEnd(sp);
if (sp->wrk->res_mode & RES_GUNZIP)
VGZ_Destroy(&sctx.vgz);
(void)VGZ_Destroy(&sctx.vgz);
sp->wrk->sctx = NULL;
assert(WRW_IsReleased(sp->wrk));
......
......@@ -408,7 +408,7 @@ ESI_Deliver(struct sess *sp)
if (vgz != NULL) {
if (obufl > 0)
(void)WRW_Write(sp->wrk, obuf, obufl);
VGZ_Destroy(&vgz);
(void)VGZ_Destroy(&vgz);
}
if (sp->wrk->gzip_resp && sp->esi_level == 0) {
/* Emit a gzip literal block with finish bit set */
......
......@@ -114,7 +114,7 @@ vfp_esi_bytes_gu(struct sess *sp, struct http_conn *htc, ssize_t bytes)
bytes -= w;
}
if (VGZ_ObufStorage(sp, vg))
return (-1);
return(FetchError(sp, "Could not get storage"));
i = VGZ_Gunzip(vg, &dp, &dl);
xxxassert(i == VGZ_OK || i == VGZ_END);
VEP_parse(sp, dp, dl);
......@@ -299,7 +299,6 @@ vfp_esi_begin(struct sess *sp, size_t estimate)
struct vef_priv *vef;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
/* XXX: snapshot WS's ? We'll need the space */
AZ(sp->wrk->vgz_rx);
if (sp->wrk->is_gzip && sp->wrk->do_gunzip) {
......@@ -308,9 +307,6 @@ vfp_esi_begin(struct sess *sp, size_t estimate)
} else if (sp->wrk->is_gunzip && sp->wrk->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(sp, "G F E");
AZ(sp->wrk->vef_priv);
sp->wrk->vef_priv = vef;
......@@ -319,9 +315,6 @@ vfp_esi_begin(struct sess *sp, size_t estimate)
sp->wrk->vgz_rx = VGZ_NewUngzip(sp, "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(sp, "G F E");
AZ(sp->wrk->vef_priv);
sp->wrk->vef_priv = vef;
......@@ -341,6 +334,7 @@ vfp_esi_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
int i;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
AZ(sp->wrk->fetch_failed);
AN(sp->wrk->vep);
if (sp->wrk->is_gzip && sp->wrk->do_gunzip)
i = vfp_esi_bytes_gu(sp, htc, bytes);
......@@ -360,35 +354,48 @@ vfp_esi_end(struct sess *sp)
struct vsb *vsb;
struct vef_priv *vef;
ssize_t l;
int retval;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
AN(sp->wrk->vep);
retval = sp->wrk->fetch_failed;
if (sp->wrk->vgz_rx != NULL && VGZ_Destroy(&sp->wrk->vgz_rx) != VGZ_END)
retval = FetchError(sp,
"Gunzip+ESI Failed at the very end");
vsb = VEP_Finish(sp);
if (vsb != NULL) {
l = VSB_len(vsb);
assert(l > 0);
/* XXX: This is a huge waste of storage... */
sp->obj->esidata = STV_alloc(sp, l);
XXXAN(sp->obj->esidata);
memcpy(sp->obj->esidata->ptr, VSB_data(vsb), l);
sp->obj->esidata->len = l;
if (!retval) {
l = VSB_len(vsb);
assert(l > 0);
/* XXX: This is a huge waste of storage... */
sp->obj->esidata = STV_alloc(sp, l);
if (sp->obj->esidata != NULL) {
memcpy(sp->obj->esidata->ptr,
VSB_data(vsb), l);
sp->obj->esidata->len = l;
} else {
retval = FetchError(sp,
"Could not allocate storage for esidata");
}
}
VSB_delete(vsb);
}
if (sp->wrk->vgz_rx != NULL)
VGZ_Destroy(&sp->wrk->vgz_rx);
if (sp->wrk->vef_priv != NULL) {
vef = sp->wrk->vef_priv;
CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);
sp->wrk->vef_priv = NULL;
VGZ_UpdateObj(vef->vgz, sp->obj);
VGZ_Destroy(&vef->vgz);
XXXAZ(vef->error);
if (VGZ_Destroy(&vef->vgz) != VGZ_END)
retval = FetchError(sp,
"ESI+Gzip Failed at the very end");
FREE_OBJ(vef);
}
return (0);
return (retval);
}
struct vfp vfp_esi = {
......
......@@ -42,6 +42,40 @@
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 sess *sp, const char *error, const char *more)
{
struct worker *w;
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(sp->wrk, WORKER_MAGIC);
w = sp->wrk;
if (!w->fetch_failed) {
if (more == NULL)
WSP(sp, SLT_FetchError, "%s", error);
else
WSP(sp, SLT_FetchError, "%s: %s", error, more);
}
w->fetch_failed = 1;
return (-1);
}
int
FetchError(struct sess *sp, const char *error)
{
return(FetchError2(sp, error, NULL));
}
/*--------------------------------------------------------------------
* VFP_NOP
*
......@@ -75,7 +109,8 @@ vfp_nop_begin(struct sess *sp, 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.
*/
......@@ -86,17 +121,18 @@ vfp_nop_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
ssize_t l, w;
struct storage *st;
AZ(sp->wrk->fetch_failed);
while (bytes > 0) {
st = FetchStorage(sp, 0);
if (st == NULL) {
htc->error = "Could not get storage";
return (-1);
}
if (st == NULL)
return(FetchError(sp, "Could not get storage"));
l = st->space - st->len;
if (l > bytes)
l = bytes;
w = HTC_Read(htc, st->ptr + st->len, l);
if (w <= 0)
if (w < 0)
return(FetchError(sp, htc->error));
if (w == 0)
return (w);
st->len += w;
sp->obj->len += w;
......@@ -203,16 +239,13 @@ fetch_straight(struct sess *sp, struct http_conn *htc, ssize_t cl)
assert(sp->wrk->body_status == BS_LENGTH);
if (cl < 0) {
WSP(sp, SLT_FetchError, "straight length field bogus");
return (-1);
return (FetchError(sp, "straight length field bogus"));
} else if (cl == 0)
return (0);
i = sp->wrk->vfp->bytes(sp, htc, cl);
if (i <= 0) {
WSP(sp, SLT_FetchError, "straight read_error: %d %d (%s)",
i, errno, htc->error);
return (-1);
return (FetchError(sp, "straight insufficient bytes"));
}
return (0);
}
......@@ -256,8 +289,7 @@ fetch_chunked(struct sess *sp, struct http_conn *htc)
}
if (u >= sizeof buf) {
WSP(sp, SLT_FetchError, "chunked header too long");
return (-1);
return (FetchError(sp,"chunked header too long"));
}
/* Skip trailing white space */
......@@ -267,15 +299,13 @@ fetch_chunked(struct sess *sp, struct http_conn *htc)
}
if (buf[u] != '\n') {
WSP(sp, SLT_FetchError, "chunked header char syntax");
return (-1);
return (FetchError(sp,"chunked header no NL"));
}
buf[u] = '\0';
cl = fetch_number(buf, 16);
if (cl < 0) {
WSP(sp, SLT_FetchError, "chunked header nbr syntax");
return (-1);
return (FetchError(sp,"chunked header number syntax"));
} else if (cl > 0) {
i = sp->wrk->vfp->bytes(sp, htc, cl);
CERR();
......@@ -286,10 +316,8 @@ fetch_chunked(struct sess *sp, struct http_conn *htc)
i = HTC_Read(htc, buf, 1);
CERR();
}
if (buf[0] != '\n') {
WSP(sp, SLT_FetchError, "chunked tail syntax");
return (-1);
}
if (buf[0] != '\n')
return (FetchError(sp,"chunked tail no NL"));
} while (cl > 0);
return (0);
}
......@@ -305,11 +333,8 @@ fetch_eof(struct sess *sp, struct http_conn *htc)
assert(sp->wrk->body_status == BS_EOF);
i = sp->wrk->vfp->bytes(sp, htc, SSIZE_MAX);
if (i < 0) {
WSP(sp, SLT_FetchError, "eof read_error: %d (%s)",
errno, htc->error);
if (i < 0)
return (-1);
}
return (0);
}
......@@ -498,6 +523,7 @@ FetchBody(struct sess *sp)
/* XXX: pick up estimate from objdr ? */
cl = 0;
w->fetch_failed = 0;
switch (w->body_status) {
case BS_NONE:
cls = 0;
......@@ -568,6 +594,7 @@ FetchBody(struct sess *sp)
sp->obj->len = 0;
return (__LINE__);
}
AZ(w->fetch_failed);
if (cls == 0 && w->do_close)
cls = 1;
......
......@@ -82,7 +82,7 @@ struct vgz {
const char *id;
struct ws *tmp;
char *tmp_snapshot;
const char *error;
int last_i;
struct storage *obuf;
......@@ -206,8 +206,6 @@ VGZ_NewGzip(struct sess *sp, 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);
}
......@@ -264,10 +262,8 @@ VGZ_ObufStorage(const struct sess *sp, struct vgz *vg)
struct storage *st;
st = FetchStorage(sp, 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);
......@@ -299,6 +295,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)
......@@ -341,6 +338,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)
......@@ -408,11 +406,11 @@ VGZ_UpdateObj(const struct vgz *vg, struct object *obj)
/*--------------------------------------------------------------------*/
void
int
VGZ_Destroy(struct vgz **vgp)
{
struct vgz *vg;
const char *err;
int i;
vg = *vgp;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
......@@ -425,14 +423,22 @@ VGZ_Destroy(struct vgz **vgp)
(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);
}
/*--------------------------------------------------------------------
......@@ -459,6 +465,7 @@ vfp_gunzip_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
size_t dl;
const void *dp;
AZ(sp->wrk->fetch_failed);
vg = sp->wrk->vgz_rx;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
AZ(vg->vz.avail_in);
......@@ -468,27 +475,25 @@ vfp_gunzip_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
if (l > bytes)
l = bytes;
w = HTC_Read(htc, ibuf, l);
if (w <= 0)
if (w < 0)
return(FetchError(sp, htc->error));
if (w == 0)
return (w);
VGZ_Ibuf(vg, ibuf, w);
bytes -= w;
}
if (VGZ_ObufStorage(sp, vg)) {
htc->error = "Could not get storage";
return (-1);
}
if (VGZ_ObufStorage(sp, vg))
return(FetchError(sp, "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(sp, "Gunzip data error"));
sp->obj->len += dl;
if (sp->wrk->do_stream)
RES_StreamPoll(sp);
}
if (i == Z_OK || i == Z_STREAM_END)
return (1);
htc->error = "See other message";
WSP(sp, SLT_FetchError, "Gunzip trouble (%d)", i);
return (-1);
assert(i == Z_OK || i == Z_STREAM_END);
return (1);
}
static int __match_proto__()
......@@ -499,7 +504,12 @@ vfp_gunzip_end(struct sess *sp)
vg = sp->wrk->vgz_rx;
sp->wrk->vgz_rx = NULL;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
VGZ_Destroy(&vg);
if (sp->wrk->fetch_failed) {
(void)VGZ_Destroy(&vg);
return(0);
}
if (VGZ_Destroy(&vg) != VGZ_END)
return(FetchError(sp, "Gunzip error at the very end"));
return (0);
}
......@@ -535,6 +545,7 @@ vfp_gzip_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
size_t dl;
const void *dp;
AZ(sp->wrk->fetch_failed);
vg = sp->wrk->vgz_rx;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
AZ(vg->vz.avail_in);
......@@ -544,15 +555,15 @@ vfp_gzip_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
if (l > bytes)
l = bytes;
w = HTC_Read(htc, ibuf, l);
if (w <= 0)
if (w < 0)
return(FetchError(sp, htc->error));
if (w == 0)
return (w);
VGZ_Ibuf(vg, ibuf, w);
bytes -= w;
}
if (VGZ_ObufStorage(sp, vg)) {
htc->error = "Could not get storage";
return (-1);
}
if (VGZ_ObufStorage(sp, vg))
return(FetchError(sp, "Could not get storage"));
i = VGZ_Gzip(vg, &dp, &dl, VGZ_NORMAL);
assert(i == Z_OK);
sp->obj->len += dl;
......@@ -574,19 +585,22 @@ vfp_gzip_end(struct sess *sp)
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
sp->wrk->vgz_rx = NULL;
if (vg->error == NULL) {
do {
VGZ_Ibuf(vg, "", 0);
if (VGZ_ObufStorage(sp, vg))
return (-1);
i = VGZ_Gzip(vg, &dp, &dl, VGZ_FINISH);
sp->obj->len += dl;
} while (i != Z_STREAM_END);
if (sp->wrk->do_stream)
RES_StreamPoll(sp);
VGZ_UpdateObj(vg, sp->obj);
if (sp->wrk->fetch_failed) {
(void)VGZ_Destroy(&vg);
return(0);
}
VGZ_Destroy(&vg);
do {
VGZ_Ibuf(vg, "", 0);
if (VGZ_ObufStorage(sp, vg))
return(FetchError(sp, "Could not get storage"));
i = VGZ_Gzip(vg, &dp, &dl, VGZ_FINISH);
sp->obj->len += dl;
} while (i != Z_STREAM_END);
if (sp->wrk->do_stream)
RES_StreamPoll(sp);
VGZ_UpdateObj(vg, sp->obj);
if (VGZ_Destroy(&vg) != VGZ_END)
return(FetchError(sp, "Gzip error at the very end"));
return (0);
}
......@@ -622,21 +636,21 @@ vfp_testgzip_bytes(struct sess *sp, struct http_conn *htc, ssize_t bytes)
const void *dp;
struct storage *st;
AZ(sp->wrk->fetch_failed);
vg = sp->wrk->vgz_rx;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
AZ(vg->vz.avail_in);
while (bytes > 0) {
st = FetchStorage(sp, 0);
if (st == NULL) {
htc->error = "Could not get storage";
vg->error = htc->error;
return (-1);
}
if (st == NULL)
return(FetchError(sp, "Could not get storage"));
l = st->space - st->len;
if (l > bytes)
l = bytes;
w = HTC_Read(htc, st->ptr + st->len, l);
if (w <= 0)
if (w < 0)
return(FetchError(sp, htc->error));
if (w == 0)
return (w);
bytes -= w;
VGZ_Ibuf(vg, st->ptr + st->len, w);
......@@ -648,23 +662,16 @@ vfp_testgzip_bytes(struct sess *sp, 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";
WSP(sp, SLT_FetchError,
"Invalid Gzip data: %s", vg->vz.msg);
return (-1);
}
if (i == VGZ_END && !VGZ_IbufEmpty(vg))
return(FetchError(sp, "Junk after gzip data"));
if (i != VGZ_OK && i != VGZ_END)
return(FetchError2(sp,
"Invalid Gzip data", vg->vz.msg));
}
}
if (i == VGZ_OK || i == VGZ_END)
return (1);
htc->error = "See other message";
WSP(sp, SLT_FetchError, "Gunzip trouble (%d)", i);
return (-1);
assert(i == VGZ_OK || i == VGZ_END);
return (1);
}
static int __match_proto__()
......@@ -675,8 +682,13 @@ vfp_testgzip_end(struct sess *sp)
vg = sp->wrk->vgz_rx;
sp->wrk->vgz_rx = NULL;
CHECK_OBJ_NOTNULL(vg, VGZ_MAGIC);
if (sp->wrk->fetch_failed) {
(void)VGZ_Destroy(&vg);
return(0);
}
VGZ_UpdateObj(vg, sp->obj);
VGZ_Destroy(&vg);
if (VGZ_Destroy(&vg) != VGZ_END)
return(FetchError(sp, "TestGunzip error at the very end"));
return (0);
}
......
......@@ -187,7 +187,7 @@ res_WriteGunzipObj(struct sess *sp)
(void)WRW_Write(sp->wrk, obuf, obufl);
(void)WRW_Flush(sp->wrk);
}
VGZ_Destroy(&vg);
(void)VGZ_Destroy(&vg);
assert(u == sp->obj->len);
}
......
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
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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment