[master] 5f122e8 Determine req.body status earlier and add more paranioa around that state.

Poul-Henning Kamp phk at varnish-cache.org
Fri Jul 5 13:07:11 CEST 2013


commit 5f122e8374f4c50fdfeb144d0a7ba081be9cceae
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Fri Jul 5 11:06:39 2013 +0000

    Determine req.body status earlier and add more paranioa around that
    state.

diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 041626f..76fc1c5 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -60,6 +60,7 @@ ved_include(struct req *preq, const char *src, const char *host)
 	wrk_ws_wm = WS_Snapshot(wrk->aws); /* XXX ? */
 
 	req = SES_GetReq(wrk, preq->sp);
+	req->req_body_status = REQ_BODY_NONE;
 	VSLb(req->vsl, SLT_Begin, "esireq %u", preq->vsl->wid & VSL_IDENTMASK);
 	VSLb(preq->vsl, SLT_Link, "esireq %u", req->vsl->wid & VSL_IDENTMASK);
 	req->esi_level = preq->esi_level + 1;
diff --git a/bin/varnishd/cache/cache_http1_fsm.c b/bin/varnishd/cache/cache_http1_fsm.c
index b9bbacf..d4a296a 100644
--- a/bin/varnishd/cache/cache_http1_fsm.c
+++ b/bin/varnishd/cache/cache_http1_fsm.c
@@ -201,6 +201,7 @@ http1_cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 
 	req->t_req = NAN;
 	req->t_resp = NAN;
+	req->req_body_status = REQ_BODY_INIT;
 
 	// req->req_bodybytes = 0;
 	req->resp_bodybytes = 0;
@@ -239,6 +240,38 @@ http1_cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 /*----------------------------------------------------------------------
  */
 
+static enum req_body_state_e
+http1_req_body_status(struct req *req)
+{
+	char *ptr, *endp;
+
+	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+
+	if (http_GetHdr(req->http, H_Content_Length, &ptr)) {
+		AN(ptr);
+		if (*ptr == '\0')
+			return (REQ_BODY_FAIL);
+		req->req_bodybytes = strtoul(ptr, &endp, 10);
+		if (*endp != '\0' && !vct_islws(*endp))
+			return (REQ_BODY_FAIL);
+		if (req->req_bodybytes == 0)
+			return (REQ_BODY_NONE);
+		req->h1.mode = CL;
+		req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
+		return (REQ_BODY_PRESENT);
+	}
+
+	if (http_GetHdr(req->http, H_Transfer_Encoding, NULL)) {
+		req->h1.mode = CHUNKED;
+		VSLb(req->vsl, SLT_Debug, "Transfer-Encoding in request");
+		return (REQ_BODY_FAIL);
+	}
+	return (REQ_BODY_NONE);
+}
+
+/*----------------------------------------------------------------------
+ */
+
 static enum req_fsm_nxt
 http1_dissect(struct worker *wrk, struct req *req)
 {
@@ -248,6 +281,8 @@ http1_dissect(struct worker *wrk, struct req *req)
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
+	memset(&req->h1, 0, sizeof req->h1);
+
 	/*
 	 * Cache_req_fsm zeros the vxid once a requests is processed.
 	 * Allocate a new one only now that we know will need it.
@@ -294,8 +329,12 @@ http1_dissect(struct worker *wrk, struct req *req)
 		wrk->stats.client_req++;
 
 	http_Unset(req->http, H_Expect);
-	/* XXX: pull in req-body and make it available instead. */
-	req->req_body_status = REQ_BODY_INIT;
+	if (req->req_body_status == REQ_BODY_INIT)
+		req->req_body_status = http1_req_body_status(req);
+	else
+		assert(req->req_body_status == REQ_BODY_NONE);
+
+	assert(req->req_body_status != REQ_BODY_INIT);
 
 	HTTP_Copy(req->http0, req->http);	// For ESI & restart
 
@@ -394,55 +433,8 @@ HTTP1_Session(struct worker *wrk, struct req *req)
 	}
 }
 
-/*----------------------------------------------------------------------
- */
-
-static int
-http1_setup_req_body(struct req *req)
-{
-	char *ptr, *endp;
-
-	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-	memset(&req->h1, 0, sizeof req->h1);
-
-	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);
-		}
-		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);
-		}
-		req->h1.mode = CL;
-		req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
-		req->req_body_status = REQ_BODY_PRESENT;
-		return (0);
-	}
-
-	if (http_GetHdr(req->http, H_Transfer_Encoding, NULL)) {
-		req->h1.mode = CHUNKED;
-		VSLb(req->vsl, SLT_Debug,
-		    "Transfer-Encoding in request");
-		req->req_body_status = REQ_BODY_FAIL;
-		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, void *buf,
-    ssize_t len)
+http1_iter_req_body(struct req *req, void *buf, ssize_t len)
 {
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
@@ -486,36 +478,31 @@ HTTP1_IterateReqBody(struct req *req, req_body_iter_f *func, void *priv)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	AN(func);
 
-	if (req->req_body_status == REQ_BODY_CACHED) {
+	switch(req->req_body_status) {
+	case 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)
+	case REQ_BODY_NONE:
 		return (0);
-
-	if (req->req_body_status != REQ_BODY_INIT)
-		return (-1);
-
-	i = http1_setup_req_body(req);
-	if (i < 0)
-		return (i);
-
-	if (req->req_body_status == REQ_BODY_NONE)
-		return (0);
-
+	case REQ_BODY_PRESENT:
+		break;
+	default:
+		WRONG("Wrong req_body_status in HTTP1_IterateReqBody()");
+	}
 	do {
 		l = http1_iter_req_body(req, buf, sizeof buf);
-		if (l < 0)
+		if (l < 0) {
 			return (l);
+		}
 		if (l > 0) {
 			i = func(req, priv, buf, l);
-			if (i)
+			if (i) {
 				return (i);
+			}
 		}
 	} while (l > 0);
 	return(0);
@@ -542,6 +529,8 @@ int
 HTTP1_DiscardReqBody(struct req *req)
 {
 
+	if (req->req_body_status == REQ_BODY_DONE)
+		return(0);
 	return(HTTP1_IterateReqBody(req, httpq_req_body_discard, NULL));
 }
 
@@ -554,31 +543,24 @@ HTTP1_CacheReqBody(struct req *req, ssize_t maxsize)
 {
 	struct storage *st;
 	ssize_t l;
-	int i;
 
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 
-	if (req->req_body_status == REQ_BODY_CACHED)
+	switch(req->req_body_status) {
+	case REQ_BODY_CACHED:
+	case REQ_BODY_NONE:
 		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);
-	if (i < 0)
-		return (i);
+	case REQ_BODY_PRESENT:
+		break;
+	default:
+		WRONG("Wrong req_body_status in HTTP1_CacheReqBody()");
+	}
 
 	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) {
diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index d7ab8f8..81be78e 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -79,7 +79,7 @@ const char *
 reqbody_status_2str(enum req_body_state_e e)
 {
 	switch (e) {
-#define REQ_BODY(U) case REQ_BODY_##U: return("R_BODY_" #U); break;
+#define REQ_BODY(U) case REQ_BODY_##U: return("R_BODY_" #U);
 #include "tbl/req_body.h"
 #undef REQ_BODY
 	default:



More information about the varnish-commit mailing list