varnish / varnish-modules

Collection of Varnish Cache modules (vmods) by Varnish Software
Other
182 stars 86 forks source link

RFC: dynamic header names #147

Closed nigoroll closed 4 years ago

nigoroll commented 4 years ago

Hi, I would like to ask for feedback / coordination on the following improvement idea:

I need a way to set headers with dynamic names. Until varnish-cache may support such use cases, the obvious place to add this functionality would be the header vmod. A full implementation would probably need all the existing header Functions except for regsub with a HTTP and STRING arguments instead of HEADER. The obvious suggestion would be to add Functions prefixed by dyn_, so vcl could look like

header.dyn_append(req, "MyName", "myval");
header.dyn_copy(req, "MyName", req, "Othername");
myvar.set(header.dyn_get(req, "MyName", "^prefix"));
header.dyn_remove(req, "MyName", "^prefix");

Obviously there are infinitely many alternatives like a separate vmod, optional arguments, whatever.

I would welcome feedback and ideas, preferably better ideas than mine :)

nigoroll commented 4 years ago

never mind, I had a better idea for which I will open a PR

rezan commented 4 years ago

Just an FYI, this VMOD is considered feature complete:

https://github.com/varnish/varnish-modules/blob/master/README.rst#feature-complete-vmods

nigoroll commented 4 years ago

it is up to you if you want to accept PRs or not. But fortunately you did just merge #141 and I do not think that the "feature complete" status makes much sense for a vmod by the name "header".

gquintard commented 4 years ago

Just a heads-up, I'm still supposed to get vmod_header in core via vmod_std, so we may not even have to have this discussion.

On Fri, Nov 29, 2019, 08:12 Nils Goroll notifications@github.com wrote:

it is up to you if you want to accept PRs or not. But fortunately you did just merge #141 https://github.com/varnish/varnish-modules/pull/141 and I do not think that the "feature complete" status makes much sense for a vmod with a name "header".

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/varnish/varnish-modules/issues/147?email_source=notifications&email_token=AA42AKKZZJZNVRGMFTKHNTLQWE5PVA5CNFSM4JS7C4HKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFPGOMY#issuecomment-559834931, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA42AKNTFDQ43ZTPQK6B4BLQWE5PVANCNFSM4JS7C4HA .

nigoroll commented 4 years ago

yes, I support that idea. But I would actually prefer to keep the name header