[master] 6edf9c3 Overhaul HTTP request/response size limits:
Poul-Henning Kamp
phk at varnish-cache.org
Wed Feb 23 12:26:50 CET 2011
commit 6edf9c379ed0ff20171c87c056730cf9084949b4
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Feb 23 11:24:28 2011 +0000
Overhaul HTTP request/response size limits:
http_headers is now called http_max_hdr and covers both requests and responses.
http_{req|resp}_size sets an upper limit on requests and responses, and
we (still) summarily close the connection if it is exceeded. (default: 32k)
http_{req|resp}_hdr_len sets the max size for any HTTP header line
except the first line (URL/response). (default 512)
diff --git a/bin/varnishd/cache.h b/bin/varnishd/cache.h
index 0ab2dc1..9ca8034 100644
--- a/bin/varnishd/cache.h
+++ b/bin/varnishd/cache.h
@@ -183,6 +183,8 @@ struct http_conn {
#define HTTP_CONN_MAGIC 0x3e19edd1
int fd;
+ unsigned maxbytes;
+ unsigned maxhdr;
struct ws *ws;
txt rxbuf;
txt pipeline;
@@ -702,7 +704,8 @@ void http_Unset(struct http *hp, const char *hdr);
void http_CollectHdr(struct http *hp, const char *hdr);
/* cache_httpconn.c */
-void HTC_Init(struct http_conn *htc, struct ws *ws, int fd);
+void HTC_Init(struct http_conn *htc, struct ws *ws, int fd, unsigned maxbytes,
+ unsigned maxhdr);
int HTC_Reinit(struct http_conn *htc);
int HTC_Rx(struct http_conn *htc);
ssize_t HTC_Read(struct http_conn *htc, void *d, size_t len);
diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
index 4a8b6ac..30c9f32 100644
--- a/bin/varnishd/cache_center.c
+++ b/bin/varnishd/cache_center.c
@@ -382,7 +382,7 @@ cnt_error(struct sess *sp)
HSH_Prealloc(sp);
/* XXX: 1024 is a pure guess */
sp->obj = STV_NewObject(sp, NULL, 1024, 0,
- params->http_headers);
+ params->http_max_hdr);
sp->obj->xid = sp->xid;
sp->obj->entered = sp->t_req;
} else {
@@ -769,7 +769,8 @@ cnt_first(struct sess *sp)
sp->ws_ses = WS_Snapshot(sp->ws);
/* Receive a HTTP protocol request */
- HTC_Init(sp->htc, sp->ws, sp->fd);
+ HTC_Init(sp->htc, sp->ws, sp->fd, params->http_req_size,
+ params->http_req_hdr_len);
sp->wrk->lastused = sp->t_open;
sp->acct_tmp.sess++;
diff --git a/bin/varnishd/cache_fetch.c b/bin/varnishd/cache_fetch.c
index 72549c8..911d576 100644
--- a/bin/varnishd/cache_fetch.c
+++ b/bin/varnishd/cache_fetch.c
@@ -438,7 +438,8 @@ FetchHdr(struct sess *sp)
/* Receive response */
- HTC_Init(sp->wrk->htc, sp->wrk->ws, vc->fd);
+ HTC_Init(sp->wrk->htc, sp->wrk->ws, vc->fd, params->http_resp_size,
+ params->http_resp_hdr_len);
TCP_set_read_timeout(vc->fd, vc->first_byte_timeout);
diff --git a/bin/varnishd/cache_http.c b/bin/varnishd/cache_http.c
index 323cead..4ebf878 100644
--- a/bin/varnishd/cache_http.c
+++ b/bin/varnishd/cache_http.c
@@ -491,9 +491,11 @@ http_GetReq(const struct http *hp)
*/
static int
-http_dissect_hdrs(struct worker *w, struct http *hp, int fd, char *p, txt t)
+http_dissect_hdrs(struct worker *w, struct http *hp, int fd, char *p,
+ const struct http_conn *htc)
{
char *q, *r;
+ txt t = htc->rxbuf;
if (*p == '\r')
p++;
@@ -524,6 +526,12 @@ http_dissect_hdrs(struct worker *w, struct http *hp, int fd, char *p, txt t)
*q++ = ' ';
}
+ if (q - p > htc->maxhdr) {
+ VSC_main->losthdr++;
+ WSL(w, SLT_LostHeader, fd, "%.*s", q - p, p);
+ return (400);
+ }
+
/* Empty header = end of headers */
if (p == q)
break;
@@ -629,7 +637,7 @@ http_splitline(struct worker *w, int fd, struct http *hp,
WSLH(w, fd, hp, h3);
}
- return (http_dissect_hdrs(w, hp, fd, p, htc->rxbuf));
+ return (http_dissect_hdrs(w, hp, fd, p, htc));
}
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/cache_httpconn.c b/bin/varnishd/cache_httpconn.c
index a73b5e1..93a7820 100644
--- a/bin/varnishd/cache_httpconn.c
+++ b/bin/varnishd/cache_httpconn.c
@@ -46,8 +46,7 @@ SVNID("$Id$")
* Check if we have a complete HTTP request or response yet
*
* Return values:
- * -1 No, and you can nuke the (white-space) content.
- * 0 No, keep trying
+ * 0 No, keep trying
* >0 Yes, it is this many bytes long.
*/
@@ -83,14 +82,17 @@ htc_header_complete(txt *t)
/*--------------------------------------------------------------------*/
void
-HTC_Init(struct http_conn *htc, struct ws *ws, int fd)
+HTC_Init(struct http_conn *htc, struct ws *ws, int fd, unsigned maxbytes,
+ unsigned maxhdr)
{
htc->magic = HTTP_CONN_MAGIC;
htc->ws = ws;
htc->fd = fd;
- /* XXX: ->s or ->f ? or param ? */
- (void)WS_Reserve(htc->ws, (htc->ws->e - htc->ws->s) / 2);
+ htc->maxbytes = maxbytes;
+ htc->maxhdr = maxhdr;
+
+ (void)WS_Reserve(htc->ws, htc->maxbytes);
htc->rxbuf.b = ws->f;
htc->rxbuf.e = ws->f;
*htc->rxbuf.e = '\0';
@@ -110,7 +112,7 @@ HTC_Reinit(struct http_conn *htc)
unsigned l;
CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
- (void)WS_Reserve(htc->ws, (htc->ws->e - htc->ws->s) / 2);
+ (void)WS_Reserve(htc->ws, htc->maxbytes);
htc->rxbuf.b = htc->ws->f;
htc->rxbuf.e = htc->ws->f;
if (htc->pipeline.b != NULL) {
diff --git a/bin/varnishd/cache_pool.c b/bin/varnishd/cache_pool.c
index 27b7253..225c08e 100644
--- a/bin/varnishd/cache_pool.c
+++ b/bin/varnishd/cache_pool.c
@@ -224,7 +224,7 @@ wrk_thread(void *priv)
CAST_OBJ_NOTNULL(qp, priv, WQ_MAGIC);
/* We need to snapshot these two for consistency */
- nhttp = params->http_headers;
+ nhttp = params->http_max_hdr;
siov = nhttp * 2;
if (siov > IOV_MAX)
siov = IOV_MAX;
diff --git a/bin/varnishd/cache_session.c b/bin/varnishd/cache_session.c
index f69d35e..8f26890 100644
--- a/bin/varnishd/cache_session.c
+++ b/bin/varnishd/cache_session.c
@@ -116,7 +116,7 @@ ses_sm_alloc(void)
* view of the value.
*/
nws = params->sess_workspace;
- nhttp = params->http_headers;
+ nhttp = params->http_max_hdr;
hl = HTTP_estimate(nhttp);
l = sizeof *sm + nws + 2 * hl;
p = malloc(l);
diff --git a/bin/varnishd/cache_ws.c b/bin/varnishd/cache_ws.c
index e55b1c7..62133d4 100644
--- a/bin/varnishd/cache_ws.c
+++ b/bin/varnishd/cache_ws.c
@@ -173,6 +173,8 @@ WS_Reserve(struct ws *ws, unsigned bytes)
assert(ws->r == NULL);
if (bytes == 0)
b2 = ws->e - ws->f;
+ else if (bytes > ws->e - ws->f)
+ b2 = ws->e - ws->f;
else
b2 = bytes;
b2 = PRNDDN(b2);
diff --git a/bin/varnishd/heritage.h b/bin/varnishd/heritage.h
index 7a8b90b..c527f98 100644
--- a/bin/varnishd/heritage.h
+++ b/bin/varnishd/heritage.h
@@ -96,7 +96,11 @@ struct params {
/* Memory allocation hints */
unsigned sess_workspace;
unsigned shm_workspace;
- unsigned http_headers;
+ unsigned http_req_size;
+ unsigned http_req_hdr_len;
+ unsigned http_resp_size;
+ unsigned http_resp_hdr_len;
+ unsigned http_max_hdr;
unsigned shm_reclen;
diff --git a/bin/varnishd/mgt_param.c b/bin/varnishd/mgt_param.c
index aa0e693..7a9ba14 100644
--- a/bin/varnishd/mgt_param.c
+++ b/bin/varnishd/mgt_param.c
@@ -519,10 +519,44 @@ static const struct parspec input_parspec[] = {
DELAYED_EFFECT,
"65536",
"bytes" },
- { "http_headers", tweak_uint, &master.http_headers, 32, UINT_MAX,
- "Maximum number of HTTP headers we will deal with.\n"
- "This space is preallocated in sessions and workthreads only "
- "objects allocate only space for the headers they store.\n",
+ { "http_req_hdr_len", tweak_uint, &master.http_req_hdr_len,
+ 40, UINT_MAX,
+ "Maximum length of any HTTP client request header we will "
+ "allow. The limit is inclusive its continuation lines.\n",
+ 0,
+ "512", "bytes" },
+ { "http_req_size", tweak_uint, &master.http_req_size,
+ 256, UINT_MAX,
+ "Maximum number of bytes of HTTP client request we will deal "
+ "with. This is a limit on all bytes up to the double blank "
+ "line which ends the HTTP request.\n"
+ "The memory for the request is allocated from the session "
+ "workspace (param: sess_workspace) and this parameter limits "
+ "how much of that the request is allowed to take up.",
+ 0,
+ "32768", "bytes" },
+ { "http_resp_hdr_len", tweak_uint, &master.http_resp_hdr_len,
+ 40, UINT_MAX,
+ "Maximum length of any HTTP backend response header we will "
+ "allow. The limit is inclusive its continuation lines.\n",
+ 0,
+ "512", "bytes" },
+ { "http_resp_size", tweak_uint, &master.http_resp_size,
+ 256, UINT_MAX,
+ "Maximum number of bytes of HTTP backend resonse we will deal "
+ "with. This is a limit on all bytes up to the double blank "
+ "line which ends the HTTP request.\n"
+ "The memory for the request is allocated from the worker "
+ "workspace (param: sess_workspace) and this parameter limits "
+ "how much of that the request is allowed to take up.",
+ 0,
+ "32768", "bytes" },
+ { "http_max_hdr", tweak_uint, &master.http_max_hdr, 32, UINT_MAX,
+ "Maximum number of HTTP headers we will deal with in "
+ "client request or backend reponses. "
+ "Note that the first line occupies five header fields.\n"
+ "This paramter does not influence storage consumption, "
+ "objects allocate exact space for the headers they store.\n",
0,
"64", "header lines" },
{ "shm_workspace", tweak_uint, &master.shm_workspace, 4096, UINT_MAX,
diff --git a/bin/varnishtest/tests/c00039.vtc b/bin/varnishtest/tests/c00039.vtc
new file mode 100644
index 0000000..a53742f
--- /dev/null
+++ b/bin/varnishtest/tests/c00039.vtc
@@ -0,0 +1,54 @@
+# $Id$
+
+test "request req and hdr length limits"
+
+server s1 {
+ rxreq
+ expect req.url == "/1"
+ txresp -bodylen 5
+
+ rxreq
+ expect req.url == "/2"
+ txresp -bodylen 5
+} -start
+
+varnish v1 \
+ -vcl+backend {
+ } -start
+
+varnish v1 -cliok "param.set http_req_size 256"
+varnish v1 -cliok "param.set http_req_hdr_len 40"
+
+client c1 {
+ txreq -url "/1" -hdr "1...5: ..0....5....0....5....0....5....0"
+ rxresp
+ expect resp.status == 200
+ txreq -url "/1" -hdr "1...5....0....5....0....5....0....5....0."
+ rxresp
+ expect resp.status == 400
+} -run
+
+client c1 {
+ txreq -url "/2" -hdr "1...5: ..0....5\n ..0....5....0....5....0"
+ rxresp
+ expect resp.status == 200
+ txreq -url "/2" -hdr "1...5....0....5\n ..0....5....0....5....0."
+ rxresp
+ expect resp.status == 400
+} -run
+
+client c1 {
+ txreq -url "/1" \
+ -hdr "1...5: ..0....5\n ..0....5....0....5....0" \
+ -hdr "1...5: ..0....5\n ..0....5....0....5....0" \
+ -hdr "1...5: ..0....5\n ..0....5....0....5....0" \
+ -hdr "1...5: ..0....5\n ..0....5....0....5....0" \
+ -hdr "1...5: ..0....5\n ..0....5....0....5....0" \
+ -hdr "1...5: ..0....5\n ..0...."
+ rxresp
+ expect resp.status == 200
+ # XXX: Varnish test does not allow us to test for the fact
+ # XXX: that the backend summarily closes on us. Adding one
+ # XXX: char to the above test, should cause that.
+} -run
+
diff --git a/bin/varnishtest/tests/c00040.vtc b/bin/varnishtest/tests/c00040.vtc
new file mode 100644
index 0000000..9faf18e
--- /dev/null
+++ b/bin/varnishtest/tests/c00040.vtc
@@ -0,0 +1,84 @@
+# $Id$
+
+test "request resp and hdr length limits"
+
+server s1 {
+ rxreq
+ expect req.url == "/1"
+ txresp \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -bodylen 1
+ rxreq
+ expect req.url == "/2"
+ txresp \
+ -hdr "1...5: ..0....5....0....5....0....5....0." \
+ -bodylen 2
+ accept
+ rxreq
+ expect req.url == "/3"
+ txresp \
+ -hdr "1...5: ..0....5....0\n ..5....0....5....0" \
+ -bodylen 3
+ rxreq
+ expect req.url == "/4"
+ txresp \
+ -hdr "1...5: ..0....5....0\n ..5....0....5....0." \
+ -bodylen 4
+
+ accept
+ rxreq
+ expect req.url == "/5"
+ txresp \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5" \
+ -hdr "1...5: ..0" \
+ -bodylen 5
+
+ rxreq
+ expect req.url == "/6"
+ txresp \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5....0" \
+ -hdr "1...5: ..0....5....0....5....0....5" \
+ -hdr "1...5: ..0." \
+ -bodylen 6
+} -start
+
+varnish v1 \
+ -vcl+backend {
+ } -start
+
+varnish v1 -cliok "param.set http_resp_size 256"
+varnish v1 -cliok "param.set http_resp_hdr_len 40"
+
+client c1 {
+ txreq -url "/1"
+ rxresp
+ expect resp.status == 200
+ txreq -url "/2"
+ rxresp
+ expect resp.status == 503
+} -run
+client c1 {
+ txreq -url "/3"
+ rxresp
+ expect resp.status == 200
+ txreq -url "/4"
+ rxresp
+ expect resp.status == 503
+} -run
+client c1 {
+ txreq -url "/5"
+ rxresp
+ expect resp.status == 200
+
+ txreq -url "/6"
+ rxresp
+ expect resp.status == 503
+} -run
+
diff --git a/bin/varnishtest/tests/r00498.vtc b/bin/varnishtest/tests/r00498.vtc
index 951d507..6e4e915 100644
--- a/bin/varnishtest/tests/r00498.vtc
+++ b/bin/varnishtest/tests/r00498.vtc
@@ -11,6 +11,8 @@ server s1 {
varnish v1 -vcl+backend {
} -start
+varnish v1 -cliok "param.set http_resp_hdr_len 32768"
+
client c1 {
txreq
rxresp
More information about the varnish-commit
mailing list