From c45fdf8c68439841ae5274b51fdbd2cba0c5ec7e Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 28 Jul 2022 15:13:58 -0600 Subject: [PATCH 1/7] Adding verification that sess.cookie is set --- session/store.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/store.js b/session/store.js index 3793877e..5aa35e29 100644 --- a/session/store.js +++ b/session/store.js @@ -68,7 +68,7 @@ Store.prototype.load = function(sid, fn){ var self = this; this.get(sid, function(err, sess){ if (err) return fn(err); - if (!sess) return fn(); + if (!sess || !sess.cookie) return fn(); var req = { sessionID: sid, sessionStore: self }; fn(null, self.createSession(req, sess)) }); From 6057549dc49aa2ab7f4c9873774670d88bff3ac7 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 28 Jul 2022 17:30:27 -0600 Subject: [PATCH 2/7] Returning an error when sess.cookie is not set --- session/store.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/session/store.js b/session/store.js index 5aa35e29..fb50cfe8 100644 --- a/session/store.js +++ b/session/store.js @@ -68,7 +68,8 @@ Store.prototype.load = function(sid, fn){ var self = this; this.get(sid, function(err, sess){ if (err) return fn(err); - if (!sess || !sess.cookie) return fn(); + if (!sess) return fn(); + if (!sess.cookie) return fn(new Error('sess.cookie is undefined')); var req = { sessionID: sid, sessionStore: self }; fn(null, self.createSession(req, sess)) }); From bb777ad0701727675edac6992f6efde4ff91478d Mon Sep 17 00:00:00 2001 From: brians Date: Thu, 28 Jul 2022 17:51:55 -0600 Subject: [PATCH 3/7] Adding corrupt session test --- test/session.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/session.js b/test/session.js index 7416b261..8af0cdb3 100644 --- a/test/session.js +++ b/test/session.js @@ -490,6 +490,34 @@ describe('session()', function(){ }) }) + describe('when session is corrupt', function () { + it('should return an error', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + req.session.count = req.session.count || 0 + req.session.count++ + res.end('hits: ' + req.session.count) + }) + + store.get = function returnCorruptSession(sid, callback) { + callback(undefined, {}); + } + + request(server) + .get('/') + .expect(200, 'hits: 1', function (err, res) { + if (err) return done(err) + store.load(sid(res), function (err, sess) { + assert.strictEqual(err.message, 'sess.cookie is undefined') + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(500, "Cannot read property 'expires' of undefined", done) + }) + }) + }) + }) + describe('when session expired in store', function () { it('should create a new session', function (done) { var count = 0 From 1f6f52453b2f2fe793f0a0aee47ed0d45645684c Mon Sep 17 00:00:00 2001 From: brians Date: Thu, 28 Jul 2022 17:59:06 -0600 Subject: [PATCH 4/7] Expanding error handling for all kinds of corrupt sessions --- session/store.js | 9 +++++++-- test/session.js | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/session/store.js b/session/store.js index fb50cfe8..46ce6287 100644 --- a/session/store.js +++ b/session/store.js @@ -69,9 +69,14 @@ Store.prototype.load = function(sid, fn){ this.get(sid, function(err, sess){ if (err) return fn(err); if (!sess) return fn(); - if (!sess.cookie) return fn(new Error('sess.cookie is undefined')); var req = { sessionID: sid, sessionStore: self }; - fn(null, self.createSession(req, sess)) + try { + sess = self.createSession(req, sess) + } catch (e) { + err = e + sess = null + } + fn(err, sess) }); }; diff --git a/test/session.js b/test/session.js index 8af0cdb3..7d4aac0f 100644 --- a/test/session.js +++ b/test/session.js @@ -508,7 +508,7 @@ describe('session()', function(){ .expect(200, 'hits: 1', function (err, res) { if (err) return done(err) store.load(sid(res), function (err, sess) { - assert.strictEqual(err.message, 'sess.cookie is undefined') + assert.strictEqual(err.message, "Cannot read property 'expires' of undefined") request(server) .get('/') .set('Cookie', cookie(res)) From c0a79e0f08f19df2e3c5d39949303d13e2414ddf Mon Sep 17 00:00:00 2001 From: brians Date: Thu, 28 Jul 2022 18:07:45 -0600 Subject: [PATCH 5/7] Updating test to pass on all node versions --- test/session.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/session.js b/test/session.js index 7d4aac0f..5ed33135 100644 --- a/test/session.js +++ b/test/session.js @@ -508,11 +508,11 @@ describe('session()', function(){ .expect(200, 'hits: 1', function (err, res) { if (err) return done(err) store.load(sid(res), function (err, sess) { - assert.strictEqual(err.message, "Cannot read property 'expires' of undefined") + assert.match(err.message, /Cannot read prop/) request(server) .get('/') .set('Cookie', cookie(res)) - .expect(500, "Cannot read property 'expires' of undefined", done) + .expect(500, /Cannot read prop/, done) }) }) }) From bbac326811120a174112c51aecf4ba082a1cde87 Mon Sep 17 00:00:00 2001 From: brians Date: Thu, 28 Jul 2022 19:41:55 -0600 Subject: [PATCH 6/7] Moving assert.match to assert.ok --- test/session.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/session.js b/test/session.js index 5ed33135..fda9d499 100644 --- a/test/session.js +++ b/test/session.js @@ -508,7 +508,8 @@ describe('session()', function(){ .expect(200, 'hits: 1', function (err, res) { if (err) return done(err) store.load(sid(res), function (err, sess) { - assert.match(err.message, /Cannot read prop/) + assert.ok(err); + assert.ok(err.message.match(/Cannot read prop/)); request(server) .get('/') .set('Cookie', cookie(res)) From c9cd25ab6662c033d06a427058cfcc72ec578dec Mon Sep 17 00:00:00 2001 From: brians Date: Fri, 3 Mar 2023 16:18:04 -0700 Subject: [PATCH 7/7] Adds corrupt session handling for .reload() --- session/session.js | 8 ++++++-- test/session.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/session/session.js b/session/session.js index fee7608c..34afa949 100644 --- a/session/session.js +++ b/session/session.js @@ -92,8 +92,12 @@ defineMethod(Session.prototype, 'reload', function reload(fn) { store.get(this.id, function(err, sess){ if (err) return fn(err); if (!sess) return fn(new Error('failed to load session')); - store.createSession(req, sess); - fn(); + try { + store.createSession(req, sess); + } catch (e) { + err = e + } + fn(err); }); return this; }); diff --git a/test/session.js b/test/session.js index fda9d499..dd23be10 100644 --- a/test/session.js +++ b/test/session.js @@ -1694,6 +1694,40 @@ describe('session()', function(){ }) }) + it('should error when the session is corrupt', function (done) { + var store = new session.MemoryStore() + var server = createServer({ store: store }, function (req, res) { + if (req.url === '/') { + req.session.active = true + res.end('session created') + return + } + + store.clear(function (err) { + if (err) return done(err) + + store.get = function returnCorruptSession(sid, callback) { + callback(undefined, {}); + } + + req.session.reload(function (err) { + res.statusCode = err ? 500 : 200 + res.end(err ? err.message : '') + }) + }) + }) + + request(server) + .get('/') + .expect(200, 'session created', function (err, res) { + if (err) return done(err) + request(server) + .get('/foo') + .set('Cookie', cookie(res)) + .expect(500, /Cannot read prop/, done) + }) + }) + it('should not override an overriden `reload` in case of errors', function (done) { var store = new session.MemoryStore() var server = createServer({ store: store, resave: false }, function (req, res) {