[4.1] 4df51cf Try to sort out the delivery of resp.body for special resp.status cases.

Poul-Henning Kamp phk at FreeBSD.org
Fri Sep 4 15:54:51 CEST 2015


commit 4df51cf26691e875682a39b279cd03f7e202fae8
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Sun Jul 26 14:40:22 2015 +0000

    Try to sort out the delivery of resp.body for special resp.status cases.
    
    Treat C-L or A-E in 204 backend responses as fetch_error.
    
    This hopefully fixes #1761

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 2df4481..6719345 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -655,7 +655,7 @@ struct sess {
  * or may not, be talking a "real" HTTP protocol itself.
  */
 
-typedef void vtr_deliver_f (struct req *, struct busyobj *);
+typedef void vtr_deliver_f (struct req *, struct busyobj *, int wantbody);
 
 struct transport {
 	unsigned		magic;
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 9aeba38..aa011b6 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -703,7 +703,7 @@ ved_vdp_bytes(struct req *req, enum vdp_action act, void **priv,
 /*--------------------------------------------------------------------*/
 
 static void __match_proto__(vtr_deliver_f)
-VED_Deliver(struct req *req, struct busyobj *bo)
+VED_Deliver(struct req *req, struct busyobj *bo, int wantbody)
 {
 	int i;
 	struct ecx *ecx;
@@ -714,6 +714,9 @@ VED_Deliver(struct req *req, struct busyobj *bo)
 
 	CAST_OBJ_NOTNULL(ecx, req->transport_priv, ECX_MAGIC);
 
+	if (wantbody == 0)
+		return;
+
 	req->res_mode |= RES_ESI_CHILD;
 	i = ObjCheckFlag(req->wrk, req->objcore, OF_GZIPED);
 	if (ecx->isgzip && i && !(req->res_mode & RES_ESI)) {
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 12703bd..aa1f4cf 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -345,7 +345,11 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 		 * [RFC2616 10.2.5 p60]
 		 */
 		wrk->stats->fetch_204++;
-		bo->htc->body_status = BS_NONE;
+		if (http_GetHdr(bo->beresp, H_Content_Length, NULL) ||
+		    http_GetHdr(bo->beresp, H_Transfer_Encoding, NULL))
+			bo->htc->body_status = BS_ERROR;
+		else
+			bo->htc->body_status = BS_NONE;
 	} else if (http_IsStatus(bo->beresp, 304)) {
 		/*
 		 * 304 is "Not Modified" it has no body.
diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c
index 23c9925..6122984 100644
--- a/bin/varnishd/cache/cache_range.c
+++ b/bin/varnishd/cache/cache_range.c
@@ -191,7 +191,10 @@ VRG_dorange(struct req *req, const char *r)
 			    "Content-Range: bytes */%jd",
 			    (intmax_t)req->resp_len);
 		http_PutResponse(req->resp, "HTTP/1.1", 416, NULL);
-		req->resp_len = -1;
-		req->wantbody = 0;
+		/*
+		 * XXX: We ought to produce a body explaining things.
+		 * XXX: That really calls for us to hit vcl_synth{}
+		 */
+		req->resp_len = 0;
 	}
 }
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index b0fe1e7..46f777e 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -51,9 +51,23 @@ static void
 cnt_vdp(struct req *req, struct busyobj *bo)
 {
 	const char *r;
+	uint16_t status;
+	int wantbody;
 
+	CHECK_OBJ_NOTNULL(req->transport, TRANSPORT_MAGIC);
 	req->res_mode = 0;
-	if (bo != NULL)
+	wantbody = 1;
+	status = http_GetStatus(req->resp);
+	if (!strcmp(req->http0->hd[HTTP_HDR_METHOD].b, "HEAD")) {
+		wantbody = 0;
+	} else if (status < 200 || status == 204) {
+		req->resp_len = 0;
+		http_Unset(req->resp, H_Content_Length);
+		wantbody = 0;
+	} else if (status == 304) {
+		http_Unset(req->resp, H_Content_Length);
+		wantbody = 0;
+	} else if (bo != NULL)
 		req->resp_len = http_GetContentLength(req->resp);
 	else
 		req->resp_len = ObjGetLen(req->wrk, req->objcore);
@@ -62,7 +76,7 @@ cnt_vdp(struct req *req, struct busyobj *bo)
 	 * Determine ESI status first.  Not dependent on wantbody, because
 	 * we want ESI to supress C-L in HEAD too.
 	 */
-	if (!req->disable_esi &&
+	if (!req->disable_esi && req->resp_len != 0 && wantbody &&
 	    ObjGetattr(req->wrk, req->objcore, OA_ESIDATA, NULL) != NULL) {
 		req->res_mode |= RES_ESI;
 		RFC2616_Weaken_Etag(req->resp);
@@ -70,7 +84,6 @@ cnt_vdp(struct req *req, struct busyobj *bo)
 		VDP_push(req, VDP_ESI, NULL, 0);
 	}
 
-
 	if (cache_param->http_gzip_support &&
 	    ObjCheckFlag(req->wrk, req->objcore, OF_GZIPED) &&
 	    !RFC2616_Req_Gzip(req->http)) {
@@ -80,18 +93,15 @@ cnt_vdp(struct req *req, struct busyobj *bo)
 
 	/*
 	 * Range comes after the others and pushes on bottom because
-	 * it can generate a correct C-L header.
+	 * it can (maybe) generate a correct C-L header.
 	 */
-	if (cache_param->http_range_support &&
-	    http_IsStatus(req->resp, 200)) {
+	if (cache_param->http_range_support && http_IsStatus(req->resp, 200)) {
 		http_SetHeader(req->resp, "Accept-Ranges: bytes");
-		if (req->wantbody &&
-		    http_GetHdr(req->http, H_Range, &r))
+		if (wantbody && http_GetHdr(req->http, H_Range, &r))
 			VRG_dorange(req, r);
 	}
 
-	CHECK_OBJ_NOTNULL(req->transport, TRANSPORT_MAGIC);
-	req->transport->deliver(req, bo);
+	req->transport->deliver(req, bo, wantbody);
 }
 
 /*--------------------------------------------------------------------
@@ -174,13 +184,8 @@ cnt_deliver(struct worker *wrk, struct req *req)
 	if (!(req->objcore->flags & OC_F_PASS)
 	    && req->esi_level == 0
 	    && http_IsStatus(req->resp, 200)
-	    && req->http->conds && RFC2616_Do_Cond(req)) {
+	    && req->http->conds && RFC2616_Do_Cond(req))
 		http_PutResponse(req->resp, "HTTP/1.1", 304, NULL);
-		req->wantbody = 0;
-	}
-
-	if (http_IsStatus(req->resp, 304))
-		req->wantbody = 0;
 
 	/* Grab a ref to the bo if there is one, and hand it down */
 	bo = HSH_RefBusy(req->objcore);
@@ -690,11 +695,6 @@ cnt_recv(struct worker *wrk, struct req *req)
 	assert(wrk->handling == VCL_RET_LOOKUP);
 	SHA256_Final(req->digest, &sha256ctx);
 
-	if (!strcmp(req->http->hd[HTTP_HDR_METHOD].b, "HEAD"))
-		req->wantbody = 0;
-	else
-		req->wantbody = 1;
-
 	switch(recv_handling) {
 	case VCL_RET_PURGE:
 		req->req_step = R_STP_PURGE;
diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c
index f0762df..324947b 100644
--- a/bin/varnishd/http1/cache_http1_deliver.c
+++ b/bin/varnishd/http1/cache_http1_deliver.c
@@ -62,7 +62,7 @@ v1d_bytes(struct req *req, enum vdp_action act, void **priv,
  */
 
 void __match_proto__(vtr_deliver_f)
-V1D_Deliver(struct req *req, struct busyobj *bo)
+V1D_Deliver(struct req *req, struct busyobj *bo, int wantbody)
 {
 	enum objiter_status ois;
 
@@ -70,20 +70,18 @@ V1D_Deliver(struct req *req, struct busyobj *bo)
 	CHECK_OBJ_ORNULL(bo, BUSYOBJ_MAGIC);
 	CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
 
-	if ((req->objcore->flags & OC_F_PRIVATE) &&
-	    !strcasecmp(http_GetMethod(req->http0), "HEAD")) {
-		/* HEAD+pass is allowed to send the C-L through unmolested. */
-	} else {
+	if (wantbody) {
 		http_Unset(req->resp, H_Content_Length);
-		if (req->resp_len >= 0 && !http_IsStatus(req->resp, 304))
+		if (req->resp_len >= 0)
 			http_PrintfHeader(req->resp,
 			    "Content-Length: %jd", req->resp_len);
 	}
 
-	if (http_GetHdr(req->resp, H_Content_Length, NULL))
+	if (req->resp_len == 0)
+		wantbody = 0;
+	else if (http_GetHdr(req->resp, H_Content_Length, NULL))
 		req->res_mode |= RES_LEN;
-
-	else if (req->wantbody) {
+	else if (wantbody) {
 		if (req->http->protover == 11) {
 			req->res_mode |= RES_CHUNKED;
 			http_SetHeader(req->resp, "Transfer-Encoding: chunked");
@@ -112,7 +110,7 @@ V1D_Deliver(struct req *req, struct busyobj *bo)
 		(void)V1L_Flush(req->wrk);
 
 	ois = OIS_DONE;
-	if (req->wantbody) {
+	if (wantbody) {
 		if (req->res_mode & RES_CHUNKED)
 			V1L_Chunked(req->wrk);
 		ois = VDP_DeliverObj(req);
diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc
index 0170b22..b1698cd 100644
--- a/bin/varnishtest/tests/c00034.vtc
+++ b/bin/varnishtest/tests/c00034.vtc
@@ -31,6 +31,8 @@ client c1 {
 	rxresp
 	expect resp.status == 416
 	expect resp.bodylen == 0
+	expect resp.http.content-length == "0"
+	expect resp.http.transfer-encoding == "<undef>"
 	expect resp.http.content-range == "bytes */100"
 
 	txreq -hdr "Range: bytes=0- 9"
diff --git a/bin/varnishtest/tests/g00001.vtc b/bin/varnishtest/tests/g00001.vtc
index ccdc39e..a1343fe 100644
--- a/bin/varnishtest/tests/g00001.vtc
+++ b/bin/varnishtest/tests/g00001.vtc
@@ -22,6 +22,8 @@ client c1 {
 	expect resp.bodylen == "3"
 } -run
 
+delay .1
+
 client c1 {
 	txreq -proto HTTP/1.0
 	rxresp
@@ -29,6 +31,8 @@ client c1 {
 	expect resp.http.content-encoding == <undef>
 } -run
 
+delay .1
+
 client c1 {
 	txreq -req HEAD
 	rxresp -no_obj
diff --git a/bin/varnishtest/tests/r01746.vtc b/bin/varnishtest/tests/r01746.vtc
index 30c6137..46793f8 100644
--- a/bin/varnishtest/tests/r01746.vtc
+++ b/bin/varnishtest/tests/r01746.vtc
@@ -6,8 +6,7 @@ server s1 {
 	rxreq
 	expect req.url == /b
 	txresp -status 204 -nolen \
-	    -hdr "Content-encoding: gzip" \
-	    -hdr "content-length: 10"
+	    -hdr "Content-encoding: gzip"
 } -start
 
 varnish v1 -vcl+backend {
diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h
index 389cc51..55bdc74 100644
--- a/include/tbl/req_flags.h
+++ b/include/tbl/req_flags.h
@@ -34,5 +34,4 @@ REQ_FLAG(disable_esi,		0, 0, "")
 REQ_FLAG(hash_ignore_busy,	1, 1, "")
 REQ_FLAG(hash_always_miss,	1, 1, "")
 REQ_FLAG(is_hit,		0, 0, "")
-REQ_FLAG(wantbody,		0, 0, "")
 /*lint -restore */



More information about the varnish-commit mailing list