From 7c2dc40e25b1f334135ba0edcea6076d81db4a05 Mon Sep 17 00:00:00 2001 From: Fabian Ehrentraud Date: Sat, 17 Aug 2024 08:13:21 +0200 Subject: [PATCH] avoid overwriting userRes headers when using userResHeaderDecorator fixes https://github.com/villadora/express-http-proxy/issues/547 --- README.md | 2 + app/steps/copyProxyResHeadersToUserRes.js | 22 --- app/steps/decorateUserResHeaders.js | 13 ++ index.js | 2 - test/decorateUserResHeaders.js | 163 +++++++++++++++------- 5 files changed, 125 insertions(+), 77 deletions(-) delete mode 100644 app/steps/copyProxyResHeadersToUserRes.js diff --git a/README.md b/README.md index c306c584..47d0cbb2 100644 --- a/README.md +++ b/README.md @@ -285,6 +285,8 @@ app.use('/proxy', proxy('www.google.com', { })); ``` +Note that by default, headers from the proxy response override headers that have previously been set on the user response. With the `userResHeaderDecorator` this can be worked around by combining the headers from `userRes` and `proxyRes` manually. + #### decorateRequest diff --git a/app/steps/copyProxyResHeadersToUserRes.js b/app/steps/copyProxyResHeadersToUserRes.js deleted file mode 100644 index 16bc872a..00000000 --- a/app/steps/copyProxyResHeadersToUserRes.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; - -function copyProxyResHeadersToUserRes(container) { - return new Promise(function (resolve) { - var res = container.user.res; - var rsp = container.proxy.res; - - if (!res.headersSent) { - res.status(rsp.statusCode); - Object.keys(rsp.headers) - .filter(function (item) { return item !== 'transfer-encoding'; }) - .forEach(function (item) { - res.set(item, rsp.headers[item]); - }); - } - - resolve(container); - }); -} - -module.exports = copyProxyResHeadersToUserRes; - diff --git a/app/steps/decorateUserResHeaders.js b/app/steps/decorateUserResHeaders.js index acb4f530..302e22bd 100644 --- a/app/steps/decorateUserResHeaders.js +++ b/app/steps/decorateUserResHeaders.js @@ -3,9 +3,22 @@ var getHeaders = require('../../lib/getHeaders'); function decorateUserResHeaders(container) { var resolverFn = container.options.userResHeaderDecorator; + var res = container.user.res; + var rsp = container.proxy.res; + var headers = getHeaders(container.user.res); + if (!res.headersSent) { + res.status(rsp.statusCode); + Object.keys(rsp.headers) + .filter(function (item) { return item !== 'transfer-encoding'; }) + .forEach(function (item) { + headers[item] = rsp.headers[item]; + }); + } + if (!resolverFn) { + res.set(headers); return Promise.resolve(container); } diff --git a/index.js b/index.js index cc1f4152..c572da06 100644 --- a/index.js +++ b/index.js @@ -9,7 +9,6 @@ var assert = require('assert'); var debug = require('debug')('express-http-proxy'); var buildProxyReq = require('./app/steps/buildProxyReq'); -var copyProxyResHeadersToUserRes = require('./app/steps/copyProxyResHeadersToUserRes'); var decorateProxyReqBody = require('./app/steps/decorateProxyReqBody'); var decorateProxyReqOpts = require('./app/steps/decorateProxyReqOpts'); var decorateUserRes = require('./app/steps/decorateUserRes'); @@ -39,7 +38,6 @@ module.exports = function proxy(host, userOptions) { .then(prepareProxyReq) .then(sendProxyRequest) .then(maybeSkipToNextHandler) - .then(copyProxyResHeadersToUserRes) .then(decorateUserResHeaders) .then(decorateUserRes) .then(sendUserRes) diff --git a/test/decorateUserResHeaders.js b/test/decorateUserResHeaders.js index 6fd925ba..e41c2efc 100644 --- a/test/decorateUserResHeaders.js +++ b/test/decorateUserResHeaders.js @@ -5,73 +5,130 @@ var express = require('express'); var request = require('supertest'); var proxy = require('../'); -describe('when userResHeaderDecorator is defined', function () { +describe('response headers', function () { + describe('when userResHeaderDecorator is defined', function () { - this.timeout(10000); + this.timeout(10000); - var app; - var serverReference; + var app; + var serverReference; - afterEach(function () { - serverReference.close(); - }); + afterEach(function () { + serverReference.close(); + }); - beforeEach(function () { - app = express(); - var pTarget = express(); - pTarget.use(function (req, res) { - res.header('x-my-not-so-secret-header', 'minnie-mouse'); - res.header('x-my-secret-header', 'mighty-mouse'); - res.json(req.headers); + beforeEach(function () { + app = express(); + var pTarget = express(); + app.use(function (req, res, next) { + res.cookie('app-set-cookie1', 'app-value1'); + res.cookie('app-set-cookie2', 'app-value2'); + next(); + }); + pTarget.use(function (req, res) { + res.header('x-my-not-so-secret-header', 'minnie-mouse'); + res.header('x-my-secret-header', 'mighty-mouse'); + res.cookie('pTarget-set-cookie1', 'pTarget-value1'); + res.cookie('pTarget-set-cookie2', 'pTarget-value2'); + res.json(req.headers); + }); + serverReference = pTarget.listen(12345); }); - serverReference = pTarget.listen(12345); - }); - afterEach(function () { - serverReference.close(); - }); + afterEach(function () { + serverReference.close(); + }); - it('can delete a header', function (done) { + it('can delete a header', function (done) { + + app.use('/proxy', proxy('http://127.0.0.1:12345', { + userResHeaderDecorator: function (headers /*, userReq, userRes, proxyReq, proxyRes */) { + delete headers['x-my-secret-header']; + return headers; + } + })); + + app.use(function (req, res) { + res.sendStatus(200); + }); + + request(app) + .get('/proxy') + .expect(function (res) { + assert(Object.keys(res.headers).indexOf('x-my-not-so-secret-header') > -1); + assert(Object.keys(res.headers).indexOf('x-my-secret-header') === -1); + }) + .end(done); + }); - app.use('/proxy', proxy('http://127.0.0.1:12345', { - userResHeaderDecorator: function (headers /*, userReq, userRes, proxyReq, proxyRes */) { - delete headers['x-my-secret-header']; - return headers; - } - })); + it('provides an interface for updating headers', function (done) { + + app.use('/proxy', proxy('http://127.0.0.1:12345', { + userResHeaderDecorator: function (headers /*, userReq, userRes, proxyReq, proxyRes */) { + headers.boltedonheader = 'franky'; + return headers; + } + })); + + app.use(function (req, res) { + res.sendStatus(200); + }); + + request(app) + .get('/proxy') + .expect(function (res) { + assert(res.headers.boltedonheader === 'franky'); + }) + .end(done); + }); - app.use(function (req, res) { - res.sendStatus(200); + it('does not overwrite userRes headers', function (done) { + + app.use('/proxy', proxy('http://127.0.0.1:12345', { + // eslint-disable-next-line no-unused-vars + userResHeaderDecorator: function (headers, userReq, userRes, proxyReq, proxyRes) { + headers['set-cookie'] = [...userRes.getHeaders()['set-cookie'], ...proxyRes.headers['set-cookie']]; + return headers; + } + })); + + app.use(function (req, res) { + res.sendStatus(200); + }); + + request(app) + .get('/proxy') + .expect(function (res) { + assert.deepStrictEqual( + res.headers['set-cookie'], + [ + 'app-set-cookie1=app-value1; Path=/', + 'app-set-cookie2=app-value2; Path=/', + 'pTarget-set-cookie1=pTarget-value1; Path=/', + 'pTarget-set-cookie2=pTarget-value2; Path=/' + ] + ); + }) + .end(done); }); - request(app) - .get('/proxy') - .expect(function (res) { - assert(Object.keys(res.headers).indexOf('x-my-not-so-secret-header') > -1); - assert(Object.keys(res.headers).indexOf('x-my-secret-header') === -1); - }) - .end(done); - }); + it('overwrites res headers when userResHeaderDecorator is not set', function (done) { - it('provides an interface for updating headers', function (done) { + app.use('/proxy', proxy('http://127.0.0.1:12345')); - app.use('/proxy', proxy('http://127.0.0.1:12345', { - userResHeaderDecorator: function (headers /*, userReq, userRes, proxyReq, proxyRes */) { - headers.boltedonheader = 'franky'; - return headers; - } - })); + app.use(function (req, res) { + res.sendStatus(200); + }); - app.use(function (req, res) { - res.sendStatus(200); + request(app) + .get('/proxy') + .expect(function (res) { + assert.deepStrictEqual( + res.headers['set-cookie'], + ['pTarget-set-cookie1=pTarget-value1; Path=/', 'pTarget-set-cookie2=pTarget-value2; Path=/'] + ); + }) + .end(done); }); - - request(app) - .get('/proxy') - .expect(function (res) { - assert(res.headers.boltedonheader === 'franky'); - }) - .end(done); }); - });