diff --git a/README.md b/README.md index 45e68a98..cd5b6456 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# express-session +# express-session-better [![NPM Version][npm-version-image]][npm-url] [![NPM Downloads][npm-downloads-image]][node-url] @@ -12,13 +12,13 @@ This is a [Node.js](https://nodejs.org/en/) module available through the [`npm install` command](https://docs.npmjs.com/getting-started/installing-npm-packages-locally): ```sh -$ npm install express-session +$ npm install express-session-better ``` ## API ```js -var session = require('express-session') +var session = require('express-session-better') ``` ### session(options) @@ -42,7 +42,7 @@ For a list of stores, see [compatible session stores](#compatible-session-stores #### Options -`express-session` accepts these properties in the options object. +`express-session-better` accepts these properties in the options object. ##### cookie @@ -270,6 +270,19 @@ will add an empty Passport object to the session for use after a user is authenticated, which will be treated as a modification to the session, causing it to be saved. *This has been fixed in PassportJS 0.3.0* +##### checkTouchModified + +If set to `false`, unmodified sessions are always touched. However, when +`rolling` is disabled, the expiration date in the cookie can differ from that +which is in the store, since the cookie expiration date is only updated if the +session is modified, but the session in the store is updated regardless. + +If set to `true` and the store supports "touching", unmodified sessions are not +touched in the store at the end of each request. Manually calling +`session.touch()` will extend the cookie expiry, and call `store.touch()`, which +should implement server-side expiry extension. + +The default value is `false`. ##### secret **Required option** diff --git a/index.js b/index.js index 40a442ba..73d1104a 100644 --- a/index.js +++ b/index.js @@ -70,6 +70,7 @@ var defer = typeof setImmediate === 'function' * Setup session store with the given `options`. * * @param {Object} [options] + * @param {Boolean} [options.checkTouchModified] Enable check preventing touch if session is unmodified * @param {Object} [options.cookie] Options for cookie * @param {Function} [options.genid] * @param {String} [options.name=connect.sid] Session ID cookie name @@ -87,6 +88,9 @@ var defer = typeof setImmediate === 'function' function session(options) { var opts = options || {} + // get the touch modification checker option + var checkTouchModified = opts.checkTouchModified + // get the cookie options var cookieOptions = opts.cookie || {} @@ -207,6 +211,7 @@ function session(options) { var originalHash; var originalId; + var originalExpires; var savedHash; var touched = false @@ -223,7 +228,18 @@ function session(options) { return; } - if (!shouldSetCookie(req)) { + if (typeof req.sessionID !== 'string') { + debug('session ignored because of bogus req.sessionID %o', req.sessionID); + return false; + } + + // Instead of getting the hash multiple times, only call when needed, and then cache it + var hashCache; + function getHash() { + return hashCache ? hashCache : (hashCache = hash(req.session)); + } + + if (!shouldSetCookie(req, getHash)) { return; } @@ -233,7 +249,7 @@ function session(options) { return; } - if (!touched) { + if (!touched && !checkTouchModified) { // touch session req.session.touch() touched = true @@ -325,13 +341,25 @@ function session(options) { return _end.call(res, chunk, encoding); } - if (!touched) { + // cannot set cookie without a session ID + if (typeof req.sessionID !== 'string') { + debug('session ignored because of bogus req.sessionID %o', req.sessionID); + return _end.call(res, chunk, encoding); + } + + if (!touched && !checkTouchModified) { // touch session req.session.touch() touched = true } - if (shouldSave(req)) { + // Instead of getting the hash multiple times, only call when needed, and then cache it + var hashCache; + function getHash() { + return hashCache ? hashCache : (hashCache = hash(req.session)); + } + + if (shouldSave(req, getHash)) { req.session.save(function onsave(err) { if (err) { defer(next, err); @@ -341,7 +369,7 @@ function session(options) { }); return writetop(); - } else if (storeImplementsTouch && shouldTouch(req)) { + } else if (storeImplementsTouch && shouldTouch(req, getHash)) { // store implements touch method debug('touching'); store.touch(req.sessionID, req.session, function ontouch(err) { @@ -364,6 +392,7 @@ function session(options) { store.generate(req); originalId = req.sessionID; originalHash = hash(req.session); + originalExpires = req.session.cookie.expires; wrapmethods(req.session); } @@ -372,6 +401,7 @@ function session(options) { store.createSession(req, sess) originalId = req.sessionID originalHash = hash(sess) + originalExpires = req.session.cookie.expires if (!resaveSession) { savedHash = originalHash @@ -422,13 +452,13 @@ function session(options) { } // check if session has been modified - function isModified(sess) { - return originalId !== sess.id || originalHash !== hash(sess); + function isModified(sess, getHash) { + return originalId !== sess.id || originalHash !== getHash(); } // check if session has been saved - function isSaved(sess) { - return originalId === sess.id && savedHash === hash(sess); + function isSaved(sess, getHash) { + return originalId === sess.id && savedHash === getHash(); } // determine if session should be destroyed @@ -437,39 +467,23 @@ function session(options) { } // determine if session should be saved to store - function shouldSave(req) { - // cannot set cookie without a session ID - if (typeof req.sessionID !== 'string') { - debug('session ignored because of bogus req.sessionID %o', req.sessionID); - return false; - } - + function shouldSave(req, getHash) { return !saveUninitializedSession && !savedHash && cookieId !== req.sessionID - ? isModified(req.session) - : !isSaved(req.session) + ? isModified(req.session, getHash) + : !isSaved(req.session, getHash) } // determine if session should be touched - function shouldTouch(req) { - // cannot set cookie without a session ID - if (typeof req.sessionID !== 'string') { - debug('session ignored because of bogus req.sessionID %o', req.sessionID); - return false; - } - - return cookieId === req.sessionID && !shouldSave(req); + function shouldTouch(req, getHash) { + return cookieId === req.sessionID && !shouldSave(req, getHash) && + (!checkTouchModified || originalExpires !== req.session.cookie.expires || isModified(req.session, getHash)); } // determine if cookie should be set on response - function shouldSetCookie(req) { - // cannot set cookie without a session ID - if (typeof req.sessionID !== 'string') { - return false; - } - + function shouldSetCookie(req, getHash) { return cookieId !== req.sessionID - ? saveUninitializedSession || isModified(req.session) - : rollingSessions || req.session.cookie.expires != null && isModified(req.session); + ? saveUninitializedSession || isModified(req.session, getHash) + : rollingSessions || req.session.cookie.expires != null && isModified(req.session, getHash); } // generate a session if the browser doesn't send a sessionID diff --git a/package.json b/package.json index 711e9866..1350b12f 100644 --- a/package.json +++ b/package.json @@ -1,13 +1,15 @@ { - "name": "express-session", - "version": "1.17.3", - "description": "Simple session middleware for Express", + "name": "express-session-better", + "version": "2.17.4", + "description": "Simple session middleware for Express, now with proper touch management and improved performance", "author": "TJ Holowaychuk (http://tjholowaychuk.com)", "contributors": [ - "Douglas Christopher Wilson ", - "Joe Wagner " + "Shahyar Ghobadpour " ], - "repository": "expressjs/session", + "repository": { + "type": "git", + "url": "git+https://github.com/shahyar/session.git" + }, "license": "MIT", "dependencies": { "cookie": "0.4.2", @@ -43,5 +45,13 @@ "test-ci": "nyc --reporter=lcov --reporter=text npm test", "test-cov": "nyc npm test", "version": "node scripts/version-history.js && git add HISTORY.md" + }, + "bugs": { + "url": "https://github.com/shahyar/session/issues" + }, + "homepage": "https://github.com/shahyar/session#readme", + "main": "index.js", + "directories": { + "test": "test" } } diff --git a/test/session.js b/test/session.js index 7416b261..8b09157d 100644 --- a/test/session.js +++ b/test/session.js @@ -1192,6 +1192,153 @@ describe('session()', function(){ }) }); + describe('checkTouchModified option', function(){ + describe('when false', function(){ + it('should touch unmodified sessions', function(done){ + var store = new session.MemoryStore() + var server = createServer({ checkTouchModified: false, store: store, resave: false }, function (req, res) { + req.session.user = 'bob'; + res.end() + }) + + var touched = false + var _touch = store.touch; + store.touch = function touch(sid, sess, callback) { + touched = true; + _touch.call(store, sid, sess, callback); + } + + assert.strictEqual(touched, false); + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function(err, res){ + if (err) return done(err); + + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(200, function (err, res) { + assert.strictEqual(touched, true); + done(); + }) + }); + }); + }); + + describe('when true', function(){ + it('should not touch unmodified sessions', function(done){ + var store = new session.MemoryStore() + var server = createServer({ checkTouchModified: true, store: store, resave: false }, function (req, res) { + if (!req.session.user) { + req.session.user = 'bob'; + } + res.end() + }) + + var touched = false + var _touch = store.touch; + store.touch = function touch(sid, sess, callback) { + touched = true; + _touch.call(store, sid, sess, callback); + } + + assert.strictEqual(touched, false); + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function(err, res){ + if (err) return done(err); + + setTimeout(function () { + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(200, function (err, res) { + assert.strictEqual(touched, false); + done(); + }) + }, 10); + }); + }); + + it('should touch manually touched sessions', function(done){ + var store = new session.MemoryStore() + var server = createServer({ checkTouchModified: true, store: store, resave: false }, function (req, res) { + if (!req.session.user) { + req.session.touch(); + } else { + req.session.user = 'bob'; + } + res.end() + }) + + var touched = false + var _touch = store.touch; + store.touch = function touch(sid, sess, callback) { + touched = true; + _touch.call(store, sid, sess, callback); + } + + assert.strictEqual(touched, false); + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function(err, res){ + if (err) return done(err); + + setTimeout(function () { + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(200, function (err, res) { + assert.strictEqual(touched, true); + done(); + }) + }, 10); + }); + }); + + it('should not touch modified sessions, but save them instead', function(done){ + var store = new session.MemoryStore() + var server = createServer({ checkTouchModified: true, store: store, resave: false }, function (req, res) { + req.session.count = req.session.count || 0 + req.session.count++ + res.end('hits: ' + req.session.count) + }) + + var touched = false + var _touch = store.touch; + store.touch = function touch(sid, sess, callback) { + touched = true; + _touch.call(store, sid, sess, callback); + } + + assert.strictEqual(touched, false); + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, 'hits: 1', function(err, res){ + if (err) return done(err); + + setTimeout(function () { + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(200, 'hits: 2', function (err, res) { + assert.strictEqual(touched, false); + done(); + }) + }, 10); + }); + }); + }); + }); + describe('secret option', function () { it('should reject empty arrays', function () { assert.throws(createServer.bind(null, { secret: [] }), /secret option array/);