Chunked cookies should not exceed browser max (#237)

* Chunked cookies should not exceed browser max

* Clean up cookies when switching chunking vs single

* requested changes

lib/appSession.js

@@ -14,7 +14,7 @@ const { encryption: deriveKey } = require('./hkdf');
 const debug = require('./debug')('appSession');
 
 const epoch = () => (Date.now() / 1000) | 0;
-const CHUNK_BYTE_SIZE = 4000;
+const MAX_COOKIE_SIZE = 4096;
 
 function attachSessionObject(req, sessionName, value) {
   Object.defineProperty(req, sessionName, {
@@ -49,6 +49,12 @@ module.exports = (config) => {
     rollingDuration,
   } = config.session;
 
+  const { transient: emptyTransient , ...emptyCookieOptions } = cookieConfig;
+  emptyCookieOptions.expires = emptyTransient ? 0 : new Date();
+
+  const emptyCookie = cookie.serialize(`${sessionName}.0`, '', emptyCookieOptions);
+  const cookieChunkSize = MAX_COOKIE_SIZE - emptyCookie.length;
+
   let keystore = new JWKS.KeyStore();
 
   secrets.forEach((secretString, i) => {
@@ -90,19 +96,17 @@ module.exports = (config) => {
     res,
     { uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }
   ) {
-    const cookieOptions = {
-      ...cookieConfig,
-      expires: cookieConfig.transient ? 0 : new Date(exp * 1000),
-    };
-    delete cookieOptions.transient;
+    const cookies = req[COOKIES];
+    const { transient: cookieTransient , ...cookieOptions } = cookieConfig;
+    cookieOptions.expires = cookieTransient ? 0 : new Date(exp * 1000);
 
     // session was deleted or is empty, this matches all session cookies (chunked or unchunked)
     // and clears them, essentially cleaning up what we've set in the past that is now trash
     if (!req[sessionName] || !Object.keys(req[sessionName]).length) {
       debug(
         'session was deleted or is empty, clearing all matching session cookies'
       );
-      for (const cookieName of Object.keys(req[COOKIES])) {
+      for (const cookieName of Object.keys(cookies)) {
         if (cookieName.match(`^${sessionName}(?:\\.\\d)?$`)) {
           res.clearCookie(cookieName, {
             domain: cookieOptions.domain,
@@ -115,25 +119,44 @@ module.exports = (config) => {
         'found session, creating signed session cookie(s) with name %o(.i)',
         sessionName
       );
+
       const value = encrypt(JSON.stringify(req[sessionName]), {
         iat,
         uat,
         exp,
       });
 
-      const chunkCount = Math.ceil(value.length / CHUNK_BYTE_SIZE);
+      const chunkCount = Math.ceil(value.length / cookieChunkSize);
+
       if (chunkCount > 1) {
-        debug('cookie size greater than %d, chunking', CHUNK_BYTE_SIZE);
+        debug('cookie size greater than %d, chunking', cookieChunkSize);
         for (let i = 0; i < chunkCount; i++) {
           const chunkValue = value.slice(
-            i * CHUNK_BYTE_SIZE,
-            (i + 1) * CHUNK_BYTE_SIZE
+            i * cookieChunkSize,
+            (i + 1) * cookieChunkSize
           );
+
           const chunkCookieName = `${sessionName}.${i}`;
           res.cookie(chunkCookieName, chunkValue, cookieOptions);
         }
+        if (sessionName in cookies) {
+          debug('replacing non chunked cookie with chunked cookies');
+          res.clearCookie(sessionName, {
+            domain: cookieConfig.domain,
+            path: cookieConfig.path
+          });
+        }
       } else {
         res.cookie(sessionName, value, cookieOptions);
+        for (const cookieName of Object.keys(cookies)) {
+          debug('replacing chunked cookies with non chunked cookies');
+          if (cookieName.match(`^${sessionName}\\.\\d$`)) {
+            res.clearCookie(cookieName, {
+              domain: cookieConfig.domain,
+              path: cookieConfig.path
+            });
+          }
+        }
       }
     }
   }

package.json

@@ -14,7 +14,7 @@
   "scripts": {
     "lint": "eslint .",
     "start:example": "node ./examples/run_example.js",
-    "test": "mocha",
+    "test": "mocha --max-http-header-size=16384",
     "test:ci": "nyc --reporter=lcov npm test",
     "docs": "typedoc --options typedoc.js index.d.ts",
     "test:end-to-end": "mocha end-to-end"

test/appSession.tests.js

@@ -122,6 +122,87 @@ describe('appSession', () => {
     });
   });
 
+  it('should limit total cookie size to 4096 Bytes', async () => {
+    const path =
+    '/some-really-really-really-really-really-really-really-really-really-really-really-really-really-long-path';
+    server = await createServer(appSession(getConfig({ ...defaultConfig, session: { cookie: { path } } })));
+    const jar = request.jar();
+
+    await request.post('session', {
+      baseUrl,
+      jar,
+      json: {
+        sub: '__test_sub__',
+        random: crypto.randomBytes(8000).toString('base64'),
+      },
+    });
+
+    const cookies = jar
+      .getCookies(`${baseUrl}${path}`)
+      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+
+    assert.exists(cookies);
+    assert.equal(cookies['appSession.0'].length, 4096);
+    assert.equal(cookies['appSession.1'].length, 4096);
+    assert.equal(cookies['appSession.2'].length, 4096);
+    assert.isTrue(cookies['appSession.3'].length <= 4096)
+  });
+
+  it('should clean up single cookie when switching to chunked', async () => {
+    server = await createServer(appSession(getConfig(defaultConfig)));
+    const jar = request.jar();
+    jar.setCookie(`appSession=foo`, baseUrl)
+
+    const firstCookies = jar
+      .getCookies(baseUrl)
+      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+    assert.property(firstCookies, 'appSession')
+
+    await request.post('session', {
+      baseUrl,
+      jar,
+      json: {
+        sub: '__test_sub__',
+        random: crypto.randomBytes(8000).toString('base64'),
+      },
+    });
+
+    const cookies = jar
+      .getCookies(baseUrl)
+      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+
+    assert.property(cookies, 'appSession.0')
+    assert.notProperty(cookies, 'appSession')
+  });
+
+  it('should clean up chunked cookies when switching to single cookie', async () => {
+    server = await createServer(appSession(getConfig(defaultConfig)));
+    const jar = request.jar();
+    jar.setCookie(`appSession.0=foo`, baseUrl)
+    jar.setCookie(`appSession.1=foo`, baseUrl)
+
+    const firstCookies = jar
+      .getCookies(baseUrl)
+      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+    assert.property(firstCookies, 'appSession.0')
+    assert.property(firstCookies, 'appSession.1')
+
+    await request.post('session', {
+      baseUrl,
+      jar,
+      json: {
+        sub: '__test_sub__',
+      },
+    });
+
+    const cookies = jar
+      .getCookies(baseUrl)
+      .reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
+
+    assert.property(cookies, 'appSession')
+    assert.notProperty(cookies, 'appSession.0')
+  });
+
   it('should handle unordered chunked cookies', async () => {
     server = await createServer(appSession(getConfig(defaultConfig)));
     const jar = request.jar();