Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't send the Set-Cookie header multiple times for the same cookie #237

Merged
merged 30 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4b70e40
use set to update header
gurgunday Jul 22, 2023
f69221b
update tests
gurgunday Jul 22, 2023
d45f4cb
Revert "update tests" and add a new one
gurgunday Jul 25, 2023
f5d5429
use symbol
gurgunday Jul 25, 2023
b93db1a
use a Map and onSend hook
gurgunday Jul 25, 2023
7861de4
add back line
gurgunday Jul 25, 2023
5b44488
nit: remove variable and change separator
gurgunday Jul 25, 2023
52f580c
unwrap fastifyCookieOnSendHandler
gurgunday Jul 26, 2023
eab4b9e
Use || instead of ??
gurgunday Jul 26, 2023
fc09786
make it compatible with header()
gurgunday Jul 28, 2023
0d66b46
move tap test path to taprc
gurgunday Jul 28, 2023
e3703dd
use values
gurgunday Jul 28, 2023
08586d8
add examples
gurgunday Jul 28, 2023
e6b5def
istanbul ignore else
gurgunday Jul 28, 2023
eddf349
remove signCookie decorator
gurgunday Jul 30, 2023
1fbca3c
remove types
gurgunday Jul 30, 2023
7fbf6ee
Revert "remove signCookie decorator"
gurgunday Jul 30, 2023
4a1b99b
Revert "remove types"
gurgunday Jul 30, 2023
b6ca9cb
no need to pass signer to fastifyCookieSetCookie
gurgunday Jul 30, 2023
10e5414
add type and tests for serializeCookie
gurgunday Jul 30, 2023
f1bf2c1
remove once-used const
gurgunday Jul 30, 2023
ea8eeba
add type to serializeCookie options
gurgunday Jul 30, 2023
0df9253
move interface
gurgunday Jul 30, 2023
1571d81
remove examples
gurgunday Jul 30, 2023
777d5aa
move encode to seralize options
gurgunday Jul 30, 2023
1a6d9cc
fast path
gurgunday Jul 31, 2023
d8e0ec0
don't use for
gurgunday Jul 31, 2023
a451f81
use for...of
gurgunday Jul 31, 2023
235e7e0
add benchmarks
gurgunday Jul 31, 2023
36fa3a6
use undefined
gurgunday Aug 1, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .taprc
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
100: true
check-coverage: true
coverage: true
files:
- test/**/*.test.js
24 changes: 24 additions & 0 deletions benchmark/cookie-multi.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict'

const Fastify = require('fastify')
const plugin = require('../')

const secret = 'testsecret'

const fastify = Fastify()
fastify.register(plugin, { secret })

fastify.get('/', (req, reply) => {
reply
.setCookie('foo', 'foo')
.setCookie('foo', 'foo', { path: '/1' })
.setCookie('boo', 'boo', { path: '/' })
.setCookie('foo', 'foo-different', { path: '/' })
.setCookie('foo', 'foo', { path: '/2' })
.send({ hello: 'world' })
})

fastify.listen({ host: '127.0.0.1', port: 5001 }, (err, address) => {
if (err) throw err
console.log(address)
})
20 changes: 20 additions & 0 deletions benchmark/cookie.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict'

const Fastify = require('fastify')
const plugin = require('../')

const secret = 'testsecret'

const fastify = Fastify()
fastify.register(plugin, { secret })

fastify.get('/', (req, reply) => {
reply
.setCookie('foo', 'foo', { path: '/' })
.send({ hello: 'world' })
})

