Fix blank responses for store with synchronous ops

fixes #61
expressjs · Jul 7, 2014 · 901c8e0 · 901c8e0
1 parent 78b2db5
commit 901c8e0
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 2 deletions.
diff --git a/History.md b/History.md
@@ -1,3 +1,8 @@
+unreleased
+==========
+
+  * Fix blank responses for stores with synchronous operations
+
 1.6.3 / 2014-07-04
 ==================
 

diff --git a/index.js b/index.js
@@ -177,6 +177,9 @@ function session(options){
         return false;
       }
 
+      var ret;
+      var sync = true;
+
       if (chunk === undefined) {
         chunk = '';
       }
@@ -189,9 +192,22 @@ function session(options){
         store.destroy(req.sessionID, function(err){
           if (err) console.error(err.stack);
           debug('destroyed');
+
+          if (sync) {
+            ret = end.call(res, chunk, encoding);
+            sync = false;
+            return;
+          }
+
           end.call(res);
         });
-        return res.write(chunk, encoding);
+
+        if (sync) {
+          ret = res.write(chunk, encoding);
+          sync = false;
+        }
+
+        return ret;
       }
 
       // no session to save
@@ -207,9 +223,22 @@ function session(options){
         req.session.save(function(err){
           if (err) console.error(err.stack);
           debug('saved');
+
+          if (sync) {
+            ret = end.call(res, chunk, encoding);
+            sync = false;
+            return;
+          }
+
           end.call(res);
         });
-        return res.write(chunk, encoding);
+
+        if (sync) {
+          ret = res.write(chunk, encoding);
+          sync = false;
+        }
+
+        return ret;
       }
 
       return end.call(res, chunk, encoding);

diff --git a/test/session.js b/test/session.js
@@ -1357,6 +1357,44 @@ describe('session()', function(){
     })
   })
 
+  describe('synchronous store', function(){
+    it('should respond correctly on save', function(done){
+      var store = new SyncStore()
+      var server = createServer({ store: store }, function (req, res) {
+        req.session.count = req.session.count || 0
+        req.session.count++
+        res.end('hits: ' + req.session.count)
+      })
+
+      request(server)
+      .get('/')
+      .expect(200, 'hits: 1', done)
+    })
+
+    it('should respond correctly on destroy', function(done){
+      var store = new SyncStore()
+      var server = createServer({ store: store, unset: 'destroy' }, function (req, res) {
+        req.session.count = req.session.count || 0
+        var count = ++req.session.count
+        if (req.session.count > 1) {
+          req.session = null
+          res.write('destroyed\n')
+        }
+        res.end('hits: ' + count)
+      })
+
+      request(server)
+      .get('/')
+      .expect(200, 'hits: 1', function (err, res) {
+        if (err) return done(err)
+        request(server)
+        .get('/')
+        .set('Cookie', cookie(res))
+        .expect(200, 'destroyed\nhits: 2', done)
+      })
+    })
+  })
+
   describe('cookieParser()', function () {
     it('should read from req.cookies', function(done){
       var app = express()
@@ -1487,3 +1525,23 @@ function sid(res) {
   var val = match ? match[1] : undefined
   return val
 }
+
+function SyncStore() {
+  this.sessions = Object.create(null);
+}
+
+SyncStore.prototype.__proto__ = session.Store.prototype;
+
+SyncStore.prototype.destroy = function destroy(sid, callback) {
+  delete this.sessions[sid];
+  callback();
+};
+
+SyncStore.prototype.get = function get(sid, callback) {
+  callback(null, JSON.parse(this.sessions[sid]));
+};
+
+SyncStore.prototype.set = function set(sid, sess, callback) {
+  this.sessions[sid] = JSON.stringify(sess);
+  callback();
+};