Skip to content

Commit 683a8f8

Browse files
Paul McCleandougwilson
Paul McClean
authored andcommitted
feat: Option to allow regenerate() to preserve old session
Add preserve option to session.regenerate() function for use cases when it is desirable to regenerate the session, but not destroy the old session immediately. For example when there could be multiple ajax requests inflight with the old session id. Existing functionality is preserved if the option is omitted. Existing test was improved to cover session destuction and new test was added for the preserve case.
1 parent 15fe121 commit 683a8f8

File tree

4 files changed

+105
-11
lines changed

4 files changed

+105
-11
lines changed

Diff for: README.md

+19-3
Original file line numberDiff line numberDiff line change
@@ -304,15 +304,31 @@ app.get('/', function(req, res, next) {
304304
})
305305
```
306306

307-
#### Session.regenerate(callback)
307+
#### Session.regenerate([options], callback)
308308

309-
To regenerate the session simply invoke the method. Once complete,
309+
Regenerates the session for the current request. Once complete,
310310
a new SID and `Session` instance will be initialized at `req.session`
311311
and the `callback` will be invoked.
312312

313+
#### Options
314+
315+
If the options argument is omitted, the options will take their default values.
316+
The following options are supported:
317+
318+
##### destroy
319+
320+
Specifies whether the existing session should be destroyed or not. Defaults
321+
to `true`, and the existing session will be destroyed. If this is not desirable,
322+
set to `false` to preserve the existing session.
323+
313324
```js
314325
req.session.regenerate(function(err) {
315-
// will have a new session here
326+
// existing session is destroyed and will have a new session here
327+
})
328+
```
329+
```js
330+
req.session.regenerate({ destroy: false }, function(err) {
331+
// existing session is preserved but will have a new session here
316332
})
317333
```
318334

Diff for: session/session.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ defineMethod(Session.prototype, 'destroy', function destroy(fn) {
120120
* @api public
121121
*/
122122

123-
defineMethod(Session.prototype, 'regenerate', function regenerate(fn) {
124-
this.req.sessionStore.regenerate(this.req, fn);
123+
defineMethod(Session.prototype, 'regenerate', function regenerate(options, fn) {
124+
this.req.sessionStore.regenerate(this.req, options, fn);
125125
return this;
126126
});
127127

Diff for: session/store.js

+24-5
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,31 @@ util.inherits(Store, EventEmitter)
4747
* @api public
4848
*/
4949

50-
Store.prototype.regenerate = function(req, fn){
50+
Store.prototype.regenerate = function regenerate (req, options, fn) {
51+
var cb = fn
52+
var opts = options || {}
5153
var self = this;
52-
this.destroy(req.sessionID, function(err){
53-
self.generate(req);
54-
fn(err);
55-
});
54+
55+
if (typeof options === 'function') {
56+
fn = options
57+
opts = {}
58+
}
59+
60+
var destroy = opts.destroy !== undefined
61+
? opts.destroy
62+
: true
63+
64+
if (destroy) {
65+
this.destroy(req.sessionID, function (err) {
66+
self.generate(req)
67+
fn(err)
68+
})
69+
} else {
70+
process.nextTick(function () {
71+
self.generate(req)
72+
fn(null)
73+
})
74+
}
5675
};
5776

5877
/**

Diff for: test/session.js

+60-1
Original file line numberDiff line numberDiff line change
@@ -1558,7 +1558,8 @@ describe('session()', function(){
15581558

15591559
describe('.regenerate()', function(){
15601560
it('should destroy/replace the previous session', function(done){
1561-
var server = createServer(null, function (req, res) {
1561+
var store = new session.MemoryStore()
1562+
var server = createServer({store: store}, function (req, res) {
15621563
var id = req.session.id
15631564
req.session.regenerate(function (err) {
15641565
if (err) res.statusCode = 500
@@ -1574,11 +1575,41 @@ describe('session()', function(){
15741575
request(server)
15751576
.get('/')
15761577
.set('Cookie', cookie(res))
1578+
.expect(shouldDestroySessionInStore(store))
15771579
.expect(shouldSetCookie('connect.sid'))
15781580
.expect(shouldSetCookieToDifferentSessionId(sid(res)))
15791581
.expect(200, 'false', done)
15801582
});
15811583
})
1584+
1585+
it('should replace but not destroy the previous session when preserve flag is set', function(done){
1586+
var store = new session.MemoryStore()
1587+
var server = createServer({store: store}, function (req, res) {
1588+
var id = req.session.id
1589+
req.session.regenerate({destroy: false}, function (err) {
1590+
if (err) res.statusCode = 500
1591+
res.end(String(req.session.id === id))
1592+
})
1593+
})
1594+
1595+
request(server)
1596+
.get('/')
1597+
.expect(shouldSetCookie('connect.sid'))
1598+
.expect(200, function (err, res) {
1599+
if (err) return done(err)
1600+
var id = sid(res)
1601+
request(server)
1602+
.get('/')
1603+
.set('Cookie', cookie(res))
1604+
.expect(shouldSetCookie('connect.sid'))
1605+
.expect(shouldNotDestroySessionInStore(store))
1606+
.expect(200, 'false', function (err, res) {
1607+
if (err) return done(err)
1608+
assert.notEqual(sid(res), id)
1609+
done();
1610+
});
1611+
});
1612+
})
15821613
})
15831614

15841615
describe('.reload()', function () {
@@ -2216,6 +2247,34 @@ function parseSetCookie (header) {
22162247
return cookie
22172248
}
22182249

2250+
function shouldDestroySessionInStore(store) {
2251+
var _destroy = store.destroy
2252+
var count = 0
2253+
2254+
store.destroy = function destroy () {
2255+
count++
2256+
return _destroy.apply(this, arguments)
2257+
}
2258+
2259+
return function () {
2260+
assert.ok(count === 1, 'should destroy session in store')
2261+
}
2262+
}
2263+
2264+
function shouldNotDestroySessionInStore(store) {
2265+
var _destroy = store.destroy
2266+
var count = 0
2267+
2268+
store.destroy = function destroy () {
2269+
count++
2270+
return _destroy.apply(this, arguments)
2271+
}
2272+
2273+
return function () {
2274+
assert.ok(count === 0, 'should not destroy session in store')
2275+
}
2276+
}
2277+
22192278
function shouldNotHaveHeader(header) {
22202279
return function (res) {
22212280
assert.ok(!(header.toLowerCase() in res.headers), 'should not have ' + header + ' header')

0 commit comments

Comments
 (0)