From c83e9f4e5dcc5e639e1a906b1950435f86f1e596 Mon Sep 17 00:00:00 2001 From: Sigurd Fosseng Date: Sat, 27 Jan 2024 06:16:02 +0100 Subject: [PATCH 1/3] vary on delegated options --- index.js | 14 ++++++++------ test/cors.test.js | 24 ++++++++++++------------ test/hooks.test.js | 30 +++++++++++++++--------------- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/index.js b/index.js index ef6b584..15d0107 100644 --- a/index.js +++ b/index.js @@ -35,6 +35,8 @@ const hookWithPayload = [ 'onSend' ] +const optionsDelegated = Symbol('options has been delegated to a function and could be dynamic') + function validateHook (value, next) { if (validHooks.indexOf(value) !== -1) { return @@ -104,7 +106,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, payload, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -114,7 +116,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -128,7 +130,7 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl if (err) { next(err) } else { - addCorsHeadersHandler(fastify, normalizeCorsOptions(options), req, reply, next) + addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next) } }) } @@ -136,8 +138,8 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl /** * @param {import('./types').FastifyCorsOptions} opts */ -function normalizeCorsOptions (opts) { - const corsOptions = { ...defaultOptions, ...opts } +function normalizeCorsOptions (opts, { delegated = false } = { }) { + const corsOptions = { ...defaultOptions, ...opts, [optionsDelegated]: delegated } if (Array.isArray(opts.origin) && opts.origin.indexOf('*') !== -1) { corsOptions.origin = '*' } @@ -152,7 +154,7 @@ function normalizeCorsOptions (opts) { } function addCorsHeadersHandler (fastify, options, req, reply, next) { - if (typeof options.origin !== 'string' && options.origin !== false) { + if ((typeof options.origin !== 'string' && options.origin !== false) || options[optionsDelegated]) { // Always set Vary header for non-static origin option // https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches addOriginToVaryHeader(reply) diff --git a/test/cors.test.js b/test/cors.test.js index 39b575e..727c7cb 100644 --- a/test/cors.test.js +++ b/test/cors.test.js @@ -124,7 +124,7 @@ test('Should add cors headers (custom values)', t => { }) test('Should support dynamic config (callback)', t => { - t.plan(18) + t.plan(16) const configs = [{ origin: 'example.com', @@ -177,9 +177,9 @@ test('Should support dynamic config (callback)', t => { 'access-control-allow-origin': 'example.com', 'access-control-allow-credentials': 'true', 'access-control-expose-headers': 'foo, bar', - 'content-length': '2' + 'content-length': '2', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -202,9 +202,9 @@ test('Should support dynamic config (callback)', t => { 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', 'cache-control': '456', - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -221,7 +221,7 @@ test('Should support dynamic config (callback)', t => { }) test('Should support dynamic config (Promise)', t => { - t.plan(26) + t.plan(23) const configs = [{ origin: 'example.com', @@ -282,9 +282,9 @@ test('Should support dynamic config (Promise)', t => { 'access-control-allow-origin': 'example.com', 'access-control-allow-credentials': 'true', 'access-control-expose-headers': 'foo, bar', - 'content-length': '2' + 'content-length': '2', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -306,9 +306,9 @@ test('Should support dynamic config (Promise)', t => { 'access-control-allow-methods': 'GET', 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) t.equal(res.headers['cache-control'], undefined, 'cache-control omitted (invalid value)') }) @@ -332,9 +332,9 @@ test('Should support dynamic config (Promise)', t => { 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', 'cache-control': 'public, max-age=456', // cache-control included (custom string) - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ diff --git a/test/hooks.test.js b/test/hooks.test.js index 37345c3..0dd2778 100644 --- a/test/hooks.test.js +++ b/test/hooks.test.js @@ -321,7 +321,7 @@ test('Should set hook preSerialization if hook option is set to preSerialization }) test('Should support custom hook with dynamic config', t => { - t.plan(18) + t.plan(16) const configs = [{ origin: 'example.com', @@ -375,9 +375,9 @@ test('Should support custom hook with dynamic config', t => { 'access-control-allow-origin': 'example.com', 'access-control-allow-credentials': 'true', 'access-control-expose-headers': 'foo, bar', - 'content-length': '2' + 'content-length': '2', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -399,9 +399,9 @@ test('Should support custom hook with dynamic config', t => { 'access-control-allow-methods': 'GET', 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -418,7 +418,7 @@ test('Should support custom hook with dynamic config', t => { }) test('Should support custom hook with dynamic config (callback)', t => { - t.plan(18) + t.plan(16) const configs = [{ origin: 'example.com', @@ -472,9 +472,9 @@ test('Should support custom hook with dynamic config (callback)', t => { 'access-control-allow-origin': 'example.com', 'access-control-allow-credentials': 'true', 'access-control-expose-headers': 'foo, bar', - 'content-length': '2' + 'content-length': '2', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -496,9 +496,9 @@ test('Should support custom hook with dynamic config (callback)', t => { 'access-control-allow-methods': 'GET', 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -515,7 +515,7 @@ test('Should support custom hook with dynamic config (callback)', t => { }) test('Should support custom hook with dynamic config (Promise)', t => { - t.plan(18) + t.plan(16) const configs = [{ origin: 'example.com', @@ -570,9 +570,9 @@ test('Should support custom hook with dynamic config (Promise)', t => { 'access-control-allow-origin': 'example.com', 'access-control-allow-credentials': 'true', 'access-control-expose-headers': 'foo, bar', - 'content-length': '2' + 'content-length': '2', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ @@ -594,9 +594,9 @@ test('Should support custom hook with dynamic config (Promise)', t => { 'access-control-allow-methods': 'GET', 'access-control-allow-headers': 'baz, foo', 'access-control-max-age': '321', - 'content-length': '0' + 'content-length': '0', + vary: 'Origin' }) - t.notMatch(res.headers, { vary: 'Origin' }) }) fastify.inject({ From 69346b295416a40777d09caccf77d37977e8234d Mon Sep 17 00:00:00 2001 From: Sigurd Fosseng Date: Sat, 27 Jan 2024 06:40:51 +0100 Subject: [PATCH 2/3] DRY --- index.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 15d0107..c6ed277 100644 --- a/index.js +++ b/index.js @@ -35,7 +35,11 @@ const hookWithPayload = [ 'onSend' ] -const optionsDelegated = Symbol('options has been delegated to a function and could be dynamic') +const OptionsDelegated = Symbol('options has been delegated to a function and could be dynamic') +function optionsDelegated (opts) { + opts[OptionsDelegated] = true + return opts +} function validateHook (value, next) { if (validHooks.indexOf(value) !== -1) { @@ -106,7 +110,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, payload, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -116,7 +120,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -130,7 +134,7 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl if (err) { next(err) } else { - addCorsHeadersHandler(fastify, normalizeCorsOptions(options, { delegated: true }), req, reply, next) + addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next) } }) } @@ -138,8 +142,8 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl /** * @param {import('./types').FastifyCorsOptions} opts */ -function normalizeCorsOptions (opts, { delegated = false } = { }) { - const corsOptions = { ...defaultOptions, ...opts, [optionsDelegated]: delegated } +function normalizeCorsOptions (opts) { + const corsOptions = { ...defaultOptions, ...opts } if (Array.isArray(opts.origin) && opts.origin.indexOf('*') !== -1) { corsOptions.origin = '*' } @@ -154,7 +158,7 @@ function normalizeCorsOptions (opts, { delegated = false } = { }) { } function addCorsHeadersHandler (fastify, options, req, reply, next) { - if ((typeof options.origin !== 'string' && options.origin !== false) || options[optionsDelegated]) { + if ((typeof options.origin !== 'string' && options.origin !== false) || options[OptionsDelegated]) { // Always set Vary header for non-static origin option // https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches addOriginToVaryHeader(reply) From 556a6286d13c3c1d91f777de5f6a743c857490aa Mon Sep 17 00:00:00 2001 From: Sigurd Fosseng Date: Sat, 27 Jan 2024 16:36:44 +0100 Subject: [PATCH 3/3] reviewed --- index.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index c6ed277..42a9be8 100644 --- a/index.js +++ b/index.js @@ -35,12 +35,6 @@ const hookWithPayload = [ 'onSend' ] -const OptionsDelegated = Symbol('options has been delegated to a function and could be dynamic') -function optionsDelegated (opts) { - opts[OptionsDelegated] = true - return opts -} - function validateHook (value, next) { if (validHooks.indexOf(value) !== -1) { return @@ -110,7 +104,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, payload, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, true), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -120,7 +114,7 @@ function handleCorsOptionsDelegator (optionsResolver, fastify, opts, next) { fastify.addHook(hook, function handleCors (req, reply, next) { const ret = optionsResolver(req) if (ret && typeof ret.then === 'function') { - ret.then(options => addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next)).catch(next) + ret.then(options => addCorsHeadersHandler(fastify, normalizeCorsOptions(options, true), req, reply, next)).catch(next) return } next(new Error('Invalid CORS origin option')) @@ -134,7 +128,7 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl if (err) { next(err) } else { - addCorsHeadersHandler(fastify, optionsDelegated(normalizeCorsOptions(options)), req, reply, next) + addCorsHeadersHandler(fastify, normalizeCorsOptions(options, true), req, reply, next) } }) } @@ -142,7 +136,7 @@ function handleCorsOptionsCallbackDelegator (optionsResolver, fastify, req, repl /** * @param {import('./types').FastifyCorsOptions} opts */ -function normalizeCorsOptions (opts) { +function normalizeCorsOptions (opts, dynamic) { const corsOptions = { ...defaultOptions, ...opts } if (Array.isArray(opts.origin) && opts.origin.indexOf('*') !== -1) { corsOptions.origin = '*' @@ -154,11 +148,12 @@ function normalizeCorsOptions (opts) { // strings are applied directly and any other value is ignored corsOptions.cacheControl = null } + corsOptions.dynamic = dynamic || false return corsOptions } function addCorsHeadersHandler (fastify, options, req, reply, next) { - if ((typeof options.origin !== 'string' && options.origin !== false) || options[OptionsDelegated]) { + if ((typeof options.origin !== 'string' && options.origin !== false) || options.dynamic) { // Always set Vary header for non-static origin option // https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches addOriginToVaryHeader(reply)