Fix bug: cookie.expires not modified even if maxAge set

[jjqq2013]

ref: #276.

Hi, i found when access server at second time, cookie.expire will not be modified even if i set maxAge and rolling:true.

Here is my test code.

var express = require('express');
var path = require('path');
var cookieParser = require('cookie-parser');
var bodyParser = require('body-parser');
var session = require('express-session');

var app = express();

// Use the session middleware
app.use(session({
  secret: 'keyboard cat',
  rolling: true,
  saveUninitialized: true,
  resave: true,
  cookie: { maxAge: 10000 }}))

// Access the session as req.session
app.get('/', function(req, res, next) {
  var sess = req.session
  if (sess.views) {
    sess.views++
    res.setHeader('Content-Type', 'text/html')
    res.write('<p>views: ' + sess.views + '</p>')
    res.write('<p>expires in: ' + (sess.cookie.maxAge / 1000) + 's</p>')
    res.end()
  } else {
    sess.views = 1
    res.end('welcome to the session demo. refresh!')
  }
});

In above example, i set maxAge to 10 seconds, and rolling:true, and saveUninitialized:true. Other settings seems have no effect to this problem.

I run this code, then use following curl command line to test and see verbose output:

$ curl --verbose --cookie cookie --cookie-jar cookie http://localhost:3000
* Rebuilt URL to: http://localhost:3000/
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 200 OK
< X-Powered-By: Express
* Replaced cookie connect.sid="s%3AGRecnYYVv0WP0plY8qrIvV4EHG82Ts8h.aGG7rL0Fdb4fiXS0zN3u5eC9h%2BY8adZgZxzpyFZWFNs" for domain localhost, path /, expire 1455991539
< set-cookie: connect.sid=s%3AGRecnYYVv0WP0plY8qrIvV4EHG82Ts8h.aGG7rL0Fdb4fiXS0zN3u5eC9h%2BY8adZgZxzpyFZWFNs; Path=/; Expires=Sat, 20 Feb 2016 18:05:39 GMT; HttpOnly
< Date: Sat, 20 Feb 2016 18:05:29 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Connection #0 to host localhost left intact
welcome to the session demo. refresh!

After about 5 seconds, i run the command line again, it will use same cookie from last response.

$ curl --verbose --cookie cookie --cookie-jar cookie http://localhost:3000
* Rebuilt URL to: http://localhost:3000/
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.43.0
> Accept: */*
> Cookie: connect.sid=s%3AGRecnYYVv0WP0plY8qrIvV4EHG82Ts8h.aGG7rL0Fdb4fiXS0zN3u5eC9h%2BY8adZgZxzpyFZWFNs
> 
< HTTP/1.1 200 OK
< X-Powered-By: Express
< Content-Type: text/html
* Replaced cookie connect.sid="s%3AGRecnYYVv0WP0plY8qrIvV4EHG82Ts8h.aGG7rL0Fdb4fiXS0zN3u5eC9h%2BY8adZgZxzpyFZWFNs" for domain localhost, path /, expire 1455991539
< set-cookie: connect.sid=s%3AGRecnYYVv0WP0plY8qrIvV4EHG82Ts8h.aGG7rL0Fdb4fiXS0zN3u5eC9h%2BY8adZgZxzpyFZWFNs; Path=/; Expires=Sat, 20 Feb 2016 18:05:39 GMT; HttpOnly
< Date: Sat, 20 Feb 2016 18:05:35 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Connection #0 to host localhost left intact
<p>views: 2</p><p>expires in: 4.065s</p>

Please notice the expire of second response, i'v set maxAge to 10 seconds, but cookie's expire will be:

        Expires=Sat, 20 Feb 2016 18:05:39 GMT
Current time is: Sat, 20 Feb 2016 18:05:35 GMT

It means expire after 4 seconds, this is not by-design obviously.

I checked source code in index.js, line 206:

    onHeaders(res, function(){
      ... ...
      var cookie = req.session.cookie;
      ... ...
//line 206:
      setcookie(res, name, req.sessionID, secrets[0], cookie.data);

*I insert following code to line 206 to change cookie.expire* then works!

      cookie.originalMaxAge > 0 && (cookie.expires = new Date(Date.now() + cookie.originalMaxAge));

I hope you can check this and merge it.

[dougwilson]

Hi! Please update your change to use an if statement. Also, please add a test to our test suite that fails without your fix and passes with your fix.

[jjqq2013]

Hi i have added test case for this problem.
Finally i found the problem occurs only when using res.write(...), res.end() combo.

res.write(...)
res.end()

My test case:

  describe('cookie.expire', function(){
    this.timeout(86400000);
    var val;

    var app = express()
        .use(session({ secret: 'keyboard cat', cookie: { maxAge: 10000 }}))  //timeout:10 seconds
        .use(function(req, res, next){
          req.session.count = req.session.count || 0;
          req.session.count++;
          res.write(req.session.count.toString());  //this is very important. If use res.end(...) then no bug
          res.end('');
        });

    it('should be Now + .maxAge  (first test)', function(done){
      request(app)
          .get('/')
          .expect(200, '1', function (err, res) {
            var a = new Date(expires(res))
            var b = new Date
            var delta = a.valueOf() - b.valueOf()

            val = cookie(res).split(';')[0];

            assert.ok(delta > 9000 && delta <= 10000, "cookie.expire is too short as expected");
            done();
          });
    });

    it('should be Now + .maxAge (second test)', function(done){
      setTimeout(function() { //do this after 6 seconds, still not timeout
        request(app)
            .get('/')
            .set('Cookie', val)
            .expect(200, '2', function (err, res) {
              var a = new Date(expires(res))
              var b = new Date
              var delta = a.valueOf() - b.valueOf()

              var thisCookie = cookie(res).split(';')[0];
              assert.equal(thisCookie, val, "cookie value itself should be same");

              assert.ok(delta > 9000 && delta <= 10000, "cookie.expire is too short as expected");
              done();
            });
      }, 6000);
    });
  });

My test result:
Before modification:

$ npm test
... ...
  session()
    cookie.expire
      ✓ should be Now + .maxAge  (first test) (45ms)
      1) should be Now + .maxAge (second test)

  1 passing (6s)
  1 failing

  1) session() cookie.expire should be Now + .maxAge (second test):

      Uncaught AssertionError: cookie.expire is too short as expected
      + expected - actual

      -false
      +true

      at Test.<anonymous> (test/session.js:59:22)
      at Test.assert (node_modules/supertest/lib/test.js:156:6)
      at Server.assert (node_modules/supertest/lib/test.js:127:12)
      at emitCloseNT (net.js:1514:8)
... ...

After modification:

$ npm test
... ...
  session()
    cookie.expire
      ✓ should be Now + .maxAge  (first test) (58ms)
      ✓ should be Now + .maxAge (second test) (6017ms)
  2 passing (6s)

I don't dig too deep for the problem, so maybe my modification is not the best method.
Hope this help you.