-
-
Notifications
You must be signed in to change notification settings - Fork 241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
option to disable DEFLATE encoding #25
Comments
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 |
@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 |
Sorry, this gets a bit specific to my setup. I am using a CDN, and the CDN inspects the |
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 :)
ah, i see. One idea I was also floating in my head was perhaps to just prefer |
Great, just to be clear all I was proposing was an option to disable an encoding. I'm in complete agreement that the |
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 |
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 |
I have been thinking about the best way to implement this function, and I came up with the idea of adding an option called 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.
The text was updated successfully, but these errors were encountered: