From b1e56d4aba2a75d14f73e4c7ae366eab10a647c7 Mon Sep 17 00:00:00 2001 From: Deepak Ganesh Date: Sun, 2 Aug 2026 22:25:52 +0530 Subject: [PATCH] fix: avoid duplicate save after regenerate Wrap regenerated sessions so manual saves update the request save tracking. Track the explicitly saved session id separately from the original id so modification and Set-Cookie semantics remain unchanged after session id rotation. Fixes #935 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- index.js | 28 +++++++++++++++++++++- test/session.js | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 3e23e901..2b58f1fb 100644 --- a/index.js +++ b/index.js @@ -223,6 +223,7 @@ function session(options) { var originalHash; var originalId; var savedHash; + var savedId; var touched = false // expose store @@ -383,6 +384,8 @@ function session(options) { store.generate(req); originalId = req.sessionID; originalHash = hash(req.session); + savedHash = undefined + savedId = undefined wrapmethods(req.session); } @@ -394,6 +397,10 @@ function session(options) { if (!resaveSession) { savedHash = originalHash + savedId = req.sessionID + } else { + savedHash = undefined + savedId = undefined } wrapmethods(req.session) @@ -411,9 +418,20 @@ function session(options) { // wrap session methods function wrapmethods(sess) { + var _regenerate = sess.regenerate var _reload = sess.reload var _save = sess.save; + function regenerate(callback) { + debug('regenerating %s', this.id) + var sess = this + _regenerate.call(this, function () { + savedHash = undefined + savedId = undefined + rewrapmethods(sess, callback).apply(this, arguments) + }) + } + function reload(callback) { debug('reloading %s', this.id) _reload.call(this, rewrapmethods(this, callback)) @@ -422,9 +440,17 @@ function session(options) { function save() { debug('saving %s', this.id); savedHash = hash(this); + savedId = this.id; _save.apply(this, arguments); } + Object.defineProperty(sess, 'regenerate', { + configurable: true, + enumerable: false, + value: regenerate, + writable: true + }) + Object.defineProperty(sess, 'reload', { configurable: true, enumerable: false, @@ -447,7 +473,7 @@ function session(options) { // check if session has been saved function isSaved(sess) { - return originalId === sess.id && savedHash === hash(sess); + return savedId === sess.id && savedHash === hash(sess); } // determine if session should be destroyed diff --git a/test/session.js b/test/session.js index 405e3ddf..03c07139 100644 --- a/test/session.js +++ b/test/session.js @@ -1918,6 +1918,70 @@ describe('session()', function(){ .expect(200, 'false', done) }); }) + + it('should prevent end-of-request save after manual save', function (done) { + var store = new session.MemoryStore() + var count = 0 + var _set = store.set + var server = createServer({ store: store, resave: false, saveUninitialized: false }, function (req, res) { + req.session.regenerate(function (err) { + if (err) return res.end(err.message) + req.session.value = 'foo' + req.session.save(function (err) { + if (err) return res.end(err.message) + res.end('saved') + }) + }) + }) + + store.set = function set () { + count++ + return _set.apply(this, arguments) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'saved', function (err) { + if (err) return done(err) + assert.strictEqual(count, 1) + done() + }) + }) + + it('should set cookie after manual save without data changes', function (done) { + var store = new session.MemoryStore() + var count = 0 + var _set = store.set + var server = createServer({ store: store, resave: false, saveUninitialized: false }, function (req, res) { + var oldId = req.session.id + + req.session.regenerate(function (err) { + if (err) return res.end(err.message) + req.session.save(function (err) { + if (err) return res.end(err.message) + res.end(oldId + ':' + req.session.id) + }) + }) + }) + + store.set = function set () { + count++ + return _set.apply(this, arguments) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function (err, res) { + if (err) return done(err) + var ids = res.text.split(':') + assert.strictEqual(sid(res), ids[1]) + assert.notStrictEqual(sid(res), ids[0]) + assert.strictEqual(count, 1) + done() + }) + }) }) describe('.reload()', function () {