phk at varnish-cache
Apr 16, 2012, 7:07 AM
Post #1 of 1
(55 views)
Permalink
|
|
[master] 0679d60 Be much more cautious about how much workspace we have to build predictive vary string.
|
|
commit 0679d603fe4dad3027206f0cb272cd9c4aedb557
Author: Poul-Henning Kamp <phk [at] FreeBSD>
Date: Mon Apr 16 14:06:55 2012 +0000
Be much more cautious about how much workspace we have to build
predictive vary string.
Fixes #1120
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 073eb75..ee17d31 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -956,6 +956,7 @@ void RES_WriteObj(struct sess *sp);
struct vsb *VRY_Create(struct req *sp, const struct http *hp);
int VRY_Match(struct req *, const uint8_t *vary);
void VRY_Validate(const uint8_t *vary);
+void VRY_Prep(struct req *);
/* cache_vcl.c */
void VCL_Init(void);
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index 5530f1b..4a04f5b 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -1081,18 +1081,7 @@ cnt_lookup(struct sess *sp, struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(req->vcl, VCL_CONF_MAGIC);
AZ(req->busyobj);
- if (req->hash_objhead == NULL) {
- /* Not a waiting list return */
- AZ(req->vary_b);
- AZ(req->vary_l);
- AZ(req->vary_e);
- (void)WS_Reserve(req->ws, 0);
- } else {
- AN(req->ws->r);
- }
- req->vary_b = (void*)req->ws->f;
- req->vary_e = (void*)req->ws->r;
- req->vary_b[2] = '\0';
+ VRY_Prep(req);
AZ(req->objcore);
oc = HSH_Lookup(sp);
@@ -1359,7 +1348,7 @@ cnt_restart(struct sess *sp, const struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
-
+
req->director = NULL;
if (++req->restarts >= cache_param->max_restarts) {
req->err_code = 503;
diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index 138efda..bf6537f 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -176,22 +176,61 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2)
return (retval);
}
+/**********************************************************************
+ * Prepare predictive vary string
+ */
+
+void
+VRY_Prep(struct req *req)
+{
+ if (req->hash_objhead == NULL) {
+ /* Not a waiting list return */
+ AZ(req->vary_b);
+ AZ(req->vary_l);
+ AZ(req->vary_e);
+ (void)WS_Reserve(req->ws, 0);
+ } else {
+ AN(req->ws->r);
+ }
+ req->vary_b = (void*)req->ws->f;
+ req->vary_e = (void*)req->ws->r;
+ if (req->vary_b + 2 < req->vary_e)
+ req->vary_b[2] = '\0';
+}
+
+/**********************************************************************
+ * Match vary strings, and build a new cached string if possible.
+ *
+ * Return zero if there is certainly no match.
+ * Return non-zero if there could be a match or if we couldn't tell.
+ */
+
int
VRY_Match(struct req *req, const uint8_t *vary)
{
uint8_t *vsp = req->vary_b;
char *h, *e;
unsigned lh, ln;
- int i, retval = 1, oflo = 0;
+ int i, oflo = 0;
AN(vsp);
while (vary[2]) {
+ if (vsp + 2 >= req->vary_e) {
+ /*
+ * Too little workspace to find out
+ */
+ oflo = 1;
+ break;
+ }
i = vry_cmp(vary, vsp);
if (i == 1) {
- /* Build a new entry */
+ /*
+ * Different header, build a new entry,
+ * then compare again with that new entry.
+ */
- i = http_GetHdr(req->http,
- (const char*)(vary+2), &h);
+ ln = 2 + vary[2] + 2;
+ i = http_GetHdr(req->http, (const char*)(vary+2), &h);
if (i) {
/* Trim trailing space */
e = strchr(h, '\0');
@@ -199,55 +238,55 @@ VRY_Match(struct req *req, const uint8_t *vary)
e--;
lh = e - h;
assert(lh < 0xffff);
+ ln += lh;
} else {
e = h = NULL;
lh = 0xffff;
}
- /* Length of the entire new vary entry */
- ln = 2 + vary[2] + 2 + (lh == 0xffff ? 0 : lh);
- if (vsp + ln >= req->vary_e) {
- vsp = req->vary_b;
+ if (vsp + ln + 2 >= req->vary_e) {
+ /*
+ * Not enough space to build new entry
+ * and put terminator behind it.
+ */
oflo = 1;
+ break;
}
- /*
- * We MUST have space for one entry and the end marker
- * after it, which prevents old junk from confusing us
- */
- assert(vsp + ln + 2 < req->vary_e);
-
vbe16enc(vsp, (uint16_t)lh);
memcpy(vsp + 2, vary + 2, vary[2] + 2);
- if (h != NULL && e != NULL) {
- memcpy(vsp + 2 + vsp[2] + 2, h, e - h);
- vsp[2 + vary[2] + 2 + (e - h) + 2] = '\0';
- } else
- vsp[2 + vary[2] + 2 + 2] = '\0';
+ if (h != NULL)
+ memcpy(vsp + 2 + vsp[2] + 2, h, lh);
+ vsp[ln + 0] = 0xff;
+ vsp[ln + 1] = 0xff;
+ vsp[ln + 2] = 0;
+ VRY_Validate(vsp);
+ req->vary_l = vsp + 3;
i = vry_cmp(vary, vsp);
- assert(i != 1); /* hdr must be the same now */
+ assert(i == 0 || i == 2);
+ }
+ if (i == 0) {
+ /* Same header, same contents*/
+ vsp += vry_len(vsp);
+ vary += vry_len(vary);
+ } else if (i == 2) {
+ /* Same header, different contents, cannot match */
+ return (0);
}
- if (i != 0)
- retval = 0;
- vsp += vry_len(vsp);
- vary += vry_len(vary);
}
- if (vsp + 3 > req->vary_e)
- oflo = 1;
-
if (oflo) {
- /* XXX: Should log this */
vsp = req->vary_b;
- }
- vsp[0] = 0xff;
- vsp[1] = 0xff;
- vsp[2] = 0;
- if (oflo)
req->vary_l = NULL;
- else
- req->vary_l = vsp + 3;
- return (retval);
+ if (vsp + 2 < req->vary_e) {
+ vsp[0] = 0xff;
+ vsp[1] = 0xff;
+ vsp[2] = 0;
+ }
+ return (0);
+ } else {
+ return (1);
+ }
}
void
diff --git a/bin/varnishtest/tests/r01120.vtc b/bin/varnishtest/tests/r01120.vtc
new file mode 100644
index 0000000..97bc2e5
--- /dev/null
+++ b/bin/varnishtest/tests/r01120.vtc
@@ -0,0 +1,24 @@
+varnishtest "insanely long vary string"
+
+server s1 {
+ rxreq
+ txresp -hdr "Vary: Foo" -body "xxxx"
+ rxreq
+ txresp -hdr "Vary: Foo" -body "yyyyy"
+} -start
+
+varnish v1 \
+ -cliok "param.set workspace_client 8k" \
+ -cliok "param.set workspace_backend 200k" \
+ -vcl+backend {
+} -start
+
+client c1 {
+ txreq -hdr "Foo: blabla"
+ rxresp
+ expect resp.bodylen == 4
+ #txreq -hdr "Foo: blablaA"
+ txreq -hdr "Foo: blablaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaaaaaaaaaaaaaaaaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+ rxresp
+ expect resp.bodylen == 5
+} -run
_______________________________________________
varnish-commit mailing list
varnish-commit [at] varnish-cache
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit
|
