From 96f4a5a6785f8f6e6452f1132b7f5231153d4a4c Mon Sep 17 00:00:00 2001
From: phk <phk@d4fa192b-c00b-0410-8231-f00ffab90ce4>
Date: Wed, 6 Sep 2006 07:37:34 +0000
Subject: [PATCH] Fix a bug in chunked fetching:

With very short chunks, in this case 3 characters, our buffer may
contain not only the chunk length and the chunk data, but also the
next chunk length.

If the short chunk is the last chunk before the zero length chunk
at the end, unconditionally trying to fill the buffer before parsing
the length may hang because we already have everything there is to
have in the buffer.

The fix is to always try to parse the buffer before adding to it.

While here, tighten up and improve error checks of the code.

Reported by: Xing Li <xing@litespeedtech.com>



git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@917 d4fa192b-c00b-0410-8231-f00ffab90ce4
---
 varnish-cache/bin/varnishd/cache_fetch.c | 94 +++++++++++++-----------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/varnish-cache/bin/varnishd/cache_fetch.c b/varnish-cache/bin/varnishd/cache_fetch.c
index 08a5fa63..378254de 100644
--- a/varnish-cache/bin/varnishd/cache_fetch.c
+++ b/varnish-cache/bin/varnishd/cache_fetch.c
@@ -67,86 +67,94 @@ fetch_chunked(const struct sess *sp, int fd, struct http *hp)
 {
 	int i;
 	char *q;
-	unsigned char *p;
 	struct storage *st;
 	unsigned u, v;
-	char buf[20];
+	char buf[20];		/* XXX: arbitrary */
 	char *bp, *be;
 
-	i = fcntl(fd, F_GETFL);		/* XXX ? */
-	i &= ~O_NONBLOCK;
-	i = fcntl(fd, F_SETFL, i);
-
-	be = buf + sizeof buf;
+	be = buf + sizeof buf - 1;
 	bp = buf;
 	st = NULL;
+	u = 0;
 	while (1) {
-		i = http_Read(hp, fd, bp, be - bp);
-		xxxassert(i >= 0);
-		bp += i;
+		/* Try to parse buf as a chunk length */
 		*bp = '\0';
 		u = strtoul(buf, &q, 16);
-		if (q == NULL || q == buf)
+
+		/* Skip trailing whitespace */
+		if (q != NULL && q > buf) {
+			while (*q == '\t' || *q == ' ')
+				q++;
+			if (*q == '\r')
+				q++;
+		}
+
+		/* If we didn't succeed, add to buffer, try again */
+		if (q == NULL || q == buf || *q != '\n') {
+			xxxassert(be > bp);
+			i = http_Read(hp, fd, bp, be - bp);
+			xxxassert(i >= 0);
+			bp += i;
 			continue;
-		xxxassert(isspace(*q));
-		while (*q == '\t' || *q == ' ')
-			q++;
-		if (*q == '\r')
-			q++;
-		xxxassert(*q == '\n');
+		}
+
+		/* Skip NL */
 		q++;
+
+		/* Last chunk is zero bytes long */
 		if (u == 0)
 			break;
-		sp->obj->len += u;
 
 		while (u > 0) {
-			if (st != NULL && st->len < st->space) {
-				p = st->ptr + st->len;
-			} else {
-				st = stevedore->alloc(stevedore,
-				    stevedore->trim == NULL ? u : CHUNK_PREALLOC);
+
+			/* Get some storage if we don't have any */
+			if (st == NULL || st->len == st->space) {
+				v = u;
+				if (u < CHUNK_PREALLOC && 
+				    stevedore->trim != NULL)
+					v = CHUNK_PREALLOC;
+				st = stevedore->alloc(stevedore, v);
 				XXXAN(st->stevedore);
 				TAILQ_INSERT_TAIL(&sp->obj->store, st, list);
-				p = st->ptr;
 			}
 			v = st->space - st->len;
 			if (v > u)
 				v = u;
 
+			/* Handle anything left in our buffer first */
 			i = bp - q;
 			assert(i >= 0);
-			if (i == 0) {
-			} else if (v > i) {
-				memcpy(p, q, i);
-				p += i;
+			if (i > v)
+				i = v;
+			if (i != 0) {
+				memcpy(st->ptr + st->len, q, i);
 				st->len += i;
+				sp->obj->len += i;
 				u -= i;
 				v -= i;
-				q = bp = buf;
-			} else if (i >= v) {
-				memcpy(p, q, v);
-				p += v;
-				st->len += v;
-				q += v;
-				u -= v;
-				v -= v;
-				if (u == 0 && bp > q) {
-					memcpy(buf, q, bp - q);
-					q = bp = buf + (bp - q);
-				}
+				q += i;
 			}
 			if (u == 0)
 				break;
 			if (v == 0)
 				continue;
+
+			/* Pick up the rest of this chunk */
 			while (v > 0) {
-				i = http_Read(hp, fd, p, v);
+				i = http_Read(hp, fd, st->ptr + st->len, v);
 				st->len += i;
-				v -= i;
+				sp->obj->len += i;
 				u -= i;
-				p += i;
+				v -= i;
 			}
 		}
+		assert(u == 0);
+
+		/* We might still have stuff in our buffer */
+		v = bp - q;
+		if (v > 0)
+			memcpy(buf, q, v);
+		q = bp = buf + v;
 	}
 
 	if (st != NULL && stevedore->trim != NULL)
-- 
2.39.5