cookie.expire should be modified to match cookie.maxAge

[jjqq2013]

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.

[sboehler]

Hi, I have observed the same behavior: the 'rolling' option does not work consistently. However, while the fix above does mitigate the problem, I am not sure it is correct, the root cause seems to be some kind of race condition in 'index.js'.

I have slightly modified the example @QianJin2013 kindly provided in the following gist: https://gist.github.com/sboehler/f1d1f99bda1d4ee41c50

First I issue an initial request, which generates the session:

archdev~/dev/esession% DEBUG=express-session node app.js
up and running
  express-session no SID sent, generating session +0ms
  express-session set-cookie connect.sid=s%3AUlsTGDYZl1YeC3gHOb6WaZTiMwJgYKdE.ZxMQZf6UrPPUhBWRo1AnpzV7bRIJQlGNvRWN5HqtNGE; Path=/; Expires=Tue, 29 Mar 2016 15:41:55 GMT; HttpOnly +6ms
  express-session saving UlsTGDYZl1YeC3gHOb6WaZTiMwJgYKdE +5ms

Then, after around 6 seconds, I issue another request. Note that the session should still be alive, as the maxAge has been set to 10 seconds.

  express-session fetching UlsTGDYZl1YeC3gHOb6WaZTiMwJgYKdE +6s
  express-session session found +1ms
  express-session set-cookie connect.sid=s%3AUlsTGDYZl1YeC3gHOb6WaZTiMwJgYKdE.ZxMQZf6UrPPUhBWRo1AnpzV7bRIJQlGNvRWN5HqtNGE; Path=/; Expires=Tue, 29 Mar 2016 15:41:55 GMT; HttpOnly +1ms
  express-session touching +1ms
  express-session touched +1ms

And rightly so, the session is found. Note however that the cookie is being set _before the session is touched_! This causes the cookie to be sent back to the client with the original expiry date instead of the updated one, despite 'rolling' being set in the session options. Hence, with an initial maxAge of 10 seconds, the session will time out in around 4 seconds....

  express-session no SID sent, generating session +6s
  express-session set-cookie connect.sid=s%3AmlpGFan2VXvmgQoJyJNc1QNmr9PRzk9y.%2FuSQ5wySw1sM1EiCH%2B0t5HeHxtfvuO%2FTba4uvRpfMl4; Path=/; Expires=Tue, 29 Mar 2016 15:42:07 GMT; HttpOnly +1ms
  express-session saving mlpGFan2VXvmgQoJyJNc1QNmr9PRzk9y +0ms

...which is indeed what happens in my next request 6 seconds after.

I believe there is a race condition between the code which touches the session (index.js:286) and the code which writes the cookie (index.js:204). I am able to reproduce the above more or less consistently. Not sure which factors determine the execution order.

[gabeio]

@sboehler is it possible for you to open a pr so the tests run over your version