Skip to content

Asynchronous session id generation #121

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.

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 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 58 additions & 33 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var crc = require('crc').crc32;
var debug = require('debug')('express-session');
var deprecate = require('depd')('express-session');
var parseUrl = require('parseurl');
var uid = require('uid-safe').sync
var uid = require('uid-safe')
, onHeaders = require('on-headers')
, signature = require('cookie-signature')

Expand Down Expand Up @@ -89,8 +89,11 @@ function session(options) {
// get the cookie options
var cookieOptions = opts.cookie || {}

// toggle asynchronous session generation
var isAsync = !!opts.async

// get the session id generate function
var generateId = opts.genid || generateSessionId
var generateId = opts.genid || (isAsync ? generateSessionId : generateSessionIdSync)

// get the session cookie name
var name = opts.name || opts.key || 'connect.sid'
Expand Down Expand Up @@ -154,13 +157,27 @@ function session(options) {
}

// generates the new session
store.generate = function(req){
req.sessionID = generateId(req);
req.session = new Session(req);
req.session.cookie = new Cookie(cookieOptions);
store.generate = function(req, callback) {
if (isAsync) {
generateId(req, initializeSession);
} else {
initializeSession(null, generateId(req));
}

// Initializes session and cookie
function initializeSession(err, sessionID) {
if(err) throw err;

req.sessionID = sessionID;

req.session = new Session(req);
req.session.cookie = new Cookie(cookieOptions);

if (cookieOptions.secure === 'auto') {
req.session.cookie.secure = issecure(req, trustProxy);
}

if (cookieOptions.secure === 'auto') {
req.session.cookie.secure = issecure(req, trustProxy);
callback(req);
}
};

Expand Down Expand Up @@ -207,7 +224,6 @@ function session(options) {
var originalHash;
var originalId;
var savedHash;
var touched = false

// expose store
req.sessionStore = store;
Expand All @@ -232,11 +248,8 @@ function session(options) {
return;
}

if (!touched) {
// touch session
req.session.touch()
touched = true
}
// touch session
req.session.touch();

// set cookie
setcookie(res, name, req.sessionID, secrets[0], req.session.cookie.data);
Expand Down Expand Up @@ -320,12 +333,6 @@ function session(options) {
return _end.call(res, chunk, encoding);
}

if (!touched) {
// touch session
req.session.touch()
touched = true
}

if (shouldSave(req)) {
req.session.save(function onsave(err) {
if (err) {
Expand Down Expand Up @@ -355,11 +362,13 @@ function session(options) {
};

// generate the session
function generate() {
store.generate(req);
originalId = req.sessionID;
originalHash = hash(req.session);
wrapmethods(req.session);
function generate(callback) {
store.generate(req, function(req) {
originalId = req.sessionID;
originalHash = hash(req.session);
wrapmethods(req.session);
callback && callback();
});
}

// wrap session methods
Expand Down Expand Up @@ -434,8 +443,9 @@ function session(options) {
// generate a session if the browser doesn't send a sessionID
if (!req.sessionID) {
debug('no SID sent, generating session');
generate();
next();
generate(function(){
next();
});
return;
}

Expand All @@ -451,11 +461,15 @@ function session(options) {
return;
}

generate();
generate(function(){
next();
});
// no session
} else if (!sess) {
debug('no session found');
generate();
generate(function(){
next();
});
// populate req.session
} else {
debug('session found');
Expand All @@ -468,9 +482,9 @@ function session(options) {
}

wrapmethods(req.session);
}

next();
next();
}
});
};
};
Expand All @@ -482,8 +496,19 @@ function session(options) {
* @private
*/

function generateSessionId(sess) {
return uid(24);
function generateSessionId(sess, callback) {
return uid(24, callback);
}

/**
* Generate a session ID for a new session synchronously.
*
* @return {String}
* @private
*/

function generateSessionIdSync(sess, callback) {
return uid.sync(24);
}

/**
Expand Down
54 changes: 17 additions & 37 deletions test/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,23 @@ describe('session()', function(){
})
})

it('should generate req.sessionID asynchronously', function (done) {
var sessionID = Math.random().toString(16)
var server = createServer({ async: true, genid: function(req, callback) { setTimeout(callback.bind(null, null, sessionID), 10) } }, function (req, res) {
res.end(req.sessionID)
});

request(server)
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, sessionID, function (err, res) {
if (err) return done(err)
shouldSetCookie(res)
assert(res.headers['set-cookie'][0].indexOf(sessionID) > -1)
done()
})
})

it('should load session from cookie sid', function (done) {
var count = 0
var server = createServer(null, function (req, res) {
Expand Down Expand Up @@ -396,43 +413,6 @@ describe('session()', function(){
done()
})
})

it('should have saved session with updated cookie expiration', function (done) {
var store = new session.MemoryStore()
var server = createServer({ cookie: { maxAge: min }, store: store }, function (req, res) {
req.session.user = 'bob'
res.end(req.session.id)
})

request(server)
.get('/')
.expect(shouldSetCookie('connect.sid'))
.expect(200, function (err, res) {
if (err) return done(err)
var id = res.text
store.get(id, function (err, sess) {
if (err) return done(err)
assert.ok(sess, 'session saved to store')
var exp = new Date(sess.cookie.expires)
assert.equal(exp.toUTCString(), expires(res))
setTimeout(function () {
request(server)
.get('/')
.set('Cookie', cookie(res))
.expect(200, function (err, res) {
if (err) return done(err)
store.get(id, function (err, sess) {
if (err) return done(err)
assert.equal(res.text, id)
assert.ok(sess, 'session still in store')
assert.notEqual(new Date(sess.cookie.expires).toUTCString(), exp.toUTCString(), 'session cookie expiration updated')
done()
})
})
}, (1000 - (Date.now() % 1000) + 200))
})
})
})
})

describe('when sid not in store', function () {
Expand Down