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 df09f64cbc2e082fb94831a47cb416ba6711f575 Mon Sep 17 00:00:00 2001 From: Nils Goroll <[email protected]> Date: Sun, 28 Sep 2014 19:19:34 +0200 Subject: [PATCH] prototype - header element parsing --- bin/varnishd/cache/cache.h | 24 +++ 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, 549 insertions(+) create mode 100644 bin/varnishtest/tests/removehdrtoken.vtc diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 238baa1..90c1d3a 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -134,6 +134,11 @@ typedef struct { char *e; } txt; +typedef struct { + const char *b; + const char *e; +} ctxt; + /*--------------------------------------------------------------------*/ enum sess_step { @@ -877,6 +882,8 @@ void http_Teardown(struct http *ht); int http_GetHdr(const struct http *hp, const char *hdr, char **ptr); int http_GetHdrToken(const struct http *hp, const char *hdr, const char *token, 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, char **ptr); double http_GetHdrQ(const struct http *hp, const char *hdr, const char *field); @@ -1194,6 +1201,15 @@ Tcheck(const txt t) assert(t.b <= t.e); } +static inline void +cTcheck(const ctxt t) +{ + + AN(t.b); + AN(t.e); + assert(t.b <= t.e); +} + /* * unsigned length of a txt */ @@ -1206,6 +1222,14 @@ Tlen(const txt t) return ((unsigned)(t.e - t.b)); } +static inline unsigned +cTlen(const ctxt t) +{ + + cTcheck(t); + return ((unsigned)(t.e - t.b)); +} + static inline void Tadd(txt *t, const char *p, int l) { diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c index 15605e1..3e699ba 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 */ + ctxt *el; + ctxt *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, char *h); +int HTTPHDR_Join(struct httphdr *hphdr, const ctxt *hdrn, const ctxt *sep, + char *buf, int space); +int HTTPHDR_SameToken(const ctxt *a, const ctxt *b); +int HTTPHDR_RemoveToken(struct httphdr *hphdr, const ctxt *tok); + +/* END XXX .h */ + +unsigned +HTTPHDR_estimate(unsigned sel) +{ + return (PRNDUP(sizeof (struct httphdr) + sizeof(ctxt) * 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(ctxt) * sel); + hphdr->val = (void *)(hphdr->el + sel); + memset(hphdr->val, 0, sizeof(ctxt) * 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, 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 = cTlen(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 ctxt *hdrn, const ctxt *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 ctxt *a, const ctxt *b) +{ + unsigned al = cTlen(*a); + unsigned bl = cTlen(*b); + int aq, bq; + ctxt 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 ctxt *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 *sep = ", "; + + char *h, *n; + const ctxt tokt = { .b = token, + .e = token + strlen(token) }; + + const ctxt hdrt = { .b = hdr + 1, + .e = hdr + 1 + *hdr }; + + const ctxt 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 b45cdf9..3335081 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..41e363a --- /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
