[master] 9c9a990 Make up our mind: Any req.* we receive from the client with fundamental trouble gets failed back without VCL involvement.

Poul-Henning Kamp phk at varnish-cache.org
Wed Oct 30 11:26:26 CET 2013


commit 9c9a9904bdb56b62017f338baf9c8e906b88dcac
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Oct 30 10:24:58 2013 +0000

    Make up our mind:  Any req.* we receive from the client with fundamental
    trouble gets failed back without VCL involvement.
    
    Fixes	#1367

diff --git a/bin/varnishd/cache/cache_http1_fsm.c b/bin/varnishd/cache/cache_http1_fsm.c
index 6a92bc5..c1564be 100644
--- a/bin/varnishd/cache/cache_http1_fsm.c
+++ b/bin/varnishd/cache/cache_http1_fsm.c
@@ -270,7 +270,9 @@ static enum req_fsm_nxt
 http1_dissect(struct worker *wrk, struct req *req)
 {
 	const char *r_100 = "HTTP/1.1 100 Continue\r\n\r\n";
+	const char *r_400 = "HTTP/1.1 400 Bad Request\r\n\r\n";
 	const char *r_411 = "HTTP/1.1 411 Length Required\r\n\r\n";
+	const char *r_417 = "HTTP/1.1 417 Expectation Failed\r\n\r\n";
 	char *p;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -299,8 +301,9 @@ http1_dissect(struct worker *wrk, struct req *req)
 	req->err_code = HTTP1_DissectRequest(req);
 
 	/* If we could not even parse the request, just close */
-	if (req->err_code == 400) {
+	if (req->err_code != 0) {
 		wrk->stats.client_req_400++;
+		(void)write(req->sp->fd, r_400, strlen(r_400));
 		SES_Close(req->sp, SC_RX_JUNK);
 		return (REQ_FSM_DONE);
 	}
@@ -317,25 +320,27 @@ http1_dissect(struct worker *wrk, struct req *req)
 		return (REQ_FSM_DONE);
 	}
 
-	req->acct_req.req++;
-
-	req->ws_req = WS_Snapshot(req->ws);
-	req->doclose = http_DoConnection(req->http);
-
-	/* XXX: Expect headers are a mess */
-	if (req->err_code == 0 && http_GetHdr(req->http, H_Expect, &p)) {
+	if (http_GetHdr(req->http, H_Expect, &p)) {
 		if (strcasecmp(p, "100-continue")) {
 			wrk->stats.client_req_417++;
 			req->err_code = 417;
-		} else if (strlen(r_100) !=
-		    write(req->sp->fd, r_100, strlen(r_100))) {
+			(void)write(req->sp->fd, r_417, strlen(r_417));
+			SES_Close(req->sp, SC_RX_JUNK);
+			return (REQ_FSM_DONE);
+		}
+		if (strlen(r_100) != write(req->sp->fd, r_100, strlen(r_100))) {
+			// XXX: stats counter ?
 			SES_Close(req->sp, SC_REM_CLOSE);
 			return (REQ_FSM_DONE);
 		}
-	} else if (req->err_code == 413)
-		wrk->stats.client_req_413++;
-	else
-		wrk->stats.client_req++;
+	}
+
+	wrk->stats.client_req++;
+
+	AZ(req->err_code);
+	req->acct_req.req++;
+	req->ws_req = WS_Snapshot(req->ws);
+	req->doclose = http_DoConnection(req->http);
 
 	http_Unset(req->http, H_Expect);
 
diff --git a/bin/varnishd/cache/cache_http1_proto.c b/bin/varnishd/cache/cache_http1_proto.c
index 6905ac2..583d2e1 100644
--- a/bin/varnishd/cache/cache_http1_proto.c
+++ b/bin/varnishd/cache/cache_http1_proto.c
@@ -251,9 +251,9 @@ htc_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
 		}
 
 		if (q - p > htc->maxhdr) {
-			VSLb(hp->vsl, SLT_BogoHeader, "%.*s",
+			VSLb(hp->vsl, SLT_BogoHeader, "Header too long: %.*s",
 			    (int)(q - p > 20 ? 20 : q - p), p);
-			return (413);
+			return (400);
 		}
 
 		/* Empty header = end of headers */
@@ -276,9 +276,9 @@ htc_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
 			http_VSLH(hp, hp->nhd);
 			hp->nhd++;
 		} else {
-			VSLb(hp->vsl, SLT_BogoHeader, "%.*s",
+			VSLb(hp->vsl, SLT_BogoHeader, "Too many headers: %.*s",
 			    (int)(q - p > 20 ? 20 : q - p), p);
-			return (413);
+			return (400);
 		}
 	}
 	return (0);
@@ -339,7 +339,7 @@ htc_splitline(struct http *hp, const struct http_conn *htc, int req)
 	hp->hd[h2].e = p;
 
 	if (!Tlen(hp->hd[h2]))
-		return (413);
+		return (400);
 
 	/* Skip SP */
 	for (; vct_issp(*p); p++) {
diff --git a/bin/varnishtest/tests/c00039.vtc b/bin/varnishtest/tests/c00039.vtc
index 780bec2..a9d35d8 100644
--- a/bin/varnishtest/tests/c00039.vtc
+++ b/bin/varnishtest/tests/c00039.vtc
@@ -23,7 +23,7 @@ client c1 {
 	expect resp.status == 200
 	txreq -url "/1" -hdr "1...5....0....5....0....5....0....5....0."
 	rxresp
-	expect resp.status == 413
+	expect resp.status == 400
 } -run
 
 client c1 {
@@ -32,7 +32,7 @@ client c1 {
 	expect resp.status == 200
 	txreq -url "/2" -hdr "1...5....0....5\n ..0....5....0....5....0."
 	rxresp
-	expect resp.status == 413
+	expect resp.status == 400
 } -run
 
 client c1 {
diff --git a/bin/varnishtest/tests/r01367.vtc b/bin/varnishtest/tests/r01367.vtc
new file mode 100644
index 0000000..cb2b1f8
--- /dev/null
+++ b/bin/varnishtest/tests/r01367.vtc
@@ -0,0 +1,24 @@
+varnishtest "blank GET"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend { 
+	sub vcl_error {
+		return (restart);
+	}
+} -start
+
+client c1 {
+	send "GET    \nHost: example.com\n\n"
+	rxresp
+	expect resp.status == 400
+} -run
+
+client c1 {
+	txreq -hdr "Expect: Santa-Claus"
+	rxresp
+	expect resp.status == 417
+} -run



More information about the varnish-commit mailing list