Skip to content

Only touch sessions in the store when rolling sessions are enabled #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Only touch sessions in the store when rolling sessions are enabled #531

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cc345a7
Only touch sessions in the store when rolling sessions are enabled
Dec 19, 2017
64ba3e5
Fix tests broken with rolling session change to touch()
Dec 19, 2017
b6fb975
Add test to get coverage back to 100%
Dec 19, 2017
e313529
Add tests to check touch() suppression when rolling is turned off
Dec 20, 2017
abc7b0a
Add option alwaysTouchUnmodified to force unmodified sessions to alwa…
Dec 20, 2017
25021df
Fix logic error in documentation of new option
Dec 20, 2017
8543451
Revert changes made to tests now that existing behaviour is the default
Dec 20, 2017
8d0180b
Move new touching logic into shouldTouch() where it belongs
Jan 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ function session(options) {
});

return writetop();
} else if (storeImplementsTouch && shouldTouch(req)) {
} else if (rollingSessions && storeImplementsTouch && shouldTouch(req)) {
// store implements touch method
debug('touching');
store.touch(req.sessionID, req.session, function ontouch(err) {
Expand Down
27 changes: 24 additions & 3 deletions test/session.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

process.env.NO_DEPRECATION = 'express-session';

var after = require('after')
Expand Down Expand Up @@ -1074,7 +1073,7 @@ describe('session()', function(){
it('should pass session touch error', function (done) {
var cb = after(2, done)
var store = new session.MemoryStore()
var server = createServer({ store: store, resave: false }, function (req, res) {
var server = createServer({ store: store, resave: false, rolling: true }, function (req, res) {
req.session.hit = true
res.end('session saved')
})
Expand All @@ -1099,6 +1098,28 @@ describe('session()', function(){
.end(cb)
})
})

it('should not touch with bogus req.sessionID', function (done) {
var store = new session.MemoryStore()
var server = createServer({ store: store, resave: false, rolling: true }, function (req, res) {
req.sessionID = function () {}
req.session.test1 = 1
req.session.test2 = 'b'
res.end()
})

request(server)
.get('/')
.expect(shouldNotHaveHeader('Set-Cookie'))
.expect(200, function (err) {
if (err) return done(err)
store.length(function (err, length) {
if (err) return done(err)
assert.equal(length, 0)
done()
})
})
})
});

describe('saveUninitialized option', function(){
Expand Down Expand Up @@ -1702,7 +1723,7 @@ describe('session()', function(){
describe('.touch()', function () {
it('should reset session expiration', function (done) {
var store = new session.MemoryStore()
var server = createServer({ resave: false, store: store, cookie: { maxAge: min } }, function (req, res) {
var server = createServer({ resave: false, rolling: true, store: store, cookie: { maxAge: min } }, function (req, res) {
req.session.hit = true
req.session.touch()
res.end()
Expand Down