From ac46c2f2e9e2bdf8458a69478b8d10c9f57f3d93 Mon Sep 17 00:00:00 2001 From: Naz Date: Thu, 3 Nov 2022 11:15:19 +0800 Subject: [PATCH] Fixed CORS vary header modification refs https://github.com/TryGhost/Toolbox/issues/461 - The 'vary' header with 'Origin' value should only be set when an OPTIONS header is processed. Otherwise we are prone to leaking the vary header modification to further down in the request pipeline --- ghost/core/core/frontend/web/middleware/cors.js | 11 +++++++---- ghost/core/core/server/web/api/middleware/cors.js | 11 +++++++---- .../unit/server/web/api/middleware/cors.test.js | 14 +++++++++++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/ghost/core/core/frontend/web/middleware/cors.js b/ghost/core/core/frontend/web/middleware/cors.js index de49d7a565..35970839f6 100644 --- a/ghost/core/core/frontend/web/middleware/cors.js +++ b/ghost/core/core/frontend/web/middleware/cors.js @@ -61,12 +61,15 @@ function corsOptionsDelegate(req, callback) { * @param {Function} next */ const handleCaching = (req, res, next) => { - // @NOTE: try to add native support for dynamic 'vary' header value in 'cors' module - res.vary('Origin'); + const method = req.method && req.method.toUpperCase && req.method.toUpperCase(); + if (method === 'OPTIONS') { + // @NOTE: try to add native support for dynamic 'vary' header value in 'cors' module + res.vary('Origin'); + } next(); }; module.exports = [ - cors(corsOptionsDelegate), - handleCaching + handleCaching, + cors(corsOptionsDelegate) ]; diff --git a/ghost/core/core/server/web/api/middleware/cors.js b/ghost/core/core/server/web/api/middleware/cors.js index 01e5225d16..9ac668f231 100644 --- a/ghost/core/core/server/web/api/middleware/cors.js +++ b/ghost/core/core/server/web/api/middleware/cors.js @@ -88,12 +88,15 @@ function corsOptionsDelegate(req, cb) { * @param {Function} next */ const handleCaching = (req, res, next) => { - // @NOTE: try to add native support for dynamic 'vary' header value in 'cors' module - res.vary('Origin'); + const method = req.method && req.method.toUpperCase && req.method.toUpperCase(); + if (method === 'OPTIONS') { + // @NOTE: try to add native support for dynamic 'vary' header value in 'cors' module + res.vary('Origin'); + } next(); }; module.exports = [ - cors(corsOptionsDelegate), - handleCaching + handleCaching, + cors(corsOptionsDelegate) ]; diff --git a/ghost/core/test/unit/server/web/api/middleware/cors.test.js b/ghost/core/test/unit/server/web/api/middleware/cors.test.js index 02d953fb58..fd11f1cbb4 100644 --- a/ghost/core/test/unit/server/web/api/middleware/cors.test.js +++ b/ghost/core/test/unit/server/web/api/middleware/cors.test.js @@ -3,8 +3,8 @@ const sinon = require('sinon'); const rewire = require('rewire'); const configUtils = require('../../../../../utils/configUtils'); -let cors = rewire('../../../../../../core/server/web/api/middleware/cors')[0]; -let corsCaching = rewire('../../../../../../core/server/web/api/middleware/cors')[1]; +let cors = rewire('../../../../../../core/server/web/api/middleware/cors')[1]; +let corsCaching = rewire('../../../../../../core/server/web/api/middleware/cors')[0]; describe('cors', function () { let res; @@ -37,7 +37,7 @@ describe('cors', function () { afterEach(function () { sinon.restore(); configUtils.restore(); - cors = rewire('../../../../../../core/server/web/api/middleware/cors')[0]; + cors = rewire('../../../../../../core/server/web/api/middleware/cors')[1]; }); it('should not be enabled without a request origin header', function (done) { @@ -142,4 +142,12 @@ describe('cors', function () { done(); }); }); + + it('should NOT add origin value to the vary header when not an OPTIONS request', function (done) { + req.method = 'GET'; + corsCaching(req, res, function () { + should.equal(res.vary.called, false); + done(); + }); + }); });