Skip to content

Commit 275e1c5

Browse files
saschanazmcollina
saschanaz
authored and
mcollina
committed
Add Vary header only for non-static origin option
1 parent f55dd5b commit 275e1c5

File tree

4 files changed

+74
-80
lines changed

4 files changed

+74
-80
lines changed

‎index.js‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,11 @@ function normalizeCorsOptions (opts) {
152152
}
153153

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

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

@@ -263,13 +265,8 @@ function resolveOriginWrapper (fastify, origin) {
263265
}
264266

265267
function getAccessControlAllowOriginHeader (reqOrigin, originOption) {
266-
if (originOption === '*') {
267-
// allow any origin
268-
return '*'
269-
}
270-
271268
if (typeof originOption === 'string') {
272-
// fixed origin
269+
// fixed or any origin ('*')
273270
return originOption
274271
}
275272

‎test/cors.test.js‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ test('Should add cors headers when payload is a stream', t => {
6363
})
6464

6565
test('Should add cors headers (custom values)', t => {
66-
t.plan(8)
66+
t.plan(10)
6767

6868
const fastify = Fastify()
6969
fastify.register(cors, {
@@ -94,7 +94,6 @@ test('Should add cors headers (custom values)', t => {
9494
t.equal(res.payload, '')
9595
t.match(res.headers, {
9696
'access-control-allow-origin': 'example.com',
97-
vary: 'Origin',
9897
'access-control-allow-credentials': 'true',
9998
'access-control-expose-headers': 'foo, bar',
10099
'access-control-allow-methods': 'GET',
@@ -103,6 +102,7 @@ test('Should add cors headers (custom values)', t => {
103102
'cache-control': 'max-age=321',
104103
'content-length': '0'
105104
})
105+
t.notMatch(res.headers, { vary: 'Origin' })
106106
})
107107

108108
fastify.inject({
@@ -115,16 +115,16 @@ test('Should add cors headers (custom values)', t => {
115115
t.equal(res.payload, 'ok')
116116
t.match(res.headers, {
117117
'access-control-allow-origin': 'example.com',
118-
vary: 'Origin',
119118
'access-control-allow-credentials': 'true',
120119
'access-control-expose-headers': 'foo, bar',
121120
'content-length': '2'
122121
})
122+
t.notMatch(res.headers, { vary: 'Origin' })
123123
})
124124
})
125125

126126
test('Should support dynamic config (callback)', t => {
127-
t.plan(16)
127+
t.plan(18)
128128

129129
const configs = [{
130130
origin: 'example.com',
@@ -175,11 +175,11 @@ test('Should support dynamic config (callback)', t => {
175175
t.equal(res.payload, 'ok')
176176
t.match(res.headers, {
177177
'access-control-allow-origin': 'example.com',
178-
vary: 'Origin',
179178
'access-control-allow-credentials': 'true',
180179
'access-control-expose-headers': 'foo, bar',
181180
'content-length': '2'
182181
})
182+
t.notMatch(res.headers, { vary: 'Origin' })
183183
})
184184

185185
fastify.inject({
@@ -196,7 +196,6 @@ test('Should support dynamic config (callback)', t => {
196196
t.equal(res.payload, '')
197197
t.match(res.headers, {
198198
'access-control-allow-origin': 'sample.com',
199-
vary: 'Origin',
200199
'access-control-allow-credentials': 'true',
201200
'access-control-expose-headers': 'zoo, bar',
202201
'access-control-allow-methods': 'GET',
@@ -205,6 +204,7 @@ test('Should support dynamic config (callback)', t => {
205204
'cache-control': '456',
206205
'content-length': '0'
207206
})
207+
t.notMatch(res.headers, { vary: 'Origin' })
208208
})
209209

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

223223
test('Should support dynamic config (Promise)', t => {
224-
t.plan(23)
224+
t.plan(26)
225225

226226
const configs = [{
227227
origin: 'example.com',
@@ -280,11 +280,11 @@ test('Should support dynamic config (Promise)', t => {
280280
t.equal(res.payload, 'ok')
281281
t.match(res.headers, {
282282
'access-control-allow-origin': 'example.com',
283-
vary: 'Origin',
284283
'access-control-allow-credentials': 'true',
285284
'access-control-expose-headers': 'foo, bar',
286285
'content-length': '2'
287286
})
287+
t.notMatch(res.headers, { vary: 'Origin' })
288288
})
289289

290290
fastify.inject({
@@ -301,14 +301,14 @@ test('Should support dynamic config (Promise)', t => {
301301
t.equal(res.payload, '')
302302
t.match(res.headers, {
303303
'access-control-allow-origin': 'sample.com',
304-
vary: 'Origin',
305304
'access-control-allow-credentials': 'true',
306305
'access-control-expose-headers': 'zoo, bar',
307306
'access-control-allow-methods': 'GET',
308307
'access-control-allow-headers': 'baz, foo',
309308
'access-control-max-age': '321',
310309
'content-length': '0'
311310
})
311+
t.notMatch(res.headers, { vary: 'Origin' })
312312
t.equal(res.headers['cache-control'], undefined, 'cache-control omitted (invalid value)')
313313
})
314314

@@ -326,7 +326,6 @@ test('Should support dynamic config (Promise)', t => {
326326
t.equal(res.payload, '')
327327
t.match(res.headers, {
328328
'access-control-allow-origin': 'sample.com',
329-
vary: 'Origin',
330329
'access-control-allow-credentials': 'true',
331330
'access-control-expose-headers': 'zoo, bar',
332331
'access-control-allow-methods': 'GET',
@@ -335,6 +334,7 @@ test('Should support dynamic config (Promise)', t => {
335334
'cache-control': 'public, max-age=456', // cache-control included (custom string)
336335
'content-length': '0'
337336
})
337+
t.notMatch(res.headers, { vary: 'Origin' })
338338
})
339339

340340
fastify.inject({
@@ -564,7 +564,7 @@ test('Dynamic origin resolution (errored - promises)', t => {
564564
})
565565
})
566566

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

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

@@ -608,8 +607,7 @@ test('Should reply 404 without cors headers other than `vary` when origin is fal
608607
t.same(res.headers, {
609608
'content-length': '2',
610609
'content-type': 'text/plain; charset=utf-8',
611-
connection: 'keep-alive',
612-
vary: 'Origin'
610+
connection: 'keep-alive'
613611
})
614612
})
615613
})
@@ -632,14 +630,13 @@ test('Server error if origin option is falsy but not false', t => {
632630
t.same(res.headers, {
633631
'content-length': '89',
634632
'content-type': 'application/json; charset=utf-8',
635-
connection: 'keep-alive',
636-
vary: 'Origin'
633+
connection: 'keep-alive'
637634
})
638635
})
639636
})
640637

641638
test('Allow only request from a specific origin', t => {
642-
t.plan(4)
639+
t.plan(5)
643640

644641
const fastify = Fastify()
645642
fastify.register(cors, { origin: 'other.io' })
@@ -658,9 +655,9 @@ test('Allow only request from a specific origin', t => {
658655
t.equal(res.statusCode, 200)
659656
t.equal(res.payload, 'ok')
660657
t.match(res.headers, {
661-
'access-control-allow-origin': 'other.io',
662-
vary: 'Origin'
658+
'access-control-allow-origin': 'other.io'
663659
})
660+
t.notMatch(res.headers, { vary: 'Origin' })
664661
})
665662
})
666663

@@ -772,11 +769,11 @@ test('Disable preflight', t => {
772769
})
773770
})
774771

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

778775
const fastify = Fastify()
779-
fastify.register(cors)
776+
fastify.register(cors, { origin: true })
780777

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

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

835832
const fastify = Fastify()
836833

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

840-
fastify.register(cors)
837+
fastify.register(cors, { origin: true })
841838

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

860857
test('Allow only request from with specific headers', t => {
861-
t.plan(7)
858+
t.plan(8)
862859

863860
const fastify = Fastify()
864861
fastify.register(cors, {
@@ -882,9 +879,9 @@ test('Allow only request from with specific headers', t => {
882879
delete res.headers.date
883880
t.equal(res.statusCode, 204)
884881
t.match(res.headers, {
885-
'access-control-allow-headers': 'foo',
886-
vary: 'Origin'
882+
'access-control-allow-headers': 'foo'
887883
})
884+
t.notMatch(res.headers, { vary: 'Origin' })
888885
})
889886

890887
fastify.inject({

0 commit comments

Comments
 (0)