diff --git a/README.md b/README.md index 239d49b7..9bb7dc3d 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,14 @@ Force a cookie to be set on every response. This resets the expiration date. The default value is `false`. +##### saveBeforeRedirect + +Tells the module to completly buffer the response when `location` header is set. +This is because the browser is performing redirection immediately after the `location` +header is parsed without waiting for the store to finish saving. + +The default value is `false`. + ##### saveUninitialized Forces a session that is "uninitialized" to be saved to the store. A session is diff --git a/index.js b/index.js index 6f6d1b52..bef22db8 100644 --- a/index.js +++ b/index.js @@ -89,7 +89,8 @@ function session(options){ , cookie = options.cookie || {} , trustProxy = options.proxy , storeReady = true - , rollingSessions = options.rolling || false; + , rollingSessions = options.rolling || false + , saveBeforeLocationHeader = options.saveBeforeRedirect || false; var resaveSession = options.resave; var saveUninitializedSession = options.saveUninitialized; var secret = options.secret; @@ -171,6 +172,9 @@ function session(options){ var originalHash; var originalId; var savedHash; + var buffered; + var bufferedChunks; + var writeHeadLater; // expose store req.sessionStore = store; @@ -199,10 +203,63 @@ function session(options){ setcookie(res, name, req.sessionID, secrets[0], cookie.data); }); + + var _write = res.write; + + if (saveBeforeLocationHeader) { + var _writeHead = res.writeHead; + var headWritten = false; + res.writeHead = function writeHead(statusCode, reason, obj) { + if (headWritten) return; + headWritten = true; + // search for location header only for 3xx status codes + if (statusCode < 300 || statusCode >= 400) { + return _writeHead.call(res, statusCode, reason, obj); + } + + this.statusCode = statusCode; + + if ('string' != typeof(reason)) { + obj = reason; + reason = undefined; + } + + if (obj) { + var keys = Object.keys(obj); + for (var i = 0, l = keys.length; i < l; i++) { + var k = keys[i]; + if (k) res.setHeader(k, obj[k]); + } + } + + // we have a `location` header so we must buffer all writes + if (res.getHeader('location') != null) { + debug('redirect found, buffering response') + buffered = true; + bufferedChunks = new Buffer(0); + writeHeadLater = function() { _writeHead.call(res, statusCode, reason); }; + } else { + _writeHead.call(res, statusCode, reason); + } + }; + + var __write = _write; + res.write = _write = function write(chunk, encoding, callback) { + if (!headWritten) res.writeHead(this.statusCode); + + if (buffered) { + bufferedChunks = Buffer.concat([bufferedChunks, !Buffer.isBuffer(chunk) ? new Buffer(chunk, encoding) : chunk]); + if ('function' === typeof(callback)) setImmediate(callback); + return true; + } else { + return __write.apply(res, arguments); + } + }; + }; + // proxy end() to commit the session var _end = res.end; - var _write = res.write; var ended = false; res.end = function end(chunk, encoding) { if (ended) { @@ -216,12 +273,17 @@ function session(options){ function writeend() { if (sync) { - ret = _end.call(res, chunk, encoding); + if (chunk) _write.call(res, chunk, encoding); sync = false; - return; + } + + if (buffered) { + if (writeHeadLater) writeHeadLater(); + __write.call(res, bufferedChunks); } - _end.call(res); + ret = _end.call(res); + return ret; } function writetop() { @@ -305,7 +367,7 @@ function session(options){ return writetop(); } - + return _end.call(res, chunk, encoding); }; diff --git a/test/session.js b/test/session.js index f592ef3f..ad7961fe 100644 --- a/test/session.js +++ b/test/session.js @@ -368,6 +368,159 @@ describe('session()', function(){ }) }) }) + + describe('when location header is set', function () { + it('should buffer the response #1', function (done) { + var saved = false + var success = false + var store = new session.MemoryStore() + var server = createServer({ store: store, saveBeforeRedirect: true }, function (req, res) { + req.session.hit = true + res.writeHead(308, {location: 'http://xxx.com'}); + res.end('custom body'); + }) + + var _set = store.set + store.set = function set(sid, sess, callback) { + setTimeout(function () { + _set.call(store, sid, sess, function (err) { + saved = true + callback(err) + }) + }, 200) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect('location', 'http://xxx.com') + .expect(308, 'custom body', function (err) { + if (err) return done(err) + assert.ok(success) + done() + }) + .req.on('response', function() { + if (saved) success = true; + }) + }) + + it('should buffer the response #2', function (done) { + var saved = false + var success = false + var store = new session.MemoryStore() + var server = createServer({ store: store, saveBeforeRedirect: true }, function (req, res) { + req.session.hit = true + res.setHeader('Location', 'http://xxx.com') + res.statusCode = 308 + res.write('a'); + res.write('b'); + res.end('c'); + }) + + var _set = store.set + store.set = function set(sid, sess, callback) { + setTimeout(function () { + _set.call(store, sid, sess, function (err) { + saved = true + callback(err) + }) + }, 200) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect('location', 'http://xxx.com') + .expect(308, 'abc', function (err) { + if (err) return done(err) + assert.ok(success) + done() + }) + .req.on('response', function() { + if (saved) success = true; + }) + }) + + it('should buffer the response #3 (synchronous store)', function(done){ + var store = new SyncStore() + var server = createServer({ store: store, saveBeforeRedirect: true }, function (req, res) { + res.setHeader('Location', 'http://xxx.com') + res.statusCode = 308 + res.end('response') + }) + + request(server) + .get('/') + .expect(308, 'response', done) + }) + + it('should not buffer the response #1', function(done){ + var saved = false + var success = true + var store = new session.MemoryStore() + var server = createServer({ store: store, saveBeforeRedirect: true }, function (req, res) { + req.session.hit = true + res.writeHead(200, {location: 'http://xxx.com'}); + res.end('custom body'); + }) + + var _set = store.set + store.set = function set(sid, sess, callback) { + setTimeout(function () { + _set.call(store, sid, sess, function (err) { + saved = true + callback(err) + }) + }, 200) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect('location', 'http://xxx.com') + .expect(200, 'custom body', function (err) { + if (err) return done(err) + assert.ok(success) + done() + }) + .req.on('response', function() { + if (saved) success = false; + }) + }) + + it('should not buffer the response #2', function(done){ + var saved = false + var success = true + var store = new session.MemoryStore() + var server = createServer({ store: store, saveBeforeRedirect: true }, function (req, res) { + req.session.hit = true + res.writeHead(302); + res.end('custom body'); + }) + + var _set = store.set + store.set = function set(sid, sess, callback) { + setTimeout(function () { + _set.call(store, sid, sess, function (err) { + saved = true + callback(err) + }) + }, 200) + } + + request(server) + .get('/') + .expect(shouldSetCookie('connect.sid')) + .expect(302, 'custom body', function (err) { + if (err) return done(err) + assert.ok(success) + done() + }) + .req.on('response', function() { + if (saved) success = false; + }) + }) + }) describe('when sid not in store', function () { it('should create a new session', function (done) {