From 80e75eef50fe73964fed752358314d0232a00c88 Mon Sep 17 00:00:00 2001 From: Nathan Woltman Date: Wed, 19 Jun 2019 14:40:02 -0400 Subject: [PATCH] refactor: Squeeze some more performance out of clearCookie Plus various other refactoring. --- cookie.js | 27 +++++++++++++++++---------- test/test.js | 23 +++++++++++++++++++---- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/cookie.js b/cookie.js index 24e7edd..7d4d6d2 100644 --- a/cookie.js +++ b/cookie.js @@ -1,10 +1,8 @@ 'use strict'; -const {parse: parseCookie, serialize: serializeCookie} = require('cookie'); +const {parse, serialize} = require('cookie'); const {sign, unsign} = require('cookie-signature'); -const clearCookieExpiresDate = new Date(1); - function cookie(app, {secret} = {}) { app.decorateRequest('cookies', null); @@ -24,24 +22,33 @@ function cookie(app, {secret} = {}) { function onRequest(req, res, next) { const cookieHeader = req.headers.cookie; - req.cookies = cookieHeader === undefined ? {} : parseCookie(cookieHeader); + + req.cookies = cookieHeader === undefined ? {} : parse(cookieHeader); + next(); } function setCookie(name, value, options) { - options = Object.assign({path: '/'}, options); - this.append('set-cookie', serializeCookie(name, value, options)); + const opts = Object.assign({path: '/'}, options); + + this.append('set-cookie', serialize(name, value, opts)); return this; } +const clearCookieExpiresDate = new Date(1); + function clearCookie(name, options) { // IE/Edge don't support Max-Age=0, so use Expires instead - options = Object.assign({expires: null, maxAge: null}, options); - options.expires = clearCookieExpiresDate; - options.maxAge = null; // In case options contains maxAge + const clearOpts = Object.assign({ + expires: clearCookieExpiresDate, + maxAge: null, + path: '/', + }, options); + + clearOpts.maxAge = null; // In case options contains maxAge - this.setCookie(name, '', options); + this.append('set-cookie', serialize(name, '', clearOpts)); return this; } diff --git a/test/test.js b/test/test.js index 2bf229d..ef2d3a1 100644 --- a/test/test.js +++ b/test/test.js @@ -79,7 +79,7 @@ describe('res.clearCookie()', () => { const app = makeApp(); app.get('/', (req, res) => { - res.clearCookie('foo', 'bar'); + res.clearCookie('foo'); res.send(); }); @@ -90,18 +90,33 @@ describe('res.clearCookie()', () => { ); }); + it('should clear a cookie with options', async () => { + const app = makeApp(); + + app.get('/', (req, res) => { + res.clearCookie('foo', {maxAge: 3600, httpOnly: true, secure: true}); + res.send(); + }); + + const res = await app.request('/'); + assert.deepStrictEqual( + res.headers['set-cookie'], + ['foo=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure'] + ); + }); + it('should clear multiple cookies', async () => { const app = makeApp(); app.get('/', (req, res) => { - res.clearCookie('a', {expires: new Date()}); - res.clearCookie('b', {maxAge: 3600, httpOnly: true, secure: true}); + res.clearCookie('a', {path: '/some/path'}); + res.clearCookie('b', {httpOnly: true, secure: true}); res.send(); }); const res = await app.request('/'); assert.deepStrictEqual(res.headers['set-cookie'], [ - 'a=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT', + 'a=; Path=/some/path; Expires=Thu, 01 Jan 1970 00:00:00 GMT', 'b=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure', ]); });