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

Chunked cookies should not exceed browser max #237

Merged
merged 3 commits into from
Jun 3, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
34 changes: 28 additions & 6 deletions lib/appSession.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -90,6 +90,7 @@ module.exports = (config) => {
res,
{ uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }
) {
const cookies = req[COOKIES];
const cookieOptions = {
...cookieConfig,
expires: cookieConfig.transient ? 0 : new Date(exp * 1000),
Expand All @@ -102,7 +103,7 @@ module.exports = (config) => {
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,
Expand All @@ -115,25 +116,46 @@ module.exports = (config) => {
'found session, creating signed session cookie(s) with name %o(.i)',
sessionName
);
const emptyCookie = cookie.serialize(`${sessionName}.0`, '', cookieOptions);
davidpatrick marked this conversation as resolved.
Show resolved Hide resolved
const chunkSize = MAX_COOKIE_SIZE - emptyCookie.length;

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 / chunkSize);

if (chunkCount > 1) {
debug('cookie size greater than %d, chunking', CHUNK_BYTE_SIZE);
debug('cookie size greater than %d, chunking', chunkSize);
for (let i = 0; i < chunkCount; i++) {
const chunkValue = value.slice(
i * CHUNK_BYTE_SIZE,
(i + 1) * CHUNK_BYTE_SIZE
i * chunkSize,
(i + 1) * chunkSize
);

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
});
}
}
}
}
}
Expand Down
81 changes: 81 additions & 0 deletions test/appSession.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down