fastify.listen({ host: '127.0.0.1', port: 5001 }, (err, address) => {
if (err) throw err
console.log(address)
})
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
"main": "plugin.js",
"types": "types/plugin.d.ts",
"scripts": {
"coverage": "npm run test:unit -- --coverage-report=html",
"lint": "standard | snazzy",
"lint:ci": "standard",
"lint:fix": "standard --fix",
"test": "npm run unit && npm run typescript",
"typescript": "tsd",
"unit": "tap -J \"test/*.test.js\"",
"unit:report": "npm run unit -- --coverage-report=html",
"unit:verbose": "npm run unit -- -Rspec"
"test": "npm run test:unit && npm run test:typescript",
"test:typescript": "tsd",
"test:unit": "tap",
"test:unit:verbose": "npm run test:unit -- -Rspec"
},
"precommit": [
"lint",
Expand Down
63 changes: 44 additions & 19 deletions plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ const cookie = require('cookie')

const { Signer, sign, unsign } = require('./signer')

function fastifyCookieSetCookie (reply, name, value, options, signer) {
const kReplySetCookies = Symbol('fastify.reply.setCookies')

function fastifyCookieSetCookie (reply, name, value, options) {
const opts = Object.assign({}, options)

if (opts.expires && Number.isInteger(opts.expires)) {
opts.expires = new Date(opts.expires)
}

if (opts.signed) {
value = signer.sign(value)
value = reply.signCookie(value)
}

if (opts.secure === 'auto') {
Expand All @@ -24,20 +27,8 @@ function fastifyCookieSetCookie (reply, name, value, options, signer) {
}
}

const serialized = cookie.serialize(name, value, opts)
let setCookie = reply.getHeader('Set-Cookie')
if (!setCookie) {
reply.header('Set-Cookie', serialized)
return reply
}

if (typeof setCookie === 'string') {
setCookie = [setCookie]
}
reply[kReplySetCookies].set(`${name};${opts.domain};${opts.path || '/'}`, { name, value, opts })

setCookie.push(serialized)
reply.removeHeader('Set-Cookie')
reply.header('Set-Cookie', setCookie)
return reply
}

Expand All @@ -58,6 +49,7 @@ function onReqHandlerWrapper (fastify, hook) {
if (cookieHeader) {
fastifyReq.cookies = fastify.parseCookie(cookieHeader)
}
fastifyRes[kReplySetCookies] = new Map()
done()
}
: function fastifyCookieHandler (fastifyReq, fastifyRes, done) {
Expand All @@ -66,10 +58,41 @@ function onReqHandlerWrapper (fastify, hook) {
if (cookieHeader) {
fastifyReq.cookies = fastify.parseCookie(cookieHeader)
}
fastifyRes[kReplySetCookies] = new Map()
done()
}
}

function fastifyCookieOnSendHandler (fastifyReq, fastifyRes, payload, done) {
if (fastifyRes[kReplySetCookies].size) {
let setCookie = fastifyRes.getHeader('Set-Cookie')

/* istanbul ignore else */
if (setCookie === undefined) {
if (fastifyRes[kReplySetCookies].size === 1) {
for (const c of fastifyRes[kReplySetCookies].values()) {
fastifyRes.header('Set-Cookie', cookie.serialize(c.name, c.value, c.opts))
}

return done()
}

setCookie = []
} else if (typeof setCookie === 'string') {
setCookie = [setCookie]
}

for (const c of fastifyRes[kReplySetCookies].values()) {
Uzlopak marked this conversation as resolved.
Show resolved Hide resolved
setCookie.push(cookie.serialize(c.name, c.value, c.opts))
}

fastifyRes.removeHeader('Set-Cookie')
fastifyRes.header('Set-Cookie', setCookie)
}

done()
}

function getHook (hook = 'onRequest') {
const hooks = {
onRequest: 'onRequest',
Expand All @@ -89,9 +112,9 @@ function plugin (fastify, options, next) {
return next(new Error('@fastify/cookie: Invalid value provided for the hook-option. You can set the hook-option only to false, \'onRequest\' , \'preParsing\' , \'preValidation\' or \'preHandler\''))
}
const isSigner = !secret || (typeof secret.sign === 'function' && typeof secret.unsign === 'function')
const algorithm = options.algorithm || 'sha256'
const signer = isSigner ? secret : new Signer(secret, algorithm)
const signer = isSigner ? secret : new Signer(secret, options.algorithm || 'sha256')

fastify.decorate('serializeCookie', cookie.serialize)
fastify.decorate('parseCookie', parseCookie)

if (typeof secret !== 'undefined') {
Expand All @@ -106,13 +129,15 @@ function plugin (fastify, options, next) {
}

fastify.decorateRequest('cookies', null)
fastify.decorateReply('cookie', setCookie)
fastify.decorateReply(kReplySetCookies, null)

fastify.decorateReply('cookie', setCookie)
fastify.decorateReply('setCookie', setCookie)
fastify.decorateReply('clearCookie', clearCookie)

if (hook) {
fastify.addHook(hook, onReqHandlerWrapper(fastify, hook))
fastify.addHook('onSend', fastifyCookieOnSendHandler)
}

next()
Expand All @@ -132,7 +157,7 @@ function plugin (fastify, options, next) {

function setCookie (name, value, cookieOptions) {
const opts = Object.assign({}, options.parseOptions, cookieOptions)
return fastifyCookieSetCookie(this, name, value, opts, signer)
return fastifyCookieSetCookie(this, name, value, opts)
}

function clearCookie (name, cookieOptions) {
Expand Down
Loading