Open gfemec opened 9 years ago
It's a great idea! I agree you should be able to, at minimum, decide which of the built-in encoding to enable.
I thought it would only use deflate as a last resort and when the client accepted it?
Right. AFAIK, IE always sends Accept-Encoding: gzip, deflate
, and this module prefers gzip
over deflate
, which means that IE, by default, will never get a deflate response (which probably also explains why no one has ever brought this up in this module before, especially since it has been like this since at least 2012).
@gfemec can you provide details on the browser setup that causes this?
I also went ahead and read about this whole deflate thing with wrapped vs unwrapped stream and was thinking perhaps to just emit an unwrapped stream, but changed my mind when I read the latest HTTP RFCs (RFC 7230 section 4.2.2: https://tools.ietf.org/html/rfc7230#section-4.2.2) which made it clear which format was the "conforming" one.
One thing we could potentially do is prefer gzip
over deflate
, even if the client wants deflate
more (or, at least, do it when both gzip
and deflate
are preferred by the client at the same quality level).
Sorry, this gets a bit specific to my setup. I am using a CDN, and the CDN inspects the Accept-Encoding
header to determine which cached responses to send to the client. On cache miss the CDN modifies the request to my origin to include Accept-Encoding: deflate;q=1.000, gzip;q=0.999, identity;q=0.001
(ostensibly because deflate is faster than gzip) which receives a deflate'd response. Then the CDN serves this cached response to all browsers that claim to accept deflate which causes the problem on IE. I realize I could work with the CDN to change this behavior but in researching this I found a common recommendation was to disable DEFLATE entirely.
http://stackoverflow.com/questions/388595/why-use-deflate-instead-of-gzip-for-text-files-served-by-apache
http://zoompf.com/blog/2012/02/lose-the-wait-http-compression (GZIP vs DEFLATE)
No worries if you'd rather not add complexity for this edge use-case, I have already fixed my issue by modifying the accept header from the CDN but I just figured it might be a useful option to have for this middleware.
but I just figured it might be a useful option to have for this middleware.
It's no problem. I 100% agree that you should be able to turn certain encodings off, as per your original request :) I'm really glad, through, that @Fishrock123 got you to explain a little more, because ideally, we'd also like to make it work better :)
modifies the request to my origin to include Accept-Encoding: deflate;q=1.000, gzip;q=0.999, identity;q=0.001
ah, i see. One idea I was also floating in my head was perhaps to just prefer gzip
no matter what, as long as the client said it wanted it (or, conversely, only use deflate
as a last resort).
Great, just to be clear all I was proposing was an option to disable an encoding. I'm in complete agreement that the deflate
responses are correct as they are and also think the way the Accept-Encoding
header is being interpreted is correct. Would you like a PR that adds an option to specify a list of allowed encodings?
Would you like a PR that adds an option to specify a list of allowed encodings?
Yes, if you're willing. I'm' going to implement this anyway, so don't feel pressured to make one :)
So to be clear: I agree that the user should be able to choose which encodings to enable/disable (i.e. you should be able to only allow gzip, if you wish). All other discussion in here is just in general, stemmed from your original feature request, in the interest in just making the module better in general :)
Ok, as of version 1.4.0, your original issue should be no more, as we'll still send gzip
coding, even to that proxy. As for controlling, I have an API that is very flexible, but is not backwards-compatible, so I'm pushing it to v2.
Side topic, if you need a workaround; theoretically you should be able to insert a simple bit of middleware before compression that simply edits the Accept-Encoding
header and removes the deflate
algorithm from it. Then the compression middleware will never think a request supports deflate compression.
I have been thinking about the best way to implement this function, and I came up with the idea of adding an option called encodings
, which would support gzip, deflate, and in the future brotli. It would also allow customization of their options, such as flush, levels, and strategy
var compression = require('compression')
var express = require('express')
var app = express()
app.use(compression({
encodings: {
gzip: {
flush: require('zlib').constants.Z_SYNC_FLUSH
},
deflate: {
flush: require('zlib').constants.Z_SYNC_FLUSH,
}
}))
This would simplify the options and offer greater customization when using the method to encode the response. However, it would mean that this feature would be for version 2 of the package.
I ran into a problem serving compressed requests to Internet Explorer because of the zlib wrapper included with DEFLATE'd responses which, while correct according to the RFC, is not supported by IE. https://connect.microsoft.com/IE/feedback/details/1007412/interop-wininet-does-not-support-content-encoding-deflate-properly . In lieu of doing some user-agent detection or always removing the wrapper, it would be nice to have an option to disable DEFLATE usage in favor of gzip. I'm completely willing to submit a PR but wanted to open an issue first for discussion.