[master] 01dfd01 More cleanup and simplification of FetchError reporting.
Poul-Henning Kamp
phk at varnish-cache.org
Mon Oct 31 22:38:19 CET 2011
commit 01dfd019aafca831af9adabc0bcc17a03097a40d
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Oct 31 21:37:47 2011 +0000
More cleanup and simplification of FetchError reporting.
diff --git a/bin/varnishd/cache.h b/bin/varnishd/cache.h
index 53e8ae8..1875ece 100644
--- a/bin/varnishd/cache.h
+++ b/bin/varnishd/cache.h
@@ -199,7 +199,6 @@ struct http_conn {
struct ws *ws;
txt rxbuf;
txt pipeline;
- const char *error;
};
/*--------------------------------------------------------------------*/
@@ -785,7 +784,7 @@ void HTC_Init(struct http_conn *htc, struct ws *ws, int fd, unsigned vsl_id,
unsigned maxbytes, unsigned maxhdr);
int HTC_Reinit(struct http_conn *htc);
int HTC_Rx(struct http_conn *htc);
-ssize_t HTC_Read(struct http_conn *htc, void *d, size_t len);
+ssize_t HTC_Read(struct worker *w, struct http_conn *htc, void *d, size_t len);
int HTC_Complete(struct http_conn *htc);
#define HTTPH(a, b, c, d, e, f, g) extern char b[];
diff --git a/bin/varnishd/cache_esi_fetch.c b/bin/varnishd/cache_esi_fetch.c
index 1c75054..728436c 100644
--- a/bin/varnishd/cache_esi_fetch.c
+++ b/bin/varnishd/cache_esi_fetch.c
@@ -44,7 +44,8 @@
*/
static ssize_t
-vef_read(struct http_conn *htc, void *buf, ssize_t buflen, ssize_t bytes)
+vef_read(struct worker *w, struct http_conn *htc, void *buf, ssize_t buflen,
+ ssize_t bytes)
{
ssize_t d;
@@ -55,7 +56,7 @@ vef_read(struct http_conn *htc, void *buf, ssize_t buflen, ssize_t bytes)
if (d < bytes)
bytes = d;
}
- return (HTC_Read(htc, buf, bytes));
+ return (HTC_Read(w, htc, buf, bytes));
}
/*---------------------------------------------------------------------
@@ -74,7 +75,7 @@ vfp_esi_bytes_uu(struct worker *w, struct http_conn *htc, ssize_t bytes)
st = FetchStorage(w, 0);
if (st == NULL)
return (-1);
- wl = vef_read(htc,
+ wl = vef_read(w, htc,
st->ptr + st->len, st->space - st->len, bytes);
if (wl <= 0)
return (wl);
@@ -105,14 +106,14 @@ vfp_esi_bytes_gu(struct worker *w, struct http_conn *htc, ssize_t bytes)
while (bytes > 0) {
if (VGZ_IbufEmpty(vg) && bytes > 0) {
- wl = vef_read(htc, ibuf, sizeof ibuf, bytes);
+ wl = vef_read(w, htc, ibuf, sizeof ibuf, bytes);
if (wl <= 0)
return (wl);
VGZ_Ibuf(vg, ibuf, wl);
bytes -= wl;
}
if (VGZ_ObufStorage(w, vg))
- return(FetchError(w, "Could not get storage"));
+ return(-1);
i = VGZ_Gunzip(vg, &dp, &dl);
xxxassert(i == VGZ_OK || i == VGZ_END);
VEP_Parse(w, dp, dl);
@@ -180,7 +181,7 @@ vfp_vep_callback(struct worker *w, ssize_t l, enum vgz_flag flg)
}
do {
if (VGZ_ObufStorage(w, vef->vgz)) {
- vef->error = errno;
+ vef->error = ENOMEM;
vef->tot += l;
return (vef->tot);
}
@@ -203,7 +204,7 @@ vfp_vep_callback(struct worker *w, ssize_t l, enum vgz_flag flg)
}
static int
-vfp_esi_bytes_ug(const struct worker *w, struct http_conn *htc, ssize_t bytes)
+vfp_esi_bytes_ug(struct worker *w, struct http_conn *htc, ssize_t bytes)
{
ssize_t wl;
char ibuf[params->gzip_stack_buffer];
@@ -214,7 +215,7 @@ vfp_esi_bytes_ug(const struct worker *w, struct http_conn *htc, ssize_t bytes)
CHECK_OBJ_NOTNULL(vef, VEF_MAGIC);
while (bytes > 0) {
- wl = vef_read(htc, ibuf, sizeof ibuf, bytes);
+ wl = vef_read(w, htc, ibuf, sizeof ibuf, bytes);
if (wl <= 0)
return (wl);
bytes -= wl;
@@ -240,7 +241,7 @@ vfp_esi_bytes_ug(const struct worker *w, struct http_conn *htc, ssize_t bytes)
*/
static int
-vfp_esi_bytes_gg(const struct worker *w, struct http_conn *htc, size_t bytes)
+vfp_esi_bytes_gg(struct worker *w, struct http_conn *htc, size_t bytes)
{
ssize_t wl;
char ibuf[params->gzip_stack_buffer];
@@ -257,7 +258,7 @@ vfp_esi_bytes_gg(const struct worker *w, struct http_conn *htc, size_t bytes)
ibuf2[0] = 0; /* For Flexelint */
while (bytes > 0) {
- wl = vef_read(htc, ibuf, sizeof ibuf, bytes);
+ wl = vef_read(w, htc, ibuf, sizeof ibuf, bytes);
if (wl <= 0)
return (wl);
bytes -= wl;
@@ -334,6 +335,7 @@ vfp_esi_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
AZ(w->fetch_failed);
AN(w->vep);
+ assert(w->htc == htc);
if (w->is_gzip && w->do_gunzip)
i = vfp_esi_bytes_gu(w, htc, bytes);
else if (w->is_gunzip && w->do_gzip)
diff --git a/bin/varnishd/cache_fetch.c b/bin/varnishd/cache_fetch.c
index dd3a1c7..fb2ce41 100644
--- a/bin/varnishd/cache_fetch.c
+++ b/bin/varnishd/cache_fetch.c
@@ -120,14 +120,12 @@ vfp_nop_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
while (bytes > 0) {
st = FetchStorage(w, 0);
if (st == NULL)
- return(FetchError(w, "Could not get storage"));
+ return(-1);
l = st->space - st->len;
if (l > bytes)
l = bytes;
- wl = HTC_Read(htc, st->ptr + st->len, l);
- if (wl < 0)
- return(FetchError(w, htc->error));
- if (wl == 0)
+ wl = HTC_Read(w, htc, st->ptr + st->len, l);
+ if (wl <= 0)
return (wl);
st->len += wl;
w->fetch_obj->len += wl;
@@ -173,7 +171,8 @@ static struct vfp vfp_nop = {
};
/*--------------------------------------------------------------------
- * Fetch Storage
+ * Fetch Storage to put object into.
+ *
*/
struct storage *
@@ -196,7 +195,7 @@ FetchStorage(struct worker *w, ssize_t sz)
l = params->fetch_chunksize * 1024LL;
st = STV_alloc(w, l);
if (st == NULL) {
- errno = ENOMEM;
+ (void)FetchError(w, "Could not get storage");
return (NULL);
}
AZ(st->len);
@@ -247,17 +246,11 @@ fetch_straight(struct worker *w, struct http_conn *htc, ssize_t cl)
return (0);
}
-/*--------------------------------------------------------------------*/
-/* XXX: Cleanup. It must be possible somehow :-( */
-
-#define CERR() do { \
- if (i != 1) { \
- WSLB(w, SLT_FetchError, \
- "chunked read_error: %d (%s)", \
- errno, htc->error); \
- return (-1); \
- } \
- } while (0)
+/*--------------------------------------------------------------------
+ * Read a chunked HTTP object.
+ * XXX: Reading one byte at a time is pretty pessimal.
+ */
+
static int
fetch_chunked(struct worker *w, struct http_conn *htc)
@@ -271,15 +264,17 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
do {
/* Skip leading whitespace */
do {
- i = HTC_Read(htc, buf, 1);
- CERR();
+ i = HTC_Read(w, htc, buf, 1);
+ if (i <= 0)
+ return (i);
} while (vct_islws(buf[0]));
/* Collect hex digits, skipping leading zeros */
for (u = 1; u < sizeof buf; u++) {
do {
- i = HTC_Read(htc, buf + u, 1);
- CERR();
+ i = HTC_Read(w, htc, buf + u, 1);
+ if (i <= 0)
+ return (i);
} while (u == 1 && buf[0] == '0' && buf[u] == '0');
if (!vct_ishex(buf[u]))
break;
@@ -290,8 +285,9 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
/* Skip trailing white space */
while(vct_islws(buf[u]) && buf[u] != '\n') {
- i = HTC_Read(htc, buf + u, 1);
- CERR();
+ i = HTC_Read(w, htc, buf + u, 1);
+ if (i <= 0)
+ return (i);
}
if (buf[u] != '\n')
@@ -306,11 +302,13 @@ fetch_chunked(struct worker *w, struct http_conn *htc)
if (i <= 0)
return (-1);
}
- i = HTC_Read(htc, buf, 1);
- CERR();
+ i = HTC_Read(w, htc, buf, 1);
+ if (i <= 0)
+ return (-1);
if (buf[0] == '\r') {
- i = HTC_Read(htc, buf, 1);
- CERR();
+ i = HTC_Read(w, htc, buf, 1);
+ if (i <= 0)
+ return (-1);
}
if (buf[0] != '\n')
return (FetchError(w,"chunked tail no NL"));
@@ -358,7 +356,7 @@ FetchReqBody(struct sess *sp)
rdcnt = sizeof buf;
else
rdcnt = content_length;
- rdcnt = HTC_Read(sp->htc, buf, rdcnt);
+ rdcnt = HTC_Read(sp->wrk, sp->htc, buf, rdcnt);
if (rdcnt <= 0)
return (1);
content_length -= rdcnt;
@@ -459,7 +457,7 @@ FetchHdr(struct sess *sp)
if (i < 0) {
WSP(sp, SLT_FetchError, "http first read error: %d %d (%s)",
- i, errno, w->htc->error);
+ i, errno, strerror(errno));
VDI_CloseFd(sp->wrk);
/* XXX: other cleanup ? */
/* Retryable if we never received anything */
@@ -473,7 +471,7 @@ FetchHdr(struct sess *sp)
if (i < 0) {
WSP(sp, SLT_FetchError,
"http first read error: %d %d (%s)",
- i, errno, w->htc->error);
+ i, errno, strerror(errno));
VDI_CloseFd(sp->wrk);
/* XXX: other cleanup ? */
return (-1);
diff --git a/bin/varnishd/cache_gzip.c b/bin/varnishd/cache_gzip.c
index 73f4bb9..1820ffc 100644
--- a/bin/varnishd/cache_gzip.c
+++ b/bin/varnishd/cache_gzip.c
@@ -480,17 +480,15 @@ vfp_gunzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
l = sizeof ibuf;
if (l > bytes)
l = bytes;
- wl = HTC_Read(htc, ibuf, l);
- if (wl < 0)
- return(FetchError(w, htc->error));
- if (wl == 0)
+ wl = HTC_Read(w, htc, ibuf, l);
+ if (wl <= 0)
return (wl);
VGZ_Ibuf(vg, ibuf, wl);
bytes -= wl;
}
if (VGZ_ObufStorage(w, vg))
- return(FetchError(w, "Could not get storage"));
+ return(-1);
i = VGZ_Gunzip(vg, &dp, &dl);
if (i != VGZ_OK && i != VGZ_END)
return(FetchError(w, "Gunzip data error"));
@@ -560,16 +558,14 @@ vfp_gzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
l = sizeof ibuf;
if (l > bytes)
l = bytes;
- wl = HTC_Read(htc, ibuf, l);
- if (wl < 0)
- return(FetchError(w, htc->error));
- if (wl == 0)
+ wl = HTC_Read(w, htc, ibuf, l);
+ if (wl <= 0)
return (wl);
VGZ_Ibuf(vg, ibuf, wl);
bytes -= wl;
}
if (VGZ_ObufStorage(w, vg))
- return(FetchError(w, "Could not get storage"));
+ return(-1);
i = VGZ_Gzip(vg, &dp, &dl, VGZ_NORMAL);
assert(i == Z_OK);
w->fetch_obj->len += dl;
@@ -597,7 +593,7 @@ vfp_gzip_end(struct worker *w)
do {
VGZ_Ibuf(vg, "", 0);
if (VGZ_ObufStorage(w, vg))
- return(FetchError(w, "Could not get storage"));
+ return(-1);
i = VGZ_Gzip(vg, &dp, &dl, VGZ_FINISH);
w->fetch_obj->len += dl;
} while (i != Z_STREAM_END);
@@ -648,14 +644,12 @@ vfp_testgzip_bytes(struct worker *w, struct http_conn *htc, ssize_t bytes)
while (bytes > 0) {
st = FetchStorage(w, 0);
if (st == NULL)
- return(FetchError(w, "Could not get storage"));
+ return(-1);
l = st->space - st->len;
if (l > bytes)
l = bytes;
- wl = HTC_Read(htc, st->ptr + st->len, l);
- if (wl < 0)
- return(FetchError(w, htc->error));
- if (wl == 0)
+ wl = HTC_Read(w, htc, st->ptr + st->len, l);
+ if (wl <= 0)
return (wl);
bytes -= wl;
VGZ_Ibuf(vg, st->ptr + st->len, wl);
diff --git a/bin/varnishd/cache_httpconn.c b/bin/varnishd/cache_httpconn.c
index ba881c9..1b718e5 100644
--- a/bin/varnishd/cache_httpconn.c
+++ b/bin/varnishd/cache_httpconn.c
@@ -27,6 +27,17 @@
* SUCH DAMAGE.
*
* HTTP protocol requests
+ *
+ * The trouble with the "until magic sequence" design of HTTP protocol messages
+ * is that either you have to read a single character at a time, which is
+ * inefficient, or you risk reading too much, and pre-read some of the object,
+ * or even the next pipelined request, which follows the one you want.
+ *
+ * HTC reads a HTTP protocol header into a workspace, subject to limits,
+ * and stops when we see the magic marker (double [CR]NL), and if we overshoot,
+ * it keeps track of the "pipelined" data.
+ *
+ * We use this both for client and backend connections.
*/
#include "config.h"
@@ -53,7 +64,8 @@ htc_header_complete(txt *t)
/* Skip any leading white space */
for (p = t->b ; vct_issp(*p); p++)
continue;
- if (*p == '\0') {
+ if (p == t->e) {
+ /* All white space */
t->e = t->b;
*t->e = '\0';
return (0);
@@ -85,7 +97,6 @@ HTC_Init(struct http_conn *htc, struct ws *ws, int fd, unsigned vsl_id,
htc->vsl_id = vsl_id;
htc->maxbytes = maxbytes;
htc->maxhdr = maxhdr;
- htc->error = "No error recorded";
(void)WS_Reserve(htc->ws, htc->maxbytes);
htc->rxbuf.b = ws->f;
@@ -107,7 +118,6 @@ HTC_Reinit(struct http_conn *htc)
unsigned l;
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
- htc->error = "No error recorded";
(void)WS_Reserve(htc->ws, htc->maxbytes);
htc->rxbuf.b = htc->ws->f;
htc->rxbuf.e = htc->ws->f;
@@ -123,7 +133,7 @@ HTC_Reinit(struct http_conn *htc)
}
/*--------------------------------------------------------------------
- *
+ * Return 1 if we have a complete HTTP procol header
*/
int
@@ -137,6 +147,8 @@ HTC_Complete(struct http_conn *htc)
if (i == 0)
return (0);
WS_ReleaseP(htc->ws, htc->rxbuf.e);
+ AZ(htc->pipeline.b);
+ AZ(htc->pipeline.e);
if (htc->rxbuf.b + i < htc->rxbuf.e) {
htc->pipeline.b = htc->rxbuf.b + i;
htc->pipeline.e = htc->rxbuf.e;
@@ -168,6 +180,10 @@ HTC_Rx(struct http_conn *htc)
}
i = read(htc->fd, htc->rxbuf.e, i);
if (i <= 0) {
+ /*
+ * We wouldn't come here if we had a complete HTTP header
+ * so consequently an EOF can not be OK
+ */
WS_ReleaseP(htc->ws, htc->rxbuf.b);
return (-1);
}
@@ -176,16 +192,21 @@ HTC_Rx(struct http_conn *htc)
return (HTC_Complete(htc));
}
+/*--------------------------------------------------------------------
+ * Read up to len bytes, returning pipelined data first.
+ */
+
ssize_t
-HTC_Read(struct http_conn *htc, void *d, size_t len)
+HTC_Read(struct worker *w, struct http_conn *htc, void *d, size_t len)
{
size_t l;
unsigned char *p;
ssize_t i;
+ CHECK_OBJ_NOTNULL(w, WORKER_MAGIC);
+ CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
l = 0;
p = d;
- CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
if (htc->pipeline.b) {
l = Tlen(htc->pipeline);
if (l > len)
@@ -201,9 +222,8 @@ HTC_Read(struct http_conn *htc, void *d, size_t len)
return (l);
i = read(htc->fd, p, len);
if (i < 0) {
- htc->error = strerror(errno);
+ WSL(w, SLT_FetchError, htc->vsl_id, "%s", strerror(errno));
return (i);
- } else if (i == 0)
- htc->error = "Remote closed connection";
+ }
return (i + l);
}
More information about the varnish-commit
mailing list