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 () {