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

Brotli transparent support #443

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
"mocha": true,
},
"globals": {},
"parserOptions": { "ecmaVersion": 8 },
"parserOptions": {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All formatting changes here are automated by the ESlint plugin in my editor

"ecmaVersion": 8
},
"rules": {
"camelcase": [
2,
Expand All @@ -21,8 +23,13 @@
2,
{
"ArrayExpression": 1,
"CallExpression": {"arguments": 1},
"FunctionDeclaration": {"body": 1, "parameters": 2},
"CallExpression": {
"arguments": 1
},
"FunctionDeclaration": {
"body": 1,
"parameters": 2
},
"MemberExpression": 1,
"ObjectExpression": 1,
"SwitchCase": 1
Expand Down Expand Up @@ -61,10 +68,13 @@
"no-undef": 2,
"no-unused-vars": 2,
"no-eq-null": 2,
"space-before-function-paren": ["error", {
"space-before-function-paren": [
"error",
{
"anonymous": "always",
"named": "never"
}],
}
],
"no-empty": [
2,
{
Expand Down Expand Up @@ -150,7 +160,10 @@
],
"lines-around-comment": [
2,
{ "afterLineComment": true, "allowBlockEnd": true }
{
"afterLineComment": false,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

"allowBlockEnd": true
}
],
"semi": [
2,
Expand All @@ -165,4 +178,4 @@
"unix"
]
}
}
}
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,12 @@ instance, but this is not a reliable interface. I expect to close this
exploit in a future release, while providing an additional hook for mutating
the userRes before sending.

##### gzip responses
##### gzip or Brotli responses

If your proxy response is gzipped, this program will automatically unzip
it before passing to your function, then zip it back up before piping it to the
user response. There is currently no way to short-circuit this behavior.
If your proxy response is gzipped or Brotli-encoded, this program will
automatically unzip it before passing to your function, then zip it back up
before piping it to the user response. There is currently no way to
short-circuit this behavior.

#### limit

Expand Down
80 changes: 52 additions & 28 deletions app/steps/decorateUserRes.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,54 @@
var as = require('../../lib/as.js');
var debug = require('debug')('express-http-proxy');
var zlib = require('zlib');
var brotli = require('iltorb');

function isResGzipped(res) {
return res.headers['content-encoding'] === 'gzip';
var ENCODING = {
gzip: 'gzip',
brotli: 'br'
};

function isResEncoded(res, encoding) {
return res.headers['content-encoding'] === encoding;
}

function zipOrUnzip(method) {
return function(rspData, res) {
const id = data => Promise.resolve(data);

function createGzipHandler(method) {
return function (data) {
return new Promise(function (resolve, reject) {
if (isResGzipped(res) && rspData.length) {
zlib[method](rspData, function(err, buffer) {
if(err) {
reject(err);
} else {
resolve(buffer);
}
});
} else {
resolve(rspData);
if (!data || !data.length) {
return resolve(data);
}
zlib[method](data, function (err, buffer) {
if (err) {
reject(err);
} else {
resolve(buffer);
}
});

});
};
}

var maybeUnzipPromise = zipOrUnzip('gunzip');
var maybeZipPromise = zipOrUnzip('gzip');
function createEncodingHandler(res) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously, this is the central part

if (isResEncoded(res, ENCODING.gzip)) {
return {
decode: createGzipHandler('gunzip'),
encode: createGzipHandler('gzip')
};
} else if (isResEncoded(res, ENCODING.brotli)) {
return {
decode: brotli.decompress,
encode: brotli.compress
};
}
return {
decode: id,
encode: id
};
}

function verifyBuffer(rspd, reject) {
if (!Buffer.isBuffer(rspd)) {
Expand All @@ -37,11 +60,11 @@ function verifyBuffer(rspd, reject) {

function updateHeaders(res, rspdBefore, rspdAfter, reject) {
if (!res.headersSent) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automatically formatted, it looks like 4 spaces were used

res.set('content-length', rspdAfter.length);
res.set('content-length', rspdAfter.length);
} else if (rspdAfter.length !== rspdBefore.length) {
var error = '"Content-Length" is already sent,' +
'the length of response data can not be changed';
return reject(new Error(error));
var error = '"Content-Length" is already sent, ' +
'the length of response data can not be changed';
return reject(new Error(error));
}
}

Expand All @@ -52,31 +75,32 @@ function decorateProxyResBody(container) {
return Promise.resolve(container);
}

var proxyResDataPromise = maybeUnzipPromise(container.proxy.resData, container.proxy.res);
var proxyRes = container.proxy.res;
var req = container.user.req;
var res = container.user.res;
var originalResData;
var originalResData;

if (res.statusCode === 304) {
debug('Skipping userResDecorator on response 304');
return Promise.resolve(container);
}

return proxyResDataPromise
.then(function(proxyResData){
const encodingHandler = createEncodingHandler(container.proxy.res);

return encodingHandler.decode(container.proxy.resData)
.then(function (proxyResData) {
originalResData = proxyResData;
return resolverFn(proxyRes, proxyResData, req, res);
})
.then(function(modifiedResData) {
return new Promise(function(resolve, reject) {
.then(function (modifiedResData) {
return new Promise(function (resolve, reject) {
var rspd = as.buffer(modifiedResData, container.options);
verifyBuffer(rspd, reject);
updateHeaders(res, originalResData, rspd, reject);
maybeZipPromise(rspd, container.proxy.res).then(function(buffer) {
encodingHandler.encode(rspd).then(function (buffer) {
container.proxy.resData = buffer;
resolve(container);
}).catch(function(error){
}).catch(function (error) {
reject(error);
});
});
Expand Down
4 changes: 2 additions & 2 deletions lib/as.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else if (typeof body === 'string') {
ret = new Buffer(body, options.reqBodyEncoding);
ret = Buffer.from(body, options.reqBodyEncoding);
}
return ret;
}
Expand Down
Loading