[4.0] b4b628f Implement proper handling of chunked encoding of req.body.

Poul-Henning Kamp phk at FreeBSD.org
Tue Jun 24 11:31:57 CEST 2014


commit b4b628fa01591d50b2ce09e570ecd5470477cc65
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Jun 23 09:53:21 2014 +0000

    Implement proper handling of chunked encoding of req.body.
    
    Fixes	#1524

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 2b10f59..dc7e6f1 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -236,7 +236,7 @@ struct acct_req {
 /*--------------------------------------------------------------------*/
 
 struct acct_bereq {
-#define ACCT(foo)	ssize_t		foo;
+#define ACCT(foo)	uint64_t	foo;
 #include "tbl/acct_fields_bereq.h"
 #undef ACCT
 };
@@ -665,6 +665,7 @@ struct req {
 
 	unsigned char		wantbody;
 	uint64_t		req_bodybytes;	/* Parsed req bodybytes */
+	intptr_t		chunk_ctr;	/* Parsed req bodybytes */
 
 	uint64_t		resp_hdrbytes;	/* Scheduled resp hdrbytes */
 	uint64_t		resp_bodybytes; /* Scheduled resp bodybytes */
@@ -866,7 +867,7 @@ enum http1_chunked_ret {
 };
 enum http1_chunked_ret
 HTTP1_Chunked(struct http_conn *htc, intptr_t *priv, const char **error,
-    int64_t *statp, void *ptr, ssize_t *lp);
+    uint64_t *statp, void *ptr, ssize_t *lp);
 
 /* cache_http1_deliver.c */
 unsigned V1D_FlushReleaseAcct(struct req *req);
@@ -1123,7 +1124,7 @@ void WRW_Chunked(const struct worker *w);
 void WRW_EndChunk(const struct worker *w);
 void WRW_Reserve(struct worker *w, int *fd, struct vsl_log *, double t0);
 unsigned WRW_Flush(const struct worker *w);
-unsigned WRW_FlushRelease(struct worker *w, ssize_t *pacc);
+unsigned WRW_FlushRelease(struct worker *w, uint64_t *pacc);
 unsigned WRW_Write(const struct worker *w, const void *ptr, int len);
 unsigned WRW_WriteH(const struct worker *w, const txt *hh, const char *suf);
 
diff --git a/bin/varnishd/cache/cache_http1_deliver.c b/bin/varnishd/cache/cache_http1_deliver.c
index 8778303..1bdcef7 100644
--- a/bin/varnishd/cache/cache_http1_deliver.c
+++ b/bin/varnishd/cache/cache_http1_deliver.c
@@ -211,7 +211,7 @@ unsigned
 V1D_FlushReleaseAcct(struct req *req)
 {
 	unsigned u;
-	ssize_t txcnt = 0, hdrbytes;
+	uint64_t txcnt = 0, hdrbytes;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	CHECK_OBJ_NOTNULL(req->wrk, WORKER_MAGIC);
diff --git a/bin/varnishd/cache/cache_http1_fsm.c b/bin/varnishd/cache/cache_http1_fsm.c
index 9191d1c..dac34e0 100644
--- a/bin/varnishd/cache/cache_http1_fsm.c
+++ b/bin/varnishd/cache/cache_http1_fsm.c
@@ -285,6 +285,8 @@ http1_req_body_status(struct req *req)
 		req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
 		return (REQ_BODY_PRESENT);
 	}
+	if (http_HdrIs(req->http, H_Transfer_Encoding, "chunked"))
+		return (REQ_BODY_CHUNKED);
 	if (http_GetHdr(req->http, H_Transfer_Encoding, NULL))
 		return (REQ_BODY_FAIL);
 	return (REQ_BODY_NONE);
@@ -480,28 +482,46 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 }
 
 static ssize_t
-http1_iter_req_body(struct req *req, void *buf, ssize_t len)
+http1_iter_req_body(struct req *req, enum req_body_state_e bs,
+    void *buf, ssize_t len)
 {
+	const char *err;
+
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
-	AN(req->req_bodybytes);
 	AN(len);
 	AN(buf);
-	if (len > req->req_bodybytes - req->h1.bytes_done)
-		len = req->req_bodybytes - req->h1.bytes_done;
-	if (len == 0) {
-		req->req_body_status = REQ_BODY_DONE;
-		return (0);
-	}
-	len = HTTP1_Read(req->htc, buf, len);
-	if (len <= 0) {
-		req->req_body_status = REQ_BODY_FAIL;
-		return (-1);
-	}
-	req->h1.bytes_done += len;
-	req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
-	req->acct.req_bodybytes += len;
-	return (len);
+	if (bs == REQ_BODY_PRESENT) {
+		AN(req->req_bodybytes);
+		if (len > req->req_bodybytes - req->h1.bytes_done)
+			len = req->req_bodybytes - req->h1.bytes_done;
+		if (len == 0) {
+			req->req_body_status = REQ_BODY_DONE;
+			return (0);
+		}
+		len = HTTP1_Read(req->htc, buf, len);
+		if (len <= 0) {
+			req->req_body_status = REQ_BODY_FAIL;
+			return (-1);
+		}
+		req->h1.bytes_done += len;
+		req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
+		req->acct.req_bodybytes += len;
+		return (len);
+	} else if (bs == REQ_BODY_CHUNKED) {
+		switch (HTTP1_Chunked(req->htc, &req->chunk_ctr, &err,
+		    &req->acct.req_bodybytes, buf, &len)) {
+		case H1CR_ERROR:
+			return (-1);
+		case H1CR_MORE:
+			return (len);
+		case H1CR_END:
+			return (0);
+		default:
+			WRONG("invalid HTTP1_Chunked return");
+		}
+	} else
+		WRONG("Illegal req_bodystatus");
 }
 
 /*----------------------------------------------------------------------
@@ -516,6 +536,7 @@ HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
 {
 	char buf[8192];
 	struct storage *st;
+	enum req_body_state_e bs;
 	ssize_t l;
 	int i;
 
@@ -533,6 +554,7 @@ HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
 	case REQ_BODY_NONE:
 		return (0);
 	case REQ_BODY_PRESENT:
+	case REQ_BODY_CHUNKED:
 		break;
 	case REQ_BODY_DONE:
 	case REQ_BODY_TAKEN:
@@ -543,7 +565,9 @@ HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
 		WRONG("Wrong req_body_status in HTTP1_IterateReqBody()");
 	}
 	Lck_Lock(&req->sp->mtx);
-	if (req->req_body_status == REQ_BODY_PRESENT) {
+	bs = req->req_body_status;
+	if (req->req_body_status == REQ_BODY_PRESENT ||
+	    req->req_body_status == REQ_BODY_CHUNKED) {
 		req->req_body_status = REQ_BODY_TAKEN;
 		i = 0;
 	} else
@@ -556,7 +580,7 @@ HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
 	}
 
 	do {
-		l = http1_iter_req_body(req, buf, sizeof buf);
+		l = http1_iter_req_body(req, bs, buf, sizeof buf);
 		if (l < 0) {
 			req->doclose = SC_RX_BODY;
 			break;
@@ -626,6 +650,7 @@ HTTP1_CacheReqBody(struct req *req, ssize_t maxsize)
 		return (-1);
 	case REQ_BODY_NONE:
 		return (0);
+	case REQ_BODY_CHUNKED:
 	case REQ_BODY_PRESENT:
 		break;
 	default:
@@ -653,7 +678,8 @@ HTTP1_CacheReqBody(struct req *req, ssize_t maxsize)
 		}
 
 		l = st->space - st->len;
-		l = http1_iter_req_body(req, st->ptr + st->len, l);
+		l = http1_iter_req_body(req, req->req_body_status,
+		    st->ptr + st->len, l);
 		if (l < 0) {
 			req->doclose = SC_RX_BODY;
 			break;
diff --git a/bin/varnishd/cache/cache_http1_proto.c b/bin/varnishd/cache/cache_http1_proto.c
index 0dc5fe8..a3e2005 100644
--- a/bin/varnishd/cache/cache_http1_proto.c
+++ b/bin/varnishd/cache/cache_http1_proto.c
@@ -535,7 +535,7 @@ HTTP1_Write(const struct worker *w, const struct http *hp, const int *hf)
 
 enum http1_chunked_ret
 HTTP1_Chunked(struct http_conn *htc, intptr_t *priv, const char **error,
-    int64_t *statp, void *ptr, ssize_t *lp)
+    uint64_t *statp, void *ptr, ssize_t *lp)
 {
 	int i;
 	char buf[20];		/* XXX: 20 is arbitrary */
diff --git a/bin/varnishd/cache/cache_pipe.c b/bin/varnishd/cache/cache_pipe.c
index 27ea964..36ef425 100644
--- a/bin/varnishd/cache/cache_pipe.c
+++ b/bin/varnishd/cache/cache_pipe.c
@@ -43,14 +43,14 @@
 static struct lock pipestat_mtx;
 
 struct acct_pipe {
-	ssize_t		req;
-	ssize_t		bereq;
-	ssize_t		in;
-	ssize_t		out;
+	uint64_t	req;
+	uint64_t	bereq;
+	uint64_t	in;
+	uint64_t	out;
 };
 
 static int
-rdf(int fd0, int fd1, ssize_t *pcnt)
+rdf(int fd0, int fd1, uint64_t *pcnt)
 {
 	int i, j;
 	char buf[BUFSIZ], *p;
diff --git a/bin/varnishd/cache/cache_wrw.c b/bin/varnishd/cache/cache_wrw.c
index 09c7e26..cc02bbf 100644
--- a/bin/varnishd/cache/cache_wrw.c
+++ b/bin/varnishd/cache/cache_wrw.c
@@ -102,7 +102,7 @@ WRW_Reserve(struct worker *wrk, int *fd, struct vsl_log *vsl, double t0)
 }
 
 static void
-wrw_release(struct worker *wrk, ssize_t *pacc)
+wrw_release(struct worker *wrk, uint64_t *pacc)
 {
 	struct wrw *wrw;
 
@@ -219,7 +219,7 @@ WRW_Flush(const struct worker *wrk)
 }
 
 unsigned
-WRW_FlushRelease(struct worker *wrk, ssize_t *pacc)
+WRW_FlushRelease(struct worker *wrk, uint64_t *pacc)
 {
 	unsigned u;
 
diff --git a/bin/varnishtest/tests/c00067.vtc b/bin/varnishtest/tests/c00067.vtc
new file mode 100644
index 0000000..b57f904
--- /dev/null
+++ b/bin/varnishtest/tests/c00067.vtc
@@ -0,0 +1,24 @@
+varnishtest "chunked req.body"
+
+server s1 {
+	rxreq
+	expect req.bodylen == 106
+	txresp -body "ABCD"
+} -start
+
+varnish v1 -vcl+backend {
+} -start
+
+client c1 {
+	txreq -req POST -nolen -hdr "Transfer-encoding: chunked"
+	chunked {BLA}
+	delay .2
+	chunkedlen 100
+	delay .2
+	chunked {FOO}
+	delay .2
+	chunkedlen 0
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 4
+} -run
diff --git a/bin/varnishtest/tests/r01524.vtc b/bin/varnishtest/tests/r01524.vtc
new file mode 100644
index 0000000..a845da3
--- /dev/null
+++ b/bin/varnishtest/tests/r01524.vtc
@@ -0,0 +1,19 @@
+varnishtest "pipe of chunked request"
+
+server s1 {
+	rxreq
+	expect req.bodylen == 3
+	txresp -body "ABCD"
+} -start
+
+varnish v1 -vcl+backend {
+} -start
+
+client c1 {
+	txreq -req MAGIC -nolen -hdr "Transfer-encoding: chunked"
+	chunked {BLA}
+	chunkedlen 0
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 4
+} -run



More information about the varnish-commit mailing list