Don't send the Set-Cookie header multiple times for the same cookie (…

…#237)

* use set to update header

* update tests

* Revert "update tests" and add a new one

This reverts commit f69221b.

* use symbol

* use a Map and onSend hook

* add back line

* nit: remove variable and change separator

* unwrap fastifyCookieOnSendHandler

* Use || instead of ??

* make it compatible with header()

* move tap test path to taprc

* use values

* add examples

* istanbul ignore else

* remove signCookie decorator

* remove types

* Revert "remove signCookie decorator"

This reverts commit eddf349.

* Revert "remove types"

This reverts commit 1fbca3c.

* no need to pass signer to fastifyCookieSetCookie

* add type and tests for serializeCookie

* remove once-used const

* add type to serializeCookie options

* move interface

* remove examples

* move encode to seralize options

* fast path

* don't use for

* use for...of

* add benchmarks

* use undefined

.taprc

@@ -1,3 +1,2 @@
-100: true
-check-coverage: true
-coverage: true
+files:
+  - test/**/*.test.js

benchmark/cookie-multi.js

@@ -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)
+})

benchmark/cookie.js

@@ -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)
+})

package.json

@@ -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",

plugin.js

@@ -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') {
@@ -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
 }
 
@@ -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) {
@@ -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()) {
+      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',
@@ -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') {
@@ -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()
@@ -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) {