Vary header in compression#399
Vary header in compression#399jplatte merged 1 commit intotower-rs:mainfrom Dragonink:compression-vary
Conversation
|
I wonder if we should also send the header in some of the conditions where we early return without a content-encoding, to indicate to caches that a different response body is available when accept-encoding is set. Wdyt? |
|
So we always send the Vary header whatever the body will be? You're right, we should do this. I have a question about branches: why has the |
No, not in every case. If
It's okay to make PRs against either. I guess if you want this PR to be released (soon-ish) as part of 0.4.x, it makes sense to make the PR against that. There's (so far) only a tiny difference between the branches, so it will be trivial to cherry-pick changes such that they affect both branches. |
|
So we should always send the Vary except when |
|
No, I think there's another early-return branch (body already compressed, i.e. cc @davidpdrsn @Nehliin wdyt about when we should set |
|
The Content-Encoding already set is tested in let should_compress = !res.headers().contains_key(header::CONTENT_ENCODING)
&& self.predicate.should_compress(&res);Thus if I guard the Vary header with an |
|
Ping @davidpdrsn @Nehliin any opinions about this PR / the question of when Also @Dragonink have you looked into the test failure? (MSRV job failing was fixed in #418) |
|
sorry won't have time to review in the near future |
Motivation
From the "Compression in HTTP" MDN page:
Solution
We just append
Accept-Encodingto theVaryheader when constructing the compression response.