compression
compression copied to clipboard
expressjs
Added support for brotli ('br') content-encoding
https://github.com/expressjs/body-parser/pull/403
https://medium.com/oyotech/how-brotli-compression-gave-us-37-latency-improvement-14d41e50fee4 https://caniuse.com/#feat=brotli
@dougwilson There's an issue with object-assign polyfill - it does not support the very outdated versions of node.js
There's an issue with object-assign polyfill - it does not support the very outdated versions of node.js
I guess just pick a different module or an older version of that module.
I added a comment about the faster statement and still have an open question on how a user can change compression level of br. I'm also having trouble to actually get br compression to even work at all with Chrome. I'm trying to figure it out, as our number 1 issue opened here is how to get this module working, so having concrete information for how to get br working with a web browser (Chrome, for instance) would help a lot. For reference I used the example in the README and your branch and Chrome continues to only show it using gzip, even when the connection is https (which my understanding is a requirement for br to work in Chrome).
Look at Chrome's Accept-Encoding: gzip, deflate, br - it puts br last. I'm guessing they put it last as a transition period, as some poor servers out there fail when brotli is specified, or intermediaries doing even worse.
Or maybe it's because when it first arrived they considered it a good compression for WOFF fonts, but they always tested with the highest compression levels. Today people know that with level 4 you have better results on all kinds of files.
So what clients will end up using br with this PR I can test?
If you'll investigate the headers of css/js files in this url, you'll see that they're encoded with brotli. They just prioritize brotli over gzip server-side. Which is the right thing to do, like we already do for gzip over deflate.
What I mean to say is, that you can temporarily disable gzip in the code or prioritize br, to make chrome communicate with your server using brotli.
@danielgindi I agree with you.
- Cloudflare, the largest CDN in the world, prioritizes brotli over gzip at server-side.
- koa-compress has recently added brotli support and they prioritize brotli over gzip server-side too. https://github.com/koajs/compress/blob/80054a551f898d371c60dcb17b749da61cf02b5a/lib/encodings.js#L27
Hi @NathanielHill I am just waiting on the changes referenced above, specifically, when I tested this PR, it did not result in br responses in any of the major web browsers (Chrome, Firefox) that support br, so I'm kind of at a loss for what value this change would bring or if it is broken.
If this change is not going to actually result in the (better) br compression in the web browsers, perhaps someone can better explain what the purpose of this PR is? FWIW, the Express middleware shrinkray does result in br in those web browsers, so it's certainly possible to achieve.
I think this needs a test case for when "Accept-Encoding: gzip, deflate, br". Seems like maybe the code is causing gzip to be preferred so there's no difference in modern browsers (as discussed above).
Hi @NathanielHill I am just waiting on the changes referenced above, specifically, when I tested this PR, it did not result in br responses in any of the major web browsers (Chrome, Firefox) that support br, so I'm kind of at a loss for what value this change would bring or if it is broken.
If this change is not going to actually result in the (better) br compression in the web browsers, perhaps someone can better explain what the purpose of this PR is? FWIW, the Express middleware
shrinkraydoes result in br in those web browsers, so it's certainly possible to achieve.
Of course it's possible to "achieve". But I did not want to presume that I can change the prioritization here. Gzip is currently prioritized in the code.
Currently browsers do not put br first, but there are other clients (i.e api requests between servers).
If this is what you want (which is not clear at all, as you just kept your silence about this), it's easy to achieve.
Hi @danielgindi sorry if I wasn't clear. I tried to start that conversation in https://github.com/expressjs/compression/pull/172#pullrequestreview-447507768 but you never seemed to answer my question. So perhaps we should circle back around to that, and can you provide what I asked for? :
I'm also having trouble to actually get br compression to even work at all with Chrome. I'm trying to figure it out, as our number 1 issue opened here is how to get this module working, so having concrete information for how to get br working with a web browser (Chrome, for instance) would help a lot.
You never gave the name of any web browser where this actually works. The number 1 issue here is how to get this module working. If this doesn't actually work out of the box, then we need to understand why and determine what (if any) changes need to be made and what the concrete value it adds.
Perhaps we can also ask the folks that commented here: @Fonger would you expect this PR to get br in a web browser? @NathanielHill is that the same expectation you have as well and want to see released?
@dougwilson Yes, I would expect a server to send br over gzip if both are present in Accept-Encoding
@dougwilson Yes
If I see accept-encoding gzip, defalte, br, which is what modern browsers currently use, this middleware should use br instead of gzip. And that is how koa-compression does.
Hi @danielgindi sorry if I wasn't clear. I tried to start that conversation in https://github.com/expressjs/compression/pull/172#pullrequestreview-447507768 but you never seemed to answer my question. So perhaps we should circle back around to that, and can you provide what I asked for? :
Well I did answer that, but perhaps the answer is not so simple. By standard you should respect the requested prioritization, and currently browsers prefer gzip for some reason. I guess there is an assumption that a lot of servers do not support br, so br was added without much thought.
Anyway this module already re-prioritizes gzip over deflate. So if that's what we're doing, we should also do this for br over the rest.
@dougwilson I've added a commit that re-prioritizes the encodings to br-gzip-deflate. Will work with any browser now.
@dougwilson are you available to re-review this PR? I'd love to see this get merged sooner rather than later. Thanks for all you do!
Hi @nicksrandall yes, I am. It is on my to-do list, but between @danielgindi 's note and now, there have been 3 security vulnerability reports against Express that we are currently working through, which is much higher priority and has our focus at this time. Apologies.
A quick test seems to have a weird issue, in that the header accept-encoding: gzip;q=1, br;q=0.3 it sending back br instead of gzip. I would think it should be honoring the quality indicators in the negotiation for the encoding.
@danielgindi Thanks for all your work on this. Let me know if you'd like any assistance on addressing the latest issue @dougwilson reported. I'm happy to help where I can.
@danielgindi Thanks for all your work on this. Let me know if you'd like any assistance on addressing the latest issue @dougwilson reported. I'm happy to help where I can.
Well if you want to make the explicit quality indicator take precedence, then by all means... :) I'm kind of busy right now and only reprioritized these because @dougwilson insisted that he does not see any any effect with current browsers.
I'll give you permissions to the repo, so you can complicate things as you wish! ;)
IMO, I think quality indicators support can be released in the upcoming version because most browser does not use it for this moment.
We of course all know from the above conversation that web browser are not the only clients out there. The PR itself was originally never even going to work for browsers, so it seems that the primary use case was for the non-browsers and not the browsers.
Yeah. You're right and I find koa-compress also support quality indicator. We should always follow the standard.
Yea. I was pointing out the quality stuff just looking at the new commit (it was a change from the previous behavior). I would guess we should define it as "if br is at the highest accepted quality, then prefer that over the others, regardless of ordering". As in gzip, br = br and gzip;q=0.8, br;q=0.8 = br, but gzip, br;q=0.8 = gzip. It seems like (reading through the old Chrome issues) that Chrome put br last knowing servers would not serve it if they followed the standard, as it was "experimental" it... it's still at the end of the list as least preferred even today, and not sure why.
I will make a change so that if brotli and gzip are preferred equally (regardless of order) the system will prefer brotli. Otherwise, the system will prefer the more preferred compression algorithm. Sound reasonable?
My thinking is to not reinvent the wheel so I've pulled in koa-compress' logic to determine which content encoding header is preferred as it conforms the rules we've specified above. I've also added a test case to verify.
My only concern here is node version support. What is the official node version support for this lib?