Revamp the req.body handling code.

There now is an undocumented facility for buffering the req.body,
but I'm not done testing it, and in particular the error handling
has not been exercised to any extent.

bin/varnishd/cache/cache.h

@@ -77,6 +77,14 @@ body_status(enum body_status e)
 
 /*--------------------------------------------------------------------*/
 
+enum req_body_state_e {
+#define REQ_BODY(U)	REQ_BODY_##U,
+#include <tbl/req_body.h>
+#undef REQ_BODY
+};
+
+/*--------------------------------------------------------------------*/
+
 enum sess_close {
 	SC_NULL = 0,
 #define SESS_CLOSE(nm, desc)	SC_##nm,
@@ -582,14 +590,6 @@ struct object {
 
 /*--------------------------------------------------------------------*/
 
-enum req_body_state_e {
-	REQ_BODY_INIT = 0,
-	REQ_BODY_CL,
-	// REQ_BODY_CHUNKED,
-	REQ_BODY_DONE,
-	REQ_BODY_NONE
-};
-
 struct req {
 	unsigned		magic;
 #define REQ_MAGIC		0x2751aaa1
@@ -605,6 +605,8 @@ struct req {
 	enum req_step		req_step;
 	VTAILQ_ENTRY(req)	w_list;
 
+	struct storagehead	body;
+
 	/* The busy objhead we sleep on */
 	struct objhead		*hash_objhead;
 	struct busyobj		*busyobj;
@@ -777,9 +779,11 @@ void VBO_DerefBusyObj(struct worker *wrk, struct busyobj **busyobj);
 void VBO_Free(struct busyobj **vbo);
 
 /* cache_http1_fsm.c [HTTP1] */
-ssize_t HTTP1_GetReqBody(struct req *, void *buf, ssize_t len);
-int HTTP1_DiscardReqBody(struct req *req);
+typedef int (req_body_iter_f)(struct req *, void *priv, void *ptr, size_t);
 void HTTP1_Session(struct worker *, struct req *);
+int HTTP1_DiscardReqBody(struct req *req);
+int HTTP1_CacheReqBody(struct req *req, ssize_t maxsize);
+int HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv);
 
 /* cache_req_fsm.c [CNT] */
 int CNT_Request(struct worker *, struct req *);
@@ -1074,6 +1078,7 @@ void STV_close(void);
 void STV_Freestore(struct object *o);
 int STV_BanInfo(enum baninfo event, const uint8_t *ban, unsigned len);
 void STV_BanExport(const uint8_t *bans, unsigned len);
+struct storage *STV_alloc_transient(size_t size);
 
 /* storage_synth.c */
 struct vsb *SMS_Makesynth(struct object *obj);

bin/varnishd/cache/cache_fetch.c

@@ -44,8 +44,6 @@
 
 static unsigned fetchfrag;
 
-static int fetchReqBody(struct req *req, int sendbody);
-
 /*--------------------------------------------------------------------
  * We want to issue the first error we encounter on fetching and
  * supress the rest.  This function does that.
@@ -373,39 +371,20 @@ fetch_eof(struct busyobj *bo, struct http_conn *htc)
 }
 
 /*--------------------------------------------------------------------
- * Fetch any body attached to the incoming request, and either write it
- * to the backend (if we pass) or discard it (anything else).
- * This is mainly a separate function to isolate the stack buffer and
- * to contain the complexity when we start handling chunked encoding.
+ * Pass the request body to the backend
  */
 
-static int
-fetchReqBody(struct req *req, int sendbody)
+static int __match_proto__(req_body_iter_f)
+fetch_iter_req_body(struct req *req, void *priv, void *ptr, size_t l)
 {
-	char buf[8192];
-	ssize_t l = 1234;
 
-	if (req->req_body_status == REQ_BODY_DONE) {
-		AZ(sendbody);
-		return (0);
-	}
-	while (req->req_body_status != REQ_BODY_DONE &&
-	    req->req_body_status != REQ_BODY_NONE) {
-		l = HTTP1_GetReqBody(req, buf, sizeof buf);
-		if (l < 0) {
-			return (1);
-		} else if (l == 0) {
-			assert(req->req_body_status == REQ_BODY_DONE ||
-			    req->req_body_status == REQ_BODY_NONE);
-		} else if (sendbody) {
-			/* XXX: stats ? */
-			(void)WRW_Write(req->wrk, buf, l);
-			if (WRW_Flush(req->wrk)) {
-				/* XXX: Try to salvage client-conn ? */
-				req->req_body_status = REQ_BODY_DONE;
-				return (2);
-			}
-		}
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	(void)priv;
+
+	if (l > 0) {
+		(void)WRW_Write(req->wrk, ptr, l);
+		if (WRW_Flush(req->wrk))
+			return (-1);
 	}
 	return (0);
 }
@@ -468,11 +447,19 @@ FetchHdr(struct req *req, int need_host_hdr, int sendbody)
 	WRW_Reserve(wrk, &vc->fd, bo->vsl, req->t_req);	/* XXX t_resp ? */
 	(void)http_Write(wrk, hp, 0);	/* XXX: stats ? */
 
-	/* Deal with any message-body the request might have */
-	i = fetchReqBody(req, sendbody);
-	if (sendbody && req->req_body_status == REQ_BODY_DONE)
-		retry = -1;
-	if (WRW_FlushRelease(wrk) || i > 0) {
+	/* Deal with any message-body the request might (still) have */
+	i = 0;
+
+	if (sendbody) {
+		i = HTTP1_IterateReqBody(req,
+		    fetch_iter_req_body, NULL);
+		if (req->req_body_status == REQ_BODY_DONE)
+			retry = -1;
+	} else {
+		i = HTTP1_DiscardReqBody(req);
+	}
+
+	if (WRW_FlushRelease(wrk) || i != 0) {
 		VSLb(req->vsl, SLT_FetchError,
 		    "backend write error: %d (%s)",
 		    errno, strerror(errno));

bin/varnishd/cache/cache_http1_fsm.c

@@ -80,7 +80,7 @@
 #include "vtcp.h"
 #include "vtim.h"
 
-/*--------------------------------------------------------------------
+/*----------------------------------------------------------------------
  * Collect a request from the client.
  */
 
@@ -161,7 +161,7 @@ http1_wait(struct sess *sp, struct worker *wrk, struct req *req)
 	return (1);
 }
 
-/*--------------------------------------------------------------------
+/*----------------------------------------------------------------------
  * This is the final state, figure out if we should close or recycle
  * the client connection
  */
@@ -235,7 +235,7 @@ http1_cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 	}
 }
 
-/*--------------------------------------------------------------------
+/*----------------------------------------------------------------------
  */
 
 static int
@@ -296,7 +296,7 @@ http1_dissect(struct worker *wrk, struct req *req)
 	return (0);
 }
 
-/*--------------------------------------------------------------------
+/*----------------------------------------------------------------------
  */
 
 void
@@ -375,75 +375,225 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 	}
 }
 
-/*
- * XXX: DiscardReqBody() is a dedicated function, because we might
- * XXX: be able to disuade or terminate its transmission in some protocols.
- */ 
-
-int
-HTTP1_DiscardReqBody(struct req *req)
-{
-	char buf[8192];
-	ssize_t l;
+/*----------------------------------------------------------------------
+ */
 
-	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-	while (req->req_body_status != REQ_BODY_DONE &&
-	    req->req_body_status != REQ_BODY_NONE) {
-		l = HTTP1_GetReqBody(req, buf, sizeof buf);
-		if (l < 0)
-			return (-1);
-	}
-	return (0);
-}
+struct http1_r_b_s {
+	ssize_t			bytes_done;
+	ssize_t			yet;
+	enum {CL, CHUNKED}	mode;
+};
 
-ssize_t
-HTTP1_GetReqBody(struct req *req, void *buf, ssize_t len)
+static int
+http1_setup_req_body(struct req *req, struct http1_r_b_s *rbs)
 {
 	char *ptr, *endp;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	memset(rbs, 0, sizeof *rbs);
 
-	if (req->req_body_status == REQ_BODY_INIT) {
-		if (http_GetHdr(req->http, H_Content_Length, &ptr)) {
-			AN(ptr);
-			if (*ptr == '\0') {
-				req->req_body_status = REQ_BODY_DONE;
-				return (-1);
-			}
-			req->req_bodybytes = strtoul(ptr, &endp, 10);
-			if (*endp != '\0' && !vct_islws(*endp)) {
-				req->req_body_status = REQ_BODY_DONE;
-				return (-1);
-			}
-			if (req->req_bodybytes == 0) {
-				req->req_body_status = REQ_BODY_DONE;
-				return (0);
-			}
-			req->req_body_status = REQ_BODY_CL;
-		} else if (http_GetHdr(req->http, H_Transfer_Encoding, NULL)) {
-			VSLb(req->vsl, SLT_Debug,
-			    "Transfer-Encoding in request");
-			req->req_body_status = REQ_BODY_DONE;
+	assert(req->req_body_status == REQ_BODY_INIT);
+	if (http_GetHdr(req->http, H_Content_Length, &ptr)) {
+		AN(ptr);
+		if (*ptr == '\0') {
+			req->req_body_status = REQ_BODY_FAIL;
 			return (-1);
-		} else {
+		}
+		req->req_bodybytes = strtoul(ptr, &endp, 10);
+		if (*endp != '\0' && !vct_islws(*endp)) {
+			req->req_body_status = REQ_BODY_FAIL;
+			return (-1);
+		}
+		if (req->req_bodybytes == 0) {
 			req->req_body_status = REQ_BODY_NONE;
 			return (0);
 		}
+		rbs->mode = CL;
+		rbs->yet = req->req_bodybytes - rbs->bytes_done;
+		return (0);
 	}
-	if (req->req_body_status == REQ_BODY_CL) {
-		if (req->req_bodybytes == 0) {
+
+	if (http_GetHdr(req->http, H_Transfer_Encoding, NULL)) {
+		rbs->mode = CHUNKED;
+		VSLb(req->vsl, SLT_Debug,
+		    "Transfer-Encoding in request");
+		req->req_body_status = REQ_BODY_DONE;
+		return (-1);
+	}
+
+	req->req_body_status = REQ_BODY_NONE;
+	req->req_bodybytes = 0;
+	return (0);
+}
+
+static ssize_t
+http1_iter_req_body(struct req *req, struct http1_r_b_s *rbs, void *buf,
+    ssize_t len)
+{
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+
+	if (rbs->mode == CL) {
+		AN(req->req_bodybytes);
+		AN(len);
+		AN(buf);
+		if (len > req->req_bodybytes - rbs->bytes_done)
+			len = req->req_bodybytes - rbs->bytes_done;
+		if (len == 0) {
 			req->req_body_status = REQ_BODY_DONE;
 			return (0);
 		}
-		if (len > req->req_bodybytes)
-			len = req->req_bodybytes;
 		len = HTC_Read(req->htc, buf, len);
 		if (len <= 0) {
-			req->req_body_status = REQ_BODY_DONE;
+			req->req_body_status = REQ_BODY_FAIL;
 			return (-1);
 		}
-		req->req_bodybytes -= len;
+		rbs->bytes_done += len;
+		rbs->yet = req->req_bodybytes - rbs->bytes_done;
 		return (len);
 	}
+	INCOMPL();
+}
+
+/*----------------------------------------------------------------------
+ * Iterate over the req.body.
+ *
+ * This can be done exactly once if uncached, and multiple times if the
+ * req.body is cached.
+ */
+
+int
+HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
+{
+	char buf[8192];
+	struct storage *st;
+	ssize_t l;
+	int i;
+	struct http1_r_b_s	rbs;
+
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	AN(func);
+
+	if (req->req_body_status == REQ_BODY_CACHED) {
+		VTAILQ_FOREACH(st, &req->body, list) {
+			i = func(req, priv, st->ptr, st->len);
+			if (i)
+				return (i);
+		}
+		return (0);
+	}
+
+	if (req->req_body_status == REQ_BODY_NONE)
+		return (0);
+
+	if (req->req_body_status != REQ_BODY_INIT)
+		return (-1);
+
+	i = http1_setup_req_body(req, &rbs);
+	if (i < 0)
+		return (i);
+
+	if (req->req_body_status == REQ_BODY_NONE)
+		return (0);
+
+	do {
+		l = http1_iter_req_body(req, &rbs, buf, sizeof buf);
+		if (l < 0)
+			return (l);
+		if (l > 0) {
+			i = func(req, priv, buf, l);
+			if (i)
+				return (i);
+		}
+	} while (l > 0);
+	return(0);
+}
+
+/*----------------------------------------------------------------------
+ * DiscardReqBody() is a dedicated function, because we might
+ * be able to disuade or terminate its transmission in some protocols.
+ * For HTTP1 we have no such luck, and we just iterate it into oblivion.
+ */
+
+static int __match_proto__(req_body_iter_f)
+httpq_req_body_discard(struct req *req, void *priv, void *ptr, size_t len)
+{
+
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	(void)priv;
+	(void)ptr;
+	(void)len;
 	return (0);
 }
+
+int
+HTTP1_DiscardReqBody(struct req *req)
+{
+
+	return(HTTP1_IterateReqBody(req, httpq_req_body_discard, NULL));
+}
+
+/*----------------------------------------------------------------------
+ * Cache the req.body if it is smaller than the given size
+ */
+
+int
+HTTP1_CacheReqBody(struct req *req, ssize_t maxsize)
+{
+	struct storage *st;
+	ssize_t l;
+	int i;
+	struct http1_r_b_s	rbs;
+
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+
+	if (req->req_body_status == REQ_BODY_CACHED)
+		return (0);
+
+	if (req->req_body_status == REQ_BODY_NONE)
+		return (0);
+
+	if (req->req_body_status != REQ_BODY_INIT)
+		return (-1);
+
+	i = http1_setup_req_body(req, &rbs);
+	if (i < 0)
+		return (i);
+
+	if (req->req_bodybytes > maxsize) {
+		req->req_body_status = REQ_BODY_FAIL;
+		return (-1);
+	}
+
+	if (req->req_body_status == REQ_BODY_NONE)
+		return (0);
+
+	st = NULL;
+	do {
+		if (st == NULL) {
+			st = STV_alloc_transient(
+			    rbs.yet ? rbs.yet : cache_param->fetch_chunksize);
+			if (st == NULL) {
+				req->req_body_status = REQ_BODY_FAIL;
+				return (-1);
+			} else {
+				VTAILQ_INSERT_TAIL(&req->body, st, list);
+			}
+		}
+
+		l = st->space - st->len;
+		l = http1_iter_req_body(req, &rbs, st->ptr + st->len, l);
+		if (l < 0)
+			return (l);
+		if (req->req_bodybytes > maxsize) {
+			req->req_body_status = REQ_BODY_FAIL;
+			return (-1);
+		}
+		if (l > 0) {
+			st->len += l;
+			if (st->space == st->len)
+				st = NULL;
+		}
+	} while (l > 0);
+	req->req_body_status = REQ_BODY_CACHED;
+	return(0);
+}

bin/varnishd/cache/cache_panic.c

@@ -232,7 +232,7 @@ pan_busyobj(const struct busyobj *bo)
 static void
 pan_req(const struct req *req)
 {
-	const char *hand, *stp;
+	const char *hand, *stp, *body;
 
 	VSB_printf(pan_vsp, "req = %p {\n", req);
 
@@ -249,6 +249,18 @@ pan_req(const struct req *req)
 	else
 		VSB_printf(pan_vsp, "  step = 0x%x,\n", req->req_step);
 
+	switch (req->req_body_status) {
+#define REQ_BODY(U) case REQ_BODY_##U: body = "R_BODY_" #U; break;
+#include "tbl/req_body.h"
+#undef REQ_BODY
+		default: body = NULL;
+	}
+	if (body != NULL)
+		VSB_printf(pan_vsp, "  req_body = %s,\n", body);
+	else
+		VSB_printf(pan_vsp, "  req_body = 0x%x,\n",
+		    req->req_body_status);
+
 	hand = VCL_Return_Name(req->handling);
 	if (hand != NULL)
 		VSB_printf(pan_vsp, "  handling = %s,\n", hand);

bin/varnishd/cache/cache_req_fsm.c

@@ -1089,6 +1089,7 @@ cnt_recv(const struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	CHECK_OBJ_NOTNULL(req->vcl, VCL_CONF_MAGIC);
+	AZ(req->objcore);
 	AZ(req->obj);
 	AZ(req->objcore);
 	AZ(req->busyobj);

bin/varnishd/cache/cache_session.c

@@ -394,6 +394,8 @@ SES_GetReq(struct worker *wrk, struct sess *sp)
 	req->t_req = NAN;
 	req->t_resp = NAN;
 
+	VTAILQ_INIT(&req->body);
+
 	return (req);
 }
 

bin/varnishd/cache/cache_vrt.c

@@ -511,6 +511,16 @@ VRT_ban_string(const struct req *req, const char *str)
 	VAV_Free(av);
 }
 
+/*--------------------------------------------------------------------
+ *
+ */
+
+int
+VRT_CacheReqBody(struct req *req, long long maxsize)
+{
+	return (HTTP1_CacheReqBody(req, maxsize));
+}
+
 /*--------------------------------------------------------------------
  * "real" purges
  */

bin/varnishd/storage/stevedore.c

@@ -177,7 +177,7 @@ stv_alloc(struct stevedore *stv, size_t size)
 		if (st != NULL)
 			break;
 
-		if (size <= cache_param->fetch_chunksize) 
+		if (size <= cache_param->fetch_chunksize)
 			break;
 
 		size <<= 1;
@@ -416,6 +416,13 @@ STV_alloc(struct busyobj *bo, size_t size)
 	return (stv_alloc_obj(bo, size));
 }
 
+struct storage *
+STV_alloc_transient(size_t size)
+{
+
+	return (stv_alloc(stv_transient, size));
+}
+
 void
 STV_trim(struct storage *st, size_t size, int move_ok)
 {

bin/varnishtest/tests/c00055.vtc

+varnishtest "test caching of req.body"
+
+server s1 {
+	rxreq
+	expect req.bodylen == 3
+	txresp -status 200 -hdr "Foo: BAR" -body "1234"
+	accept
+	rxreq
+	expect req.bodylen == 3
+	txresp -status 200 -hdr "Foo: Foo" -body "56"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		C{ VRT_CacheReqBody(req, 1000); }C
+		return (pass);
+	}
+	sub vcl_fetch {
+		if (beresp.http.foo == "BAR") {
+			return (restart);
+		}
+	}
+} -start
+
+client c1 {
+	txreq -body "FOO"
+	rxresp
+	expect resp.http.Foo == "Foo"
+	expect resp.bodylen == 2
+} -run

include/tbl/req_body.h

+/*-
+ * Copyright (c) 2013 Varnish Software AS
+ * All rights reserved.
+ *
+ * Author: Poul-Henning Kamp <phk@phk.freebsd.dk>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ */
+
+/*lint -save -e525 -e539 */
+
+REQ_BODY(INIT)
+REQ_BODY(CACHED)
+REQ_BODY(DONE)
+REQ_BODY(FAIL)
+REQ_BODY(NONE)

include/vrt.h

@@ -170,6 +170,10 @@ struct vrt_ref {
 
 void VRT_acl_log(struct req *, const char *msg);
 
+/* req related */
+
+int VRT_CacheReqBody(struct req *, long long maxsize);
+
 /* Regexp related */
 void VRT_re_init(void **, const char *);
 void VRT_re_fini(void *);