Update: [PATCH 1/2] Add a function to remove an element/token from a header, value (like Accept-Encoding from Vary:)
Poul-Henning Kamp
phk at phk.freebsd.dk
Mon Sep 22 09:37:20 CEST 2014
--------
In message <5419C6B1.9010305 at schokola.de>, Nils Goroll writes:
>With this change, http_RemoveHdrToken also removes any values of an element
>token seperated by equals (like foo=bar)
>
>phk, does this work for you?
Question: What if the target token appears more than once ?
Overall I find the function much more complex than I like. I think it
is because the answer http_GetHdrToken() gives is not what we need to
know.
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.
When we want to do more token-based functions, we should probably make
some foreach_token() infrastructure, but lets wait until #3 case crops
up...
Two security related observations:
>+ if (! (http_GetHdr(hp, hdr, &bp) &&
>+ http_GetHdrToken(hp, hdr, token, &tp)))
>+ return (0);
>+
>+ /* GetHdrToken returns a pointer to the next char after the token */
>+ tl = strlen(token);
Subtracting from pointers is a prime minefield for security issues,
and therefore I generally try to avoid it at all cost. If I have
to do it, I try to plug in asserts which A) show that we know what
we're doing and B) stop us of we were wrong about that.
So here I would add:
assert(tp - tl >= bp);
>+ tp -= tl;
This one is a big NO-NO:
>+ VSLb(hp->vsl, SLT_LostHeader, hdr);
You must *always* supply a const char* format string:
>+ VSLb(hp->vsl, SLT_LostHeader, "%s", hdr);
Otherwise a header with '%' in it will make the printf access random
memory.
--
Poul-Henning Kamp | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG | TCP/IP since RFC 956
FreeBSD committer | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.
More information about the varnish-dev
mailing list