Hi everyone, before Hamburg VDD I had sent an email regarding req.body handling and user-accesible functions. I attach a patch for bug #1664: std.cache_req_body(BYTES size)doesn't have errors handling on the provided size limitation.
The proposed solution let Varnish either buffer the whole request body or fail the request. If this patch is ok, I can then start digging into user-accesible functions. Comments much appreciated. -- Arianna Aondio Software Developer | Varnish Software AS Mobile: +47 980 62 619 We Make Websites Fly www.varnish-software.com
From 5cb7b127504eaeb297e60dd1c1e98f247e0d3e35 Mon Sep 17 00:00:00 2001 From: Arianna Aondio <[email protected]> Date: Fri, 6 Mar 2015 17:04:04 +0100 Subject: [PATCH] Cache_req_body error handling. --- bin/varnishd/cache/cache_req_body.c | 11 ++++++++--- bin/varnishd/cache/cache_req_fsm.c | 4 +++- bin/varnishtest/tests/c00055.vtc | 5 +++++ bin/varnishtest/tests/c00067.vtc | 12 +++--------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c index c73db54..c3965ce 100644 --- a/bin/varnishd/cache/cache_req_body.c +++ b/bin/varnishd/cache/cache_req_body.c @@ -218,9 +218,10 @@ VRB_Cache(struct req *req, ssize_t maxsize) CHECK_OBJ_NOTNULL(req->htc, HTTP_CONN_MAGIC); vfc = req->htc->vfc; + VFP_Setup(vfc); + vfc->wrk = req->wrk; if (req->htc->content_length > maxsize) { - // XXX #1664 req->req_body_status = REQ_BODY_FAIL; (void)VFP_Error(vfc, "Request body too big to cache"); return (-1); @@ -230,9 +231,7 @@ VRB_Cache(struct req *req, ssize_t maxsize) AN(req->body_oc); XXXAN(STV_NewObject(req->body_oc, req->wrk, TRANSIENT_STORAGE, 8)); - VFP_Setup(vfc); vfc->http = req->http; - vfc->wrk = req->wrk; vfc->oc = req->body_oc; V1F_Setup_Fetch(vfc, req->htc); @@ -248,6 +247,12 @@ VRB_Cache(struct req *req, ssize_t maxsize) yet = 0; do { AZ(vfc->failed); + if (req->req_bodybytes > maxsize) { + req->req_body_status = REQ_BODY_FAIL; + (void)VFP_Error(vfc, "Request body too big to cache"); + VFP_Close(vfc); + return(-1); + } l = yet; if (VFP_GetStorage(vfc, &l, &ptr) != VFP_OK) break; diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index b74c66b..3ad045f 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -607,8 +607,10 @@ cnt_recv(struct worker *wrk, struct req *req) VCL_recv_method(req->vcl, wrk, req, NULL, req->http->ws); /* Attempts to cache req.body may fail */ - if (req->req_body_status == REQ_BODY_FAIL) + if (req->req_body_status == REQ_BODY_FAIL) { + req->doclose = SC_RX_BODY; return (REQ_FSM_DONE); + } recv_handling = wrk->handling; diff --git a/bin/varnishtest/tests/c00055.vtc b/bin/varnishtest/tests/c00055.vtc index 5f525d0..154c15c 100644 --- a/bin/varnishtest/tests/c00055.vtc +++ b/bin/varnishtest/tests/c00055.vtc @@ -69,5 +69,10 @@ client c1 { expect resp.status == 200 expect resp.http.X-BodyBytes == 0 } -run + +client c2 { + txreq -req POST -nolen -hdr "Content-Length: 1025" + expect_close +} -run varnish v1 -stop logexpect l1 -wait diff --git a/bin/varnishtest/tests/c00067.vtc b/bin/varnishtest/tests/c00067.vtc index 427042b..744cbcd 100644 --- a/bin/varnishtest/tests/c00067.vtc +++ b/bin/varnishtest/tests/c00067.vtc @@ -34,7 +34,7 @@ varnish v1 -vcl+backend { import ${vmod_std}; sub vcl_recv { - std.cache_req_body(1000B); + std.cache_req_body(110B); } } @@ -42,12 +42,6 @@ client c1 { txreq -req POST -nolen -hdr "Transfer-encoding: chunked" chunked {BLAS} delay .2 - chunkedlen 100 - delay .2 - chunked {TFOO} - delay .2 - chunkedlen 0 - rxresp - expect resp.status == 200 - expect resp.bodylen == 5 + chunkedlen 110 + expect_close } -run -- 1.9.1
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
