From 0477b5a0933c07ea3884aa085f7571c58692650c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Tue, 28 Dec 2021 14:25:03 -0500 Subject: [PATCH] session: call store.touch() when manually touched --- README.md | 28 ++++++- index.js | 56 ++++++++++--- session/session.js | 17 +++- test/session.js | 201 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 286 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 2d1963b7..72b73231 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,26 @@ The default value is `undefined`. if there is a direct TLS/SSL connection. - `undefined` Uses the "trust proxy" setting from express +##### propagateTouch + +Default: `false`. The default is `false` for backwards compatibility reasons +only; you are encouraged to set this to `true`. Using `false` is deprecated; the +default behavior will change to `true` in a future version and this option will +be removed. + +If `true`, calling `req.session.touch()` also does the following: + + - Suppresses the middleware's automatic call to `req.session.touch()` + (assuming it hasn't already happened). + - Immediately calls `store.touch()` if the session is initialized (changed + from its default state) or if `saveUninitialized` is enabled. + - Suppresses the middleware's automatic call to `store.touch()` (assuming it + hasn't already happened) if a call to `store.touch()` was attempted by + `req.session.touch()`. + +If `false`, `req.session.touch()` will not call `store.touch()` nor will it +suppress the automatic calls to `req.session.touch()` and `store.touch()`. + ##### resave Forces the session to be saved back to the session store, even if the session @@ -387,10 +407,12 @@ req.session.save(function(err) { }) ``` -#### Session.touch() +#### Session.touch(callback) -Updates the `.maxAge` property. Typically this is -not necessary to call, as the session middleware does this for you. +Updates the `.maxAge` property and maybe calls `store.touch()` (see the +`propagateTouch` option). It is not usually necessary to call this method, as +the session middleware does it for you. The callback is optional; if provided, +it will be called with an error argument when done. ### req.session.id diff --git a/index.js b/index.js index d7efeab9..1a886301 100644 --- a/index.js +++ b/index.js @@ -74,6 +74,7 @@ var defer = typeof setImmediate === 'function' * @param {Function} [options.genid] * @param {String} [options.name=connect.sid] Session ID cookie name * @param {Boolean} [options.proxy] + * @param {Boolean} [options.propagateTouch] Whether session.touch() should call store.touch() * @param {Boolean} [options.resave] Resave unmodified sessions back to the store * @param {Boolean} [options.rolling] Enable/disable rolling session expiration * @param {Boolean} [options.saveUninitialized] Save uninitialized sessions to the store @@ -96,6 +97,11 @@ function session(options) { // get the session cookie name var name = opts.name || opts.key || 'connect.sid' + var propagateTouch = opts.propagateTouch; + if (!propagateTouch) { + deprecate('falsy propagateTouch option; set to true'); + } + // get the session store var store = opts.store || new MemoryStore() @@ -209,6 +215,19 @@ function session(options) { var originalId; var savedHash; var touched = false + var touchedStore = false; + + function autoTouch() { + if (touched) return; + // For legacy reasons, auto-touch does not touch the session in the store. That is done later. + var backup = propagateTouch; + propagateTouch = false; + try { + req.session.touch(); + } finally { + propagateTouch = backup; + } + } // expose store req.sessionStore = store; @@ -233,11 +252,7 @@ function session(options) { return; } - if (!touched) { - // touch session - req.session.touch() - touched = true - } + autoTouch(); // set cookie setcookie(res, name, req.sessionID, secrets[0], req.session.cookie.data); @@ -325,11 +340,7 @@ function session(options) { return _end.call(res, chunk, encoding); } - if (!touched) { - // touch session - req.session.touch() - touched = true - } + autoTouch(); if (shouldSave(req)) { req.session.save(function onsave(err) { @@ -394,6 +405,7 @@ function session(options) { function wrapmethods(sess) { var _reload = sess.reload var _save = sess.save; + var _touch = sess.touch; function reload(callback) { debug('reloading %s', this.id) @@ -406,6 +418,21 @@ function session(options) { _save.apply(this, arguments); } + function touch(callback) { + debug('touching %s', this.id); + var cb = callback || function (err) { if (err) throw err; }; + var touchStore = propagateTouch && storeImplementsTouch && + // Don't touch the store if the session won't be saved to the store. + (saveUninitializedSession || isModified(this)); + _touch.call(this, touchStore ? function (err) { + if (err) return cb(err); + store.touch(this.id, this, cb); + touchedStore = true; // Set synchronously regardless of success/failure. + } : cb); + touched = true; // Set synchronously regardless of success/failure. + return this; + } + Object.defineProperty(sess, 'reload', { configurable: true, enumerable: false, @@ -419,6 +446,13 @@ function session(options) { value: save, writable: true }); + + Object.defineProperty(sess, 'touch', { + configurable: true, + enumerable: false, + value: touch, + writable: true + }); } // check if session has been modified @@ -457,7 +491,7 @@ function session(options) { return false; } - return cookieId === req.sessionID && !shouldSave(req); + return !touchedStore && cookieId === req.sessionID && !shouldSave(req); } // determine if cookie should be set on response diff --git a/session/session.js b/session/session.js index fee7608c..58846a45 100644 --- a/session/session.js +++ b/session/session.js @@ -7,6 +7,16 @@ 'use strict'; +/** + * Node.js 0.8+ async implementation. + * @private + */ + +/* istanbul ignore next */ +var defer = typeof setImmediate === 'function' + ? setImmediate + : function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) } + /** * Expose Session. */ @@ -40,12 +50,15 @@ function Session(req, data) { * the cookie from expiring when the * session is still active. * + * @param {Function} fn optional done callback * @return {Session} for chaining * @api public */ -defineMethod(Session.prototype, 'touch', function touch() { - return this.resetMaxAge(); +defineMethod(Session.prototype, 'touch', function touch(fn) { + this.resetMaxAge(); + if (fn) defer(fn); + return this; }); /** diff --git a/test/session.js b/test/session.js index 425ed2bc..a8fb1873 100644 --- a/test/session.js +++ b/test/session.js @@ -16,6 +16,11 @@ var Cookie = require('../session/cookie') var min = 60 * 1000; +/* istanbul ignore next */ +var defer = typeof setImmediate === 'function' + ? setImmediate + : function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) } + describe('session()', function(){ it('should export constructors', function(){ assert.strictEqual(typeof session.Session, 'function') @@ -874,6 +879,187 @@ describe('session()', function(){ }) }) + describe('propagateTouch option', function () { + it('defaults to false', function (done) { + var called = false; + var store = new session.MemoryStore(); + store.touch = function touch(sid, sess, callback) { called = true; defer(callback); }; + var server = createServer({ store: store }, function (req, res) { + assert(!called); + req.session.modified = true; + req.session.touch(function (err) { + if (err) throw err; + assert(!called); + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + it('does not call store.touch() if unimplemented', function (done) { + var store = new session.MemoryStore(); + store.touch = null; + var server = createServer({ propagateTouch: true, store: store }, function (req, res) { + req.session.modified = true; + req.session.touch(function (err) { + if (err) throw err; + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + it('calls store.touch() if implemented', function (done) { + var store = new session.MemoryStore(); + var called = false; + store.touch = function touch(sid, sess, callback) { called = true; defer(callback); }; + var server = createServer({ propagateTouch: true, store: store }, function (req, res) { + req.session.modified = true; + assert(!called); + req.session.touch(function (err) { + if (err) throw err; + assert(called); + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + it('waits for store.touch() to complete', function (done) { + var store = new session.MemoryStore(); + var called = false; + store.touch = function touch(sid, sess, callback) { + setTimeout(function () { called = true; callback(); }, 100); + }; + var server = createServer({ propagateTouch: true, store: store }, function (req, res) { + req.session.modified = true; + assert(!called); + req.session.touch(function (err) { + if (err) throw err; + assert(called); + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + it('passes back store.touch() error', function (done) { + var store = new session.MemoryStore(); + store.touch = function touch(sid, sess, callback) { defer(callback, new Error('boom!')); }; + var server = createServer({ propagateTouch: true, store: store }, function (req, res) { + req.session.modified = true; + req.session.touch(function (err) { + assert(err != null); + assert.strictEqual(err.message, 'boom!'); + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + xit('suppresses automatic session.touch()', function (done) { + // TODO + }); + + xit('suppresses automatic session.touch() even on failure', function (done) { + // TODO + }); + + xit('only suppresses automatic session.touch() if session.touch() attempted', function (done) { + // TODO + }); + + it('suppresses automatic store.touch()', function (done) { + var store = new session.MemoryStore(); + var calls = 0; + store.touch = function touch(sid, sess, callback) { ++calls; defer(callback); }; + var doTouch = false; + var server = createServer({ propagateTouch: true, store: store }, function (req, res) { + req.session.modified = true; + var callsBefore = calls; + req.session.touch(function (err) { + if (err) throw err; + assert.strictEqual(calls, callsBefore + 1); + res.end(); + }); + }); + assert.strictEqual(calls, 0); + // Two requests must be made for this test because the middleware never calls store.touch() + // automatically on first request (it calls store.save() instead). + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(200, function (err, res) { + if (err) return done(err); + assert.strictEqual(calls, 1); + doTouch = true; + request(server) + .get('/') + .set('Cookie', cookie(res)) + .expect(200, function (err) { + if (err) return done(err); + assert.strictEqual(calls, 2); + done(); + }); + }); + }); + + xit('suppresses automatic store.touch() even on failure', function (done) { + // TODO + }); + + xit('only suppresses automatic store.touch() if store.touch() was attempted', function (done) { + // TODO + }); + + xit('keeps working after automatic touch', function (done) { + // TODO + }); + + it('always calls store.touch() when saveUninitialized=true', function (done) { + var called = false; + var store = new session.MemoryStore(); + store.touch = function touch(sid, sess, callback) { called = true; defer(callback); }; + var server = createServer({ + propagateTouch: true, + store: store, + saveUninitialized: true, + }, function (req, res) { + assert(!called); + req.session.touch(function (err) { + if (err) throw err; + assert(called); + res.end(); + }); + }); + request(server).get('/').expect(200, done); + }); + + it('calls store.touch() iff modified when saveUninitialized=false', function (done) { + var called = false; + var store = new session.MemoryStore(); + store.touch = function touch(sid, sess, callback) { called = true; defer(callback); }; + var server = createServer({ + propagateTouch: true, + store: store, + saveUninitialized: false, + }, function (req, res) { + assert(!called); + req.session.touch(function (err) { + if (err) throw err; + req.session.modified = true; + assert(!called); + req.session.touch(function (err) { + if (err) throw err; + assert(called); + res.end(); + }); + }); + }); + request(server).get('/').expect(200, done); + }); + }); + describe('rolling option', function(){ it('should default to false', function(done){ var server = createServer(null, function (req, res) { @@ -1810,6 +1996,21 @@ describe('session()', function(){ }) }) }) + + it('should call the callback asynchronously', function (done) { + var server = createServer(null, function (req, res) { + var i = 0; + req.session.touch(function (err) { + ++i; + res.end(); + }); + assert.strictEqual(i, 0); + }); + + request(server) + .get('/') + .expect(200, done); + }); }) describe('.cookie', function(){