Skip to content

Commit

Permalink
Add Vary header only for non-static origin option
Browse files Browse the repository at this point in the history
  • Loading branch information
@saschanaz
saschanaz committed Dec 26, 2023
1 parent f55dd5b commit 122de83
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 80 deletions.
15 changes: 6 additions & 9 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,11 @@ function normalizeCorsOptions (opts) {
}

function addCorsHeadersHandler (fastify, options, req, reply, next) {
// Always set Vary header
// https://github.com/rs/cors/issues/10
addOriginToVaryHeader(reply)
if (typeof options.origin !== 'string' && options.origin !== false) {
// Always set Vary header for non-static origin option
// https://fetch.spec.whatwg.org/#cors-protocol-and-http-caches
addOriginToVaryHeader(reply)
}

const resolveOriginOption = typeof options.origin === 'function' ? resolveOriginWrapper(fastify, options.origin) : (_, cb) => cb(null, options.origin)

Expand Down Expand Up @@ -263,13 +265,8 @@ function resolveOriginWrapper (fastify, origin) {
}

function getAccessControlAllowOriginHeader (reqOrigin, originOption) {
if (originOption === '*') {
// allow any origin
return '*'
}

if (typeof originOption === 'string') {
// fixed origin
// fixed or any origin ('*')
return originOption
}

Expand Down
51 changes: 24 additions & 27 deletions test/cors.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test('Should add cors headers when payload is a stream', t => {
})

test('Should add cors headers (custom values)', t => {
t.plan(8)
t.plan(10)

const fastify = Fastify()
fastify.register(cors, {
Expand Down Expand Up @@ -94,7 +94,6 @@ test('Should add cors headers (custom values)', t => {
t.equal(res.payload, '')
t.match(res.headers, {
'access-control-allow-origin': 'example.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'foo, bar',
'access-control-allow-methods': 'GET',
Expand All @@ -103,6 +102,7 @@ test('Should add cors headers (custom values)', t => {
'cache-control': 'max-age=321',
'content-length': '0'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand All @@ -115,16 +115,16 @@ test('Should add cors headers (custom values)', t => {
t.equal(res.payload, 'ok')
t.match(res.headers, {
'access-control-allow-origin': 'example.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'foo, bar',
'content-length': '2'
})
t.notMatch(res.headers, { vary: 'Origin' })
})
})

test('Should support dynamic config (callback)', t => {
t.plan(16)
t.plan(18)

const configs = [{
origin: 'example.com',
Expand Down Expand Up @@ -175,11 +175,11 @@ test('Should support dynamic config (callback)', t => {
t.equal(res.payload, 'ok')
t.match(res.headers, {
'access-control-allow-origin': 'example.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'foo, bar',
'content-length': '2'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand All @@ -196,7 +196,6 @@ test('Should support dynamic config (callback)', t => {
t.equal(res.payload, '')
t.match(res.headers, {
'access-control-allow-origin': 'sample.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'zoo, bar',
'access-control-allow-methods': 'GET',
Expand All @@ -205,6 +204,7 @@ test('Should support dynamic config (callback)', t => {
'cache-control': '456',
'content-length': '0'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand All @@ -221,7 +221,7 @@ test('Should support dynamic config (callback)', t => {
})

test('Should support dynamic config (Promise)', t => {
t.plan(23)
t.plan(26)

const configs = [{
origin: 'example.com',
Expand Down Expand Up @@ -280,11 +280,11 @@ test('Should support dynamic config (Promise)', t => {
t.equal(res.payload, 'ok')
t.match(res.headers, {
'access-control-allow-origin': 'example.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'foo, bar',
'content-length': '2'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand All @@ -301,14 +301,14 @@ test('Should support dynamic config (Promise)', t => {
t.equal(res.payload, '')
t.match(res.headers, {
'access-control-allow-origin': 'sample.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'zoo, bar',
'access-control-allow-methods': 'GET',
'access-control-allow-headers': 'baz, foo',
'access-control-max-age': '321',
'content-length': '0'
})
t.notMatch(res.headers, { vary: 'Origin' })
t.equal(res.headers['cache-control'], undefined, 'cache-control omitted (invalid value)')
})

Expand All @@ -326,7 +326,6 @@ test('Should support dynamic config (Promise)', t => {
t.equal(res.payload, '')
t.match(res.headers, {
'access-control-allow-origin': 'sample.com',
vary: 'Origin',
'access-control-allow-credentials': 'true',
'access-control-expose-headers': 'zoo, bar',
'access-control-allow-methods': 'GET',
Expand All @@ -335,6 +334,7 @@ test('Should support dynamic config (Promise)', t => {
'cache-control': 'public, max-age=456', // cache-control included (custom string)
'content-length': '0'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand Down Expand Up @@ -564,7 +564,7 @@ test('Dynamic origin resolution (errored - promises)', t => {
})
})

test('Should reply 404 without cors headers other than `vary` when origin is false', t => {
test('Should reply 404 without cors headers when origin is false', t => {
t.plan(8)

const fastify = Fastify()
Expand Down Expand Up @@ -592,8 +592,7 @@ test('Should reply 404 without cors headers other than `vary` when origin is fal
t.same(res.headers, {
'content-length': '76',
'content-type': 'application/json; charset=utf-8',
connection: 'keep-alive',
vary: 'Origin'
connection: 'keep-alive'
})
})

Expand All @@ -608,8 +607,7 @@ test('Should reply 404 without cors headers other than `vary` when origin is fal
t.same(res.headers, {
'content-length': '2',
'content-type': 'text/plain; charset=utf-8',
connection: 'keep-alive',
vary: 'Origin'
connection: 'keep-alive'
})
})
})
Expand All @@ -632,14 +630,13 @@ test('Server error if origin option is falsy but not false', t => {
t.same(res.headers, {
'content-length': '89',
'content-type': 'application/json; charset=utf-8',
connection: 'keep-alive',
vary: 'Origin'
connection: 'keep-alive'
})
})
})

test('Allow only request from a specific origin', t => {
t.plan(4)
t.plan(5)

const fastify = Fastify()
fastify.register(cors, { origin: 'other.io' })
Expand All @@ -658,9 +655,9 @@ test('Allow only request from a specific origin', t => {
t.equal(res.statusCode, 200)
t.equal(res.payload, 'ok')
t.match(res.headers, {
'access-control-allow-origin': 'other.io',
vary: 'Origin'
'access-control-allow-origin': 'other.io'
})
t.notMatch(res.headers, { vary: 'Origin' })
})
})

Expand Down Expand Up @@ -772,11 +769,11 @@ test('Disable preflight', t => {
})
})

test('Should always add vary header to `Origin` by default', t => {
test('Should always add vary header to `Origin` for reflected origin', t => {
t.plan(12)

const fastify = Fastify()
fastify.register(cors)
fastify.register(cors, { origin: true })

fastify.get('/', (req, reply) => {
reply.send('ok')
Expand Down Expand Up @@ -829,15 +826,15 @@ test('Should always add vary header to `Origin` by default', t => {
})
})

test('Should always add vary header to `Origin` by default (vary is array)', t => {
test('Should always add vary header to `Origin` for reflected origin (vary is array)', t => {
t.plan(4)

const fastify = Fastify()

// Mock getHeader function
fastify.decorateReply('getHeader', (name) => ['foo', 'bar'])

fastify.register(cors)
fastify.register(cors, { origin: true })

fastify.get('/', (req, reply) => {
reply.send('ok')
Expand All @@ -858,7 +855,7 @@ test('Should always add vary header to `Origin` by default (vary is array)', t =
})

test('Allow only request from with specific headers', t => {
t.plan(7)
t.plan(8)

const fastify = Fastify()
fastify.register(cors, {
Expand All @@ -882,9 +879,9 @@ test('Allow only request from with specific headers', t => {
delete res.headers.date
t.equal(res.statusCode, 204)
t.match(res.headers, {
'access-control-allow-headers': 'foo',
vary: 'Origin'
'access-control-allow-headers': 'foo'
})
t.notMatch(res.headers, { vary: 'Origin' })
})

fastify.inject({
Expand Down
Loading

0 comments on commit 122de83

Please sign in to comment.