lpar / gzipped

Replacement for golang http.FileServer which supports precompressed static assets.
BSD 3-Clause "New" or "Revised" License
94 stars 15 forks source link

identity should not be used in content-encoding header #10

Closed hhromic closed 4 years ago

hhromic commented 4 years ago

Hi, at the moment lpar/gzipped is setting the Content-Encoding header in the response according to the detected filetype found:

https://github.com/lpar/gzipped/blob/1a8abffd89c54062a03d50ac0093851dd55bcab6/fileserver.go#L109-L118

This is okay for gzip and br encodings but not for identity.

RFC 2616 (HTTP) states that ìdentity should not be used in Content-Encoding but only in the Accept-Encoding header [1] for negotiation:

identity
    The default (identity) encoding; the use of no transformation
    whatsoever. This content-coding is used only in the Accept-
    Encoding header, and SHOULD NOT be used in the Content-Encoding
    header.

The concrete problem with this is that when combining lpar/gzipped together with nytimes/gziphandler. The latter will not compress any response whose Content-Encoding header is already set by an inner middleware, i.e. by lpar/gzipped.

This is logical if lpar/gzipped already is returning pre-compressed content. However due to lpar/gzipped setting Content-Encoding: identity for unknown compressed content effectively prevents nytimes/gziphandler to further process the response, i.e. potentially compressing it on-the-fly.

This means that it is not possible to combine both to serve pre-compressed content and on-the-fly compressed content at the same time as suggested in the README file.

The solution is simply to not set any Content-Encoding if the found file is not handled by lpar/gzipped already, so it can be delegated to the next middleware. If you agree with this I can put together a PR.

[1] https://tools.ietf.org/html/rfc2616#section-3.5

lpar commented 4 years ago

I assumed anyone wanting to use both lpar/gzipped and nytimes/gziphandler would be using them on different handler paths, rather than layered!

Still, it's a bug that it sets identity in Content-Encoding, and a patch for that would be more than welcome.

hhromic commented 4 years ago

I'll work on it then!

Yes we are using them both in a tree of handlers to achieve the best of both worlds for all routes in the application: pre-compressed static content being served by ldap/gzipped and on-the-fly dynamic compression being done globally by nytimes/gziphandler.

rootHandler (gziphandler)
     |__ fileHandler (gzipped)
     |__ apiHandler (JSON responses)
     \__ anotherApiHandler (JSON responses)

This gives us plenty of flexibility because gziphandler will compress any text-based response no matter from which handler it comes from. With gzipped we can have a set of static assets pre-compressed and uncompressed on disk (great for debugging) and all is served compressed in the end.