varnish / varnish-modules

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

Xkey is not able to handle multiple message-header fields with the same field-name #102

Closed hugocruz closed 6 years ago

hugocruz commented 6 years ago

A backend sometimes transforms:

XKey: 1234 
XKey: 5678

into

XKey: 1234,5678

From the RFC 2616 this should be allowed.

Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.

Of course a VCL workaround can handle this, but shouldn't this be allowed by Xkey?

tavisma commented 6 years ago

would also like to see support for this, makes supporting both XKEY and Fast.ly Surrogate-Keys easier

SpikesDivZero commented 6 years ago

Hi @gquintard, I see this issue is currently assigned to you. Seeing as this issue's been open for a while now, would you mind if I put up a PR for review that I believe accomplishes this goal?

I do have some secondary concerns with regards to how this could be a breaking change (it's feasible that some users may be using comma as a key joiner), but I'm inclined to think that if properly documented or announced, this is an acceptable change in behavior to bring the VMOD closer to RFC specs.

gquintard commented 6 years ago

By all means, please propose a PR, this is not on my priority list right now, but I will be glad to discuss it with you.

I'm not sure I see this as a breaking change, but let's worry about that once we have some code

On Sun, Oct 21, 2018, 13:18 Spikes notifications@github.com wrote:

Hi @gquintard https://github.com/gquintard, I see this issue is currently assigned to you. Seeing as this issue's been open for a while now, would you mind if I put up a PR for review that I believe accomplishes this goal?

I do have some secondary concerns with regards to how this could be a breaking change (it's feasible that some users may be using comma as a key joiner), but I'm inclined to think that if properly documented or announced, this is an acceptable change in behavior to bring the VMOD closer to RFC specs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/varnish/varnish-modules/issues/102#issuecomment-431700580, or mute the thread https://github.com/notifications/unsubscribe-auth/ADmgKc81VJzJRskpkK6d1ZD2DhGQvKCLks5unNamgaJpZM4R84i7 .