Skip to content

Allow to choose the PREFERRED_ENCODING order #220

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

Open
aannoune opened this issue Feb 12, 2025 · 10 comments
Open

Allow to choose the PREFERRED_ENCODING order #220

aannoune opened this issue Feb 12, 2025 · 10 comments

Comments

@aannoune
Copy link

Hello ,
Since v1.8.0, the preferred compression method is brotli.

I'm using the Compression module in an aws lambda behind Api Gateway but brotli compression is not handled by it.
As most of the clients are now passing gzip, deflate, br as default Accept-Encoding header this cause decompression errors.

It would be nice to have an option that allows to change the order of the PREFERRED_ENCODING array to be able to have gzip as preferred compression method.

@victorsferreira
Copy link

same here. We added a header in the client to force gzip as the compression mode

Accept-Encoding: gzip

@bjohansebas
Copy link
Member

I'm not sure if we want to add that option, and the issue is that we rely on the Accept-Encoding header, which tells us which encodings are accepted. Since Brotli is listed in that header, we assume that Brotli can be used to compress the request.

Another option would be to allow passing false in the configuration to indicate that we don't want Brotli to be available. cc: @UlisesGascon

var compression = require('compression')
var express = require('express')

var app = express()

// compress all responses
app.use(compression({ brotli: false }))

@aannoune
Copy link
Author

I'm not sure if we want to add that option, and the issue is that we rely on the Accept-Encoding header, which tells us which encodings are accepted. Since Brotli is listed in that header, we assume that Brotli can be used to compress the request.

Another option would be to allow passing false in the configuration to indicate that we don't want Brotli to be available. cc: @UlisesGascon

var compression = require('compression')
var express = require('express')

var app = express()

// compress all responses
app.use(compression({ brotli: false }))

That is also a solution : Disabling brotli would set the preferred compression to Gzip.
@victorsferreira it could be a solution if you have hand on the clients. In my case, unfortunately i do not ..
to fix that issue i had to make an extra middleware that filter the Accept-Encoding headers before passing the request to the Compression middleware

@bjohansebas
Copy link
Member

@victorsferreira @aannoune is there any error when compressing the response?

@victorsferreira
Copy link

In our case compression is a dependency of another package and we do not control it.

We added the Header in the API gateway layer asking for a Gzip, because something in the middle wasn't dealing well with the BR response.

In a second application we added compression@1.7.4 as dependency and NPM used it (since it is downgraded) for the other package.

@bjohansebas
Copy link
Member

I still don't really understand what their problems are, whether it's an issue with the package or because their clients are sending the header with Brotli support when they actually don't support it for decompression. More information would be very helpful.

@victorsferreira
Copy link

@bjohansebas we aren't 100% sure what is causing the issue on our application, but it's either the API gateway that is not understanding the response compressed with BR or the WAF that is filtering the packages compressed with BR.

either way it's causing a major problem in our application as the frontend is receiving a binary instead of the JSON response (still compressed).

I believe it's a major change in terms of architecture and infrastructure and I recommend reverting this priority order to what it previously was. Some people will need to fix their applications with urgency when this package automatically updates after a deploy.

@aannoune
Copy link
Author

@bjohansebas : i detected the issue with postman: it does accept brotli encoding , but API gateway returns a binary stream.

User's Browser will send brotli in Accept-Encoding as default.. but api that are behind AWS API gateway with this library will return bad responses

@bjohansebas
Copy link
Member

I don't have much idea of how AWS infrastructure works, but after reading a bit, I understood it better.

Since changing the order of preference at this moment is a breaking change, we could add that option, but you would have to verify that it actually works. As some have mentioned, not everyone has full control over this package to manage the preference, so I'm not sure if it makes sense.

PRs are welcome

@bjohansebas bjohansebas removed their assignment Feb 13, 2025
@aannoune
Copy link
Author

aannoune commented Feb 14, 2025

@bjohansebas ,I'll be glad to test the change anyway
After reflexion, an option to disable a compression type might appear as a more understandable option to most of the users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants