Skip to content
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

Open
gfemec opened this issue Jan 1, 2015 · 12 comments · May be fixed by #27
Open

option to disable DEFLATE encoding #25

gfemec opened this issue Jan 1, 2015 · 12 comments · May be fixed by #27
Assignees
Milestone

Comments

@gfemec
Copy link

gfemec commented Jan 1, 2015

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.

@dougwilson
Copy link
Contributor

It's a great idea! I agree you should be able to, at minimum, decide which of the built-in encoding to enable.

@Fishrock123
Copy link
Contributor

I thought it would only use deflate as a last resort and when the client accepted it?

@dougwilson
Copy link
Contributor

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).

@Fishrock123
Copy link
Contributor

@gfemec can you provide details on the browser setup that causes this?

@dougwilson
Copy link
Contributor

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).

@gfemec
Copy link
Author

gfemec commented Jan 1, 2015

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.

@dougwilson
Copy link
Contributor

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).

@gfemec
Copy link
Author

gfemec commented Jan 1, 2015

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?

@dougwilson
Copy link
Contributor

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 :)

gfemec pushed a commit to gfemec/compression that referenced this issue Jan 1, 2015
gfemec pushed a commit to gfemec/compression that referenced this issue Jan 1, 2015
@dougwilson dougwilson self-assigned this Jan 1, 2015
gfemec pushed a commit to gfemec/compression that referenced this issue Jan 1, 2015
gfemec pushed a commit to gfemec/compression that referenced this issue Jan 1, 2015
@gfemec gfemec linked a pull request Jan 1, 2015 that will close this issue
@dougwilson
Copy link
Contributor

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.

@dantman
Copy link

dantman commented Feb 1, 2016

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.

@bjohansebas
Copy link
Member

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.

@bjohansebas bjohansebas added this to the 2.0 milestone Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants