Brotli transparent support #443
Conversation
| @@ -7,7 +7,9 @@ | |||
| "mocha": true, | |||
| }, | |||
| "globals": {}, | |||
| "parserOptions": { "ecmaVersion": 8 }, | |||
| "parserOptions": { | |||
There was a problem hiding this comment.
All formatting changes here are automated by the ESlint plugin in my editor
| @@ -150,7 +160,10 @@ | |||
| ], | |||
| "lines-around-comment": [ | |||
| 2, | |||
| { "afterLineComment": true, "allowBlockEnd": true } | |||
| { | |||
| "afterLineComment": false, | |||
There was a problem hiding this comment.
I've changed this because the existing code wasn't following this rule (and also I think it's fairly common to have no empty line between a single line comment and the next line it explains)
| @@ -37,11 +60,11 @@ function verifyBuffer(rspd, reject) { | |||
|
|
|||
| function updateHeaders(res, rspdBefore, rspdAfter, reject) { | |||
| if (!res.headersSent) { | |||
There was a problem hiding this comment.
Automatically formatted, it looks like 4 spaces were used
| }); | ||
| }; | ||
| } | ||
|
|
||
| var maybeUnzipPromise = zipOrUnzip('gunzip'); | ||
| var maybeZipPromise = zipOrUnzip('gzip'); | ||
| function createEncodingHandler(res) { |
There was a problem hiding this comment.
Obviously, this is the central part
| @@ -10,9 +10,9 @@ function asBuffer(body, options) { | |||
| if (Buffer.isBuffer(body)) { | |||
| ret = body; | |||
| } else if (typeof body === 'object') { | |||
| ret = new Buffer(JSON.stringify(body), options.reqBodyEncoding); | |||
| ret = Buffer.from(JSON.stringify(body), options.reqBodyEncoding); | |||
There was a problem hiding this comment.
Buffer constructor is deprecated and the from method is available in node v6+ so it shouldn't be a breaking change. Fixed it to remove the warnings during the tests
| var request = require('supertest'); | ||
| // superagent/supertest don't support `br` encoding |
| @@ -169,13 +176,111 @@ describe('userResDecorator', function () { | |||
| request(proxyApp) | |||
| .get('/') | |||
| .expect(function (res) { | |||
| res.headers.location.match(/localhost:3000/); | |||
| assert(res.headers.location.match(/localhost:3000/)); | |||
There was a problem hiding this comment.
I assume it was missing here. Didn't check other tests
| beforeEach(function () { | ||
| app = express(); | ||
| target = express(); | ||
| target.use(compression({ threshold: 0 })); |
There was a problem hiding this comment.
The threshold ensures the small payload is still gzip-encoded
|
|
||
| request(app) | ||
| .get('/proxy/test') | ||
| .set('Accept-Encoding', 'gzip') |
| // with the original binary data | ||
| encoding: null, | ||
| headers: { | ||
| 'Accept-Encoding': 'br' |
|
Huh, the |
I'll leave a couple of comments to justify some non-obvious changes