fix(gzip): Make sure Vary header is always added if a response can be compressed#2865
Conversation
|
couldn't we just move the |
|
Unfortunately, it also needs to be added when the accept-encoding header is not gzip. |
I understand that, that's the goal of this PR. Can't we really avoid this huge diff just to move that up? |
|
My thinking was I would need to implement a responder for non-gzip encodings, and I also wanted to use the same logic for determining if I shouldn't add the Vary header (body is too small, or streaming response with content-encoding already set). Anyway, I'll try to get it smaller. |
|
hold, I'm going to review carefully. No need to try anything for now. Thanks for the PR. |
|
This PR is actually changing the behavior on when we do the compression. It actually implements what is proposed here: #2753 |
|
Without flushing, the tests would fail. |
That's a change in behavior. I don't think it should flush on every chunk. |
ee2652d to
f62e152
Compare
Fixes #2863
Summary
Since the presense or absense of the
Accept-Encodingheader can change the response body,Varyshould always be added whether the client requested gzip encoding or not. Otherwise some caching implementations might return a non-compressed response even when a compressed one was requested.This change may seem a bit complex, but it reworks the
GZipMiddlewareto behave the same no matter what the compression is (or lack of). Basic changes include:apply_compressionand don't update thecontent-encodingorcontent-lengthheaders if the body didn't change.IdentityResponderfromGZipResponderwhich will be used to respond to non-gzip requests. Itsapply_compressionmethod will return the body unchanged.Checklist