Commit 6e4e013f authored by Poul-Henning Kamp's avatar Poul-Henning Kamp

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
parent 2fceda5d
......@@ -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);
......
......@@ -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));
......
......@@ -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 */
......
......@@ -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 = {
......
......@@ -42,6 +42,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
*
......@@ -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);
}
......
......@@ -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);
}
......
......@@ -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);
}
......
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