Update after txt constification (my fault :P)
iow this patch is based upon e40edceffff77d0578237a85cf5109cd0d069599
-
Hi,
On 22/09/14 09:37, Poul-Henning Kamp wrote:
>
> I would do it something like this instead:
>
> did = 0
> Reserve workspace
> loop over tokens in src-header
> if token != target
> copy token to workspace
> did = 1
> if (did):
> release unused workspace
> else:
> return all reserved workspace
> return (did)
>
> That would largely be a duplication of http_GetHdrToken() just a few
> lines longer.
I actually have done an implementation along these lines, but I did not like it
at all, it got really lengthy, lacked generality etc etc.
With the idea in mind that we could have general header element editing
facilities (no matter in a vmod or core/vcl), I have written a more generic
prototype around the suggested http_RemoveHdrToken function following the design
example of struct http and the HTTP_* functions.
For the protoype I have worked in cache_http.c, the source structure and the
question of which functions to declare static are things I'd like to think about
later.
struct httphdr holds two txt arrays for the parse of a header line - one for
names and one for values (think "max-age=200")
HTTPHDR_estimate and HTTPHDR_create closely follow the HTTP_ examples.
HTTPHDR_Tokenize dissects a header line into element keys and values. The
seperating characters are to be generalized later.
HTTPHDR_Join constructs a header line from a struct httphdr on some buffer
provided.
HTTPHDR_SameToken compares two header elemnts passed as (c)txt structs either
case sensitively for the quoted or case insensitively for the unquoted case.
HTTPHDR_RemoveToken nulls all instances of a token name in a struct httphdr
http_RemoveHdrToken (and the corresponding VRT) is the PoC implementation. It
builds the struct httphdr parse on the stack and only leaves a finalized header
on the workspace.
bin/varnishtest/tests/removehdrtoken.vtc is a working test for the remove header
token example.
I'd very much appreciate feedback on this.
Nils
>From f22f65fb8f00a322d798085df4c4a3647e9dd229 Mon Sep 17 00:00:00 2001
From: Nils Goroll <[email protected]>
Date: Sun, 28 Sep 2014 20:18:40 +0200
Subject: [PATCH] prototype - header element parsing - update1
---
bin/varnishd/cache/cache.h | 2 +
bin/varnishd/cache/cache_http.c | 310 +++++++++++++++++++++++++++++++
bin/varnishd/cache/cache_vrt.c | 15 ++
bin/varnishtest/tests/removehdrtoken.vtc | 199 ++++++++++++++++++++
include/vrt.h | 1 +
5 files changed, 527 insertions(+)
create mode 100644 bin/varnishtest/tests/removehdrtoken.vtc
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 39359b4..76377fa 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -879,6 +879,8 @@ void http_Teardown(struct http *ht);
int http_GetHdr(const struct http *hp, const char *hdr, const char **ptr);
int http_GetHdrToken(const struct http *hp, const char *hdr,
const char *token, const char **ptr);
+int http_RemoveHdrToken(struct http *hp, const char *hdr,
+ const char *token);
int http_GetHdrField(const struct http *hp, const char *hdr,
const char *field, const char **ptr);
double http_GetHdrQ(const struct http *hp, const char *hdr, const char *field);
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 05de39f..bf5b582 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -461,6 +461,316 @@ http_GetHdrToken(const struct http *hp, const char *hdr,
return (0);
}
+/* XXX .h */
+
+struct httphdr {
+ unsigned magic;
+#define HTTPHDR_MAGIC 0x0edf23d7
+
+ uint8_t sel; /* size of the el array */
+ uint8_t nel; /* next free element */
+ uint8_t lel; /* element limit - max used + 1 */
+ txt *el;
+ txt *val;
+};
+
+unsigned HTTPHDR_estimate(unsigned sel);
+struct httphdr *HTTPHDR_create(void *p, uint8_t sel);
+void HTTPHDR_lel(struct httphdr *hphdr);
+void HTTPHDR_nel(struct httphdr *hphdr);
+void HTTPHDR_freeel(struct httphdr *hphdr, uint8_t i);
+int HTTPHDR_Tokenize(struct httphdr *hphdr, const char *h);
+int HTTPHDR_Join(struct httphdr *hphdr, const txt *hdrn, const txt *sep,
+ char *buf, int space);
+int HTTPHDR_SameToken(const txt *a, const txt *b);
+int HTTPHDR_RemoveToken(struct httphdr *hphdr, const txt *tok);
+
+/* END XXX .h */
+
+unsigned
+HTTPHDR_estimate(unsigned sel)
+{
+ return (PRNDUP(sizeof (struct httphdr) + sizeof(txt) * sel * 2));
+}
+
+struct httphdr *
+HTTPHDR_create(void *p, uint8_t sel)
+{
+ struct httphdr *hphdr;
+
+ hphdr = p;
+ hphdr->magic = HTTPHDR_MAGIC;
+ hphdr->el = (void*)(hphdr + 1);
+ memset(hphdr->el, 0, sizeof(txt) * sel);
+ hphdr->val = (void *)(hphdr->el + sel);
+ memset(hphdr->val, 0, sizeof(txt) * sel);
+ hphdr->sel = sel;
+ hphdr->nel = hphdr->lel = 0;
+
+ return(hphdr);
+}
+
+/* set hphdr->lel after update of hphdr->nel */
+inline void
+HTTPHDR_lel(struct httphdr *hphdr) {
+ if (hphdr->nel >= hphdr->lel)
+ hphdr->lel = hphdr->nel + 1;
+ assert(hphdr->lel <= hphdr->sel);
+}
+
+/* set hphdr->nel to next free element */
+inline void
+HTTPHDR_nel(struct httphdr *hphdr) {
+ while (hphdr->el[hphdr->nel].b != NULL)
+ hphdr->nel++;
+}
+
+/* free this element */
+inline void
+HTTPHDR_freeel(struct httphdr *hphdr, uint8_t i)
+{
+ hphdr->el[i].b = hphdr->el[i].e = NULL;
+ hphdr->val[i].b = hphdr->val[i].e = NULL;
+
+ if (i < hphdr->nel)
+ hphdr->nel = i;
+
+ if (i == hphdr->lel - 1)
+ hphdr->lel--;
+}
+
+#define MAX_HTTPHDR_TOKEN 16
+
+/*
+ * XXX do we want to error for "tok tok" (without comma) ?
+ */
+
+int
+HTTPHDR_Tokenize(struct httphdr *hphdr, const char *h)
+{
+ CHECK_OBJ_NOTNULL(hphdr, HTTPHDR_MAGIC);
+ AN(h);
+
+ while (*h != '\0') {
+ /* Skip leading/trailing whitespace and commas */
+ if (vct_islws(*h) || *h == ',') {
+ h++;
+ continue;
+ }
+
+ if (hphdr->nel == hphdr->sel)
+ return (0); /* nospc */
+ assert(hphdr->nel < hphdr->sel);
+
+ hphdr->el[hphdr->nel].b = h;
+ /* sanity */
+ hphdr->val[hphdr->nel].b = hphdr->val[hphdr->nel].e = NULL;
+ while(*h != '\0' && (! (vct_islws(*h) || *h == ','))) {
+ if (*h == '=') {
+ hphdr->el[hphdr->nel].e = h;
+ h++;
+ hphdr->val[hphdr->nel].b = h;
+ while(*h != '\0' &&
+ (! (vct_islws(*h) || *h == ',')))
+ h++;
+ hphdr->val[hphdr->nel].e = h;
+ } else {
+ h++;
+ }
+ }
+
+ if (hphdr->val[hphdr->nel].e == NULL)
+ hphdr->el[hphdr->nel].e = h;
+
+ HTTPHDR_lel(hphdr);
+ HTTPHDR_nel(hphdr);
+ }
+
+ return (1);
+}
+
+#define appnd_txt(where, what, space) do { \
+ unsigned _l = Tlen(what); \
+ if ((_l + 1) > (space)) \
+ return (-ENOSPC); \
+ memcpy((where), (what).b, _l); \
+ (where) += (_l); \
+ (space) -= (_l); \
+ } while(0);
+
+/*
+ * Join a httphdr into a new string by copying
+ *
+ * return values:
+ * -ENOSPC if not enogh space
+ * 0 nothing to do
+ * bytes used (including trailing null) otherwise
+ *
+ * may leave partial results in buf when returning 0
+ */
+int
+HTTPHDR_Join(struct httphdr *hphdr, const txt *hdrn, const txt *sep,
+ char *buf, int space)
+{
+ const unsigned ospace = space;
+ int r = 0;
+ uint8_t i;
+
+ if (hdrn->b[0] != '\0') {
+ appnd_txt(buf, *hdrn, space);
+ /* appnd_txt checks for one more byte */
+ *buf++ = ' ';
+ space--;
+ }
+
+ for (i = 0; i < hphdr->lel; i++) {
+ if (hphdr->el[i].b == NULL)
+ continue;
+ if (r != 0)
+ appnd_txt(buf, *sep, space);
+ r = 1;
+ appnd_txt(buf, hphdr->el[i], space);
+ if (hphdr->val[i].b != NULL) {
+ /* appnd_txt checks for one more byte */
+ *buf++ = '=';
+ space--;
+ appnd_txt(buf, hphdr->val[i], space);
+ }
+ }
+
+ if (r == 0)
+ return (0);
+
+ /* appnd_txt checks for one more byte */
+ *buf++ = '\0';
+ space--;
+
+ assert(space < ospace);
+ assert(space >= 0);
+ return (ospace - space);
+}
+
+/* XXX more generic name */
+inline int
+HTTPHDR_SameToken(const txt *a, const txt *b)
+{
+ unsigned al = Tlen(*a);
+ unsigned bl = Tlen(*b);
+ int aq, bq;
+ txt ac, bc;
+
+ if ((aq = (al > 2 && a->b[0] == '"' && a->e[-1] == '"'))) {
+ ac.b = a->b + 1;
+ ac.e = a->e - 1;
+ a = ∾
+ al -= 2;
+ }
+
+ if ((bq = (bl > 2 && b->b[0] == '"' && b->e[-1] == '"'))) {
+ bc.b = b->b + 1;
+ bc.e = b->e - 1;
+ b = &bc;
+ bl -= 2;
+ }
+
+ if (al != bl)
+ return (0);
+
+ return ((aq || bq)
+ ? !memcmp(a->b, b->b, al)
+ : !strncasecmp(a->b, b->b, al));
+}
+
+int
+HTTPHDR_RemoveToken(struct httphdr *hphdr, const txt *tok)
+{
+ int found = 0;
+ uint8_t i;
+
+ for (i = 0; i < hphdr->lel; i++) {
+ if (hphdr->el[i].b == NULL)
+ continue;
+
+ if (HTTPHDR_SameToken(&hphdr->el[i], tok)) {
+ HTTPHDR_freeel(hphdr, i);
+ found++;
+ }
+ }
+
+ return (found);
+}
+
+int
+http_RemoveHdrToken(struct http *hp, const char *hdr,
+ const char *token)
+{
+ char hphdrspc[HTTPHDR_estimate(MAX_HTTPHDR_TOKEN)];
+ struct httphdr *hphdr;
+
+ const char *h, *sep = ", ";
+ char *n;
+
+ const txt tokt = { .b = token,
+ .e = token + strlen(token) };
+
+ const txt hdrt = { .b = hdr + 1,
+ .e = hdr + 1 + *hdr };
+
+ const txt sept = { .b = sep,
+ .e = sep + strlen(sep) };
+
+ unsigned ws_l;
+ int ws_used;
+
+ AN(token);
+ if (*token == '\0')
+ return (0);
+
+ if (! http_GetHdr(hp, hdr, &h))
+ return (0);
+ AN(h);
+
+ hphdr = HTTPHDR_create((void *)hphdrspc, MAX_HTTPHDR_TOKEN);
+ if (HTTPHDR_Tokenize(hphdr, h) != 1) {
+ http_fail(hp);
+ VSLb(hp->vsl, SLT_LostHeader,
+ "Remove %s from %s more than max %d tokens",
+ token, hdr + 1, MAX_HTTPHDR_TOKEN);
+ }
+
+ if (hphdr->lel == 0) {
+ /* nothing but white space/commas */
+ http_Unset(hp, hdr);
+ return (1);
+ }
+
+ if (HTTPHDR_RemoveToken(hphdr, &tokt) == 0)
+ return (0);
+
+ if (WS_Overflowed(hp->ws))
+ return (0);
+
+ ws_l = WS_Reserve(hp->ws, 0);
+ n = hp->ws->f;
+ ws_used = HTTPHDR_Join(hphdr, &hdrt, &sept, n, ws_l);
+
+ if (ws_used < 0) {
+ http_fail(hp);
+ VSLb(hp->vsl, SLT_LostHeader,
+ "Remove %s from %s Not enough workspace",
+ token, hdr + 1);
+ WS_Release(hp->ws, 0);
+ return (0);
+ }
+
+ WS_Release(hp->ws, ws_used);
+ http_Unset(hp, hdr);
+
+ if (ws_used > 0)
+ http_SetHeader(hp, n);
+ return (1);
+}
+
/*--------------------------------------------------------------------
* Find a given headerfields Q value.
*/
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index c41bcaa..f28e767 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -245,6 +245,21 @@ VRT_SetHdr(VRT_CTX , const struct gethdr_s *hs,
va_end(ap);
}
+void
+VRT_RemoveHdrToken(VRT_CTX , const struct gethdr_s *hs,
+ const char *token)
+{
+ struct http *hp;
+
+ CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+ AN(hs);
+ AN(hs->what);
+ hp = vrt_selecthttp(ctx, hs->where);
+ CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
+
+ (void)http_RemoveHdrToken(hp, hs->what, token);
+}
+
/*--------------------------------------------------------------------*/
void
diff --git a/bin/varnishtest/tests/removehdrtoken.vtc b/bin/varnishtest/tests/removehdrtoken.vtc
new file mode 100644
index 0000000..d267ef5
--- /dev/null
+++ b/bin/varnishtest/tests/removehdrtoken.vtc
@@ -0,0 +1,199 @@
+varnishtest "Test VRT_RemoveHdrToken / http_RemoveHdrToken"
+
+server s1 {
+ rxreq
+ expect req.url == "/nohdr"
+ txresp -hdr "X-TestA: removeme"
+
+ rxreq
+ expect req.url == "/unchanged"
+ txresp -hdr "X-Test: unchanged"
+
+ rxreq
+ expect req.url == "/fullremovalws"
+ txresp -hdr "X-Test: , ,, "
+
+ rxreq
+ expect req.url == "/fullremoval1"
+ txresp -hdr "X-Test: ReMoveMe"
+
+ rxreq
+ expect req.url == "/fullremoval1q"
+ txresp -hdr {X-Test: "removemequot"}
+
+ rxreq
+ expect req.url == "/fullremoval2"
+ txresp -hdr "X-Test: , ReMoveMe, removeME=,,, reMOVEme=val ,,,removeme "
+
+ rxreq
+ expect req.url == "/fullremoval2q"
+ txresp -hdr {X-Test: ,, "removemequot" , "removemequot" , , "removemequot"=val,"removemequot"}
+
+ rxreq
+ expect req.url == "/front1"
+ txresp -hdr "X-Test: ReMoveMe, remaining"
+
+ rxreq
+ expect req.url == "/front1q"
+ txresp -hdr {X-Test: "removemequot" ,remaining}
+
+ rxreq
+ expect req.url == "/front2"
+ txresp -hdr "X-Test: , ReMoveMe=val, remaining , "
+
+ rxreq
+ expect req.url == "/front2q"
+ txresp -hdr {X-Test: ,, "removemequot"=1 , , remaining, }
+
+ rxreq
+ expect req.url == "/end1"
+ txresp -hdr "X-Test: remaining, ReMoveMe"
+
+ rxreq
+ expect req.url == "/end1q"
+ txresp -hdr {X-Test: remaining, "removemequot"}
+
+ rxreq
+ expect req.url == "/end2"
+ txresp -hdr "X-Test: , remaining, ReMoveMe=val, , "
+
+ rxreq
+ expect req.url == "/end2q"
+ txresp -hdr {X-Test: ,remaining, "removemequot"= , ,}
+
+ rxreq
+ expect req.url == "/middle1"
+ txresp -hdr "X-Test: remaining=1, ReMoveMe, remaining"
+
+ rxreq
+ expect req.url == "/middle1q"
+ txresp -hdr {X-Test: remaining=1, "removemequot", remaining}
+
+ rxreq
+ expect req.url == "/middle2"
+ txresp -hdr "X-Test: , remaining=1, ReMoveMe=val,,, remaining, , "
+
+ rxreq
+ expect req.url == "/middle2q"
+ txresp -hdr {X-Test: ,remaining=1, "removemequot"= remaining , ,}
+
+ rxreq
+ expect req.url == "/much"
+ txresp -hdr "X-Test: ,, removeme ,removeme=abc , remaining=1, ReMoveMe=val,,, remaining, , "
+
+ rxreq
+ expect req.url == "/muchq"
+ txresp -hdr {X-Test: , "removemequot"=abc ,remaining=1, "removemequot","removemequot"= remaining , ,}
+
+} -start
+
+varnish v1 -cliok "param.set vcc_allow_inline_c true" -vcl+backend {
+
+ C{
+ static const struct gethdr_s VGC_HDR_BERESP_test =
+ { HDR_BERESP, "\007X-Test:"};
+ }C
+
+ sub vcl_recv {
+ return(pass);
+ }
+
+ sub vcl_backend_response {
+ C{
+ /* no effect */
+ VRT_RemoveHdrToken(ctx, &VGC_HDR_BERESP_test, "");
+ VRT_RemoveHdrToken(ctx, &VGC_HDR_BERESP_test, "removeme");
+ VRT_RemoveHdrToken(ctx, &VGC_HDR_BERESP_test, "\"removemequot\"");
+ }C
+ }
+} -start
+
+client c1 {
+ txreq -url /nohdr
+ rxresp
+ expect resp.http.X-TestA == removeme
+
+ txreq -url /unchanged
+ rxresp
+ expect resp.http.X-Test == unchanged
+
+ txreq -url /fullremovalws
+ rxresp
+ expect resp.http.X-Test == <undef>
+
+ txreq -url /fullremoval1
+ rxresp
+ expect resp.http.X-Test == <undef>
+
+ txreq -url /fullremoval1q
+ rxresp
+ expect resp.http.X-Test == <undef>
+
+ txreq -url /fullremoval2
+ rxresp
+ expect resp.http.X-Test == <undef>
+
+ txreq -url /fullremoval2q
+ rxresp
+ expect resp.http.X-Test == <undef>
+
+ txreq -url /front1
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /front1q
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /front2
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /front2q
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /end1
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /end1q
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /end2
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /end2q
+ rxresp
+ expect resp.http.X-Test == remaining
+
+ txreq -url /middle1
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+ txreq -url /middle1q
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+ txreq -url /middle2
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+ txreq -url /middle2q
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+ txreq -url /much
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+ txreq -url /muchq
+ rxresp
+ expect resp.http.X-Test == "remaining=1, remaining"
+
+} -run
+
+# todo
+# - overflowed ws
diff --git a/include/vrt.h b/include/vrt.h
index 3e21a5f..2f5bd21 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -210,6 +210,7 @@ int VRT_switch_config(const char *);
const char *VRT_GetHdr(VRT_CTX, const struct gethdr_s *);
void VRT_SetHdr(VRT_CTX, const struct gethdr_s *, const char *, ...);
+void VRT_RemoveHdrToken(VRT_CTX , const struct gethdr_s *, const char *);
void VRT_handling(VRT_CTX, unsigned hand);
void VRT_hashdata(VRT_CTX, const char *str, ...);
--
2.1.0
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
