Add duplicate vary header check

nytimes / gziphandler

Go middleware to gzip HTTP responses

https://godoc.org/github.com/NYTimes/gziphandler

Apache License 2.0

868 stars 129 forks source link

Add duplicate vary header check #97

Closed dtomcej closed 3 years ago

dtomcej commented 4 years ago

This PR:

Adds a check to see if the Vary header has already been set for the response.
Modifies the Vary header if needed.
Sets the header, instead of adding to prevent duplicates.
Adds a test to confirm that the header is properly set with an upstream header present.

This allows this handler to be used in chains where upstream links may set the Vary header on the response before it is returned to this handler.

hhromic commented 4 years ago

Just hit into this problem while combining nytimes/gziphandler with lpar/gzipped (that also sets the Vary header). Thanks for the fix and hopefully gets merged soon!

In my case I'm getting duplicate Vary headers with the same value:

Vary: Accept-Encoding
Vary: Accept-Encoding

numToStr commented 4 years ago

Any ETA when this going to be merged?

tmthrgd commented 4 years ago

There isn’t any big here as it’s perfectly valid to include multiple Vary headers in a response. HTTP provides well defined merge semantics for headers: Vary: a & Vary: b being exactly equivalent to Vary: a, b.

While it is a tad unfortunate to include duplicates, that’s also not a bug and could be filtered out by a higher level wrapper if you really didn’t like having duplicates.

I understand containous/traefik includes code the same as this PR. That code has the same bugs as this PR and should be removed or replaced.