echo icon indicating copy to clipboard operation
echo copied to clipboard

Published 20 hours ago • verified publisher icon labstack

Bugs fixed in Gzip middleware

Open rickb777 opened this issue 2 years ago • 2 comments

  1. The Accept-Encoding header parser is more precise. Previously it had been a simple sub-string match, not aware of the possible header grammar.

  2. Downstream middleware or handlers will not gzip the response twice now. This makes the middleware easier to use with packages such as Prometheus (which also implements gzipping).

  3. Vary header is handled more cleanly. It is not sent unless it is needed. It is now removed from 204 or other empty responses. Although this was not strictly wrong, it was unnecessary and wasteful.

rickb777 avatar Dec 14 '21 15:12 rickb777

Any plans to continue working on this PR @rickb777?

lammel avatar Jan 23 '22 23:01 lammel

@lammel I see most of these changes applied to master currently except the removeOneHeaderValue part.

I can continue if there is anything that needs to be done here or otherwise we can close this?

cemremengu avatar Jun 01 '22 07:06 cemremengu

Seems like all changes landed on master already. Only one topic remains that was handled with this PR: Shall the gzip ContentEncoding header be set if the response is empty (res.Size == 0)?

According to RFC 7231:

A sender that generates a message containing a payload body SHOULD generate a Content-Type header field in that message

Although no clear indication is given what should be done if no payload is sent I think this implies we should not add the header if no payload has been written to the response. We might do that small fix on master and close this, as this PR seems rather stale

lammel avatar Dec 04 '22 19:12 lammel

Closing this PR in favor of #2355. Thanks @rickb777 for getting this started.

lammel avatar Dec 06 '22 00:12 lammel