From f0ba08bbf9ba2592c54a431af23e3523b18af2a6 Mon Sep 17 00:00:00 2001 From: "Anson Y. W" Date: Tue, 24 Sep 2013 19:07:04 +0800 Subject: [PATCH 1/2] add a test for req.seesion.cookie.maxAge changed to large value --- test/session.js | 65 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/test/session.js b/test/session.js index 6de1a3325..b0757cf45 100644 --- a/test/session.js +++ b/test/session.js @@ -484,29 +484,74 @@ describe('connect.session()', function(){ }) describe('.maxAge', function(){ - it('should set relative in milliseconds', function(done){ - var app = connect() - .use(connect.cookieParser()) - .use(connect.session({ secret: 'keyboard cat' })) - .use(function(req, res, next){ - req.session.cookie.maxAge = 2000; - res.end(); - }); + var id; + var app = connect() + .use(connect.cookieParser()) + .use(connect.session({ secret: 'keyboard cat', cookie: { maxAge: 2000 }})) + .use(function(req, res, next){ + req.session.count = req.session.count || 0; + req.session.count++; + if (req.session.count == 2) req.session.cookie.maxAge = 5000; + if (req.session.count == 3) req.session.cookie.maxAge = 3000000000; + res.end(req.session.count.toString()); + }); + it('should set relative in milliseconds', function(done){ app.request() .get('/') .end(function(res){ var a = new Date(expires(res)) , b = new Date; + id = sid(res); + a.getYear().should.equal(b.getYear()); a.getMonth().should.equal(b.getMonth()); a.getDate().should.equal(b.getDate()); - // TODO: check 2s + rotate a.getSeconds().should.not.equal(b.getSeconds()); + var delta = a.valueOf() - b.valueOf(); + (delta > 1000 && delta < 2000).should.be.ok; + res.body.should.equal('1'); done(); }); - }) + }); + + it('should modify cookie when changed', function(done){ + app.request() + .get('/') + .set('Cookie', 'connect.sid=' + id) + .end(function(res){ + var a = new Date(expires(res)) + , b = new Date; + + id = sid(res); + + a.getYear().should.equal(b.getYear()); + a.getMonth().should.equal(b.getMonth()); + a.getSeconds().should.not.equal(b.getSeconds()); + var delta = a.valueOf() - b.valueOf(); + (delta > 4000 && delta < 5000).should.be.ok; + res.body.should.equal('2'); + done(); + }); + }); + + it('should modify cookie when changed to large value', function(done){ + app.request() + .get('/') + .set('Cookie', 'connect.sid=' + id) + .end(function(res){ + var a = new Date(expires(res)) + , b = new Date; + + id = sid(res); + + var delta = a.valueOf() - b.valueOf(); + (delta > 2999999000 && delta < 3000000000).should.be.ok; + res.body.should.equal('3'); + done(); + }); + }); }) describe('.expires', function(){ From 06ef585967f24f7b8a69ce9fe98f5a6ec39d3e90 Mon Sep 17 00:00:00 2001 From: "Anson Y. W" Date: Wed, 21 Aug 2013 12:22:38 +0800 Subject: [PATCH 2/2] remove req.session.cookie.hasLongExpires because YAGNI It's not only not needed, but also cause problems. https://github.com/senchalabs/connect/issues/859 --- lib/middleware/session.js | 7 ++----- lib/middleware/session/cookie.js | 12 ------------ 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/lib/middleware/session.js b/lib/middleware/session.js index d918d30a0..128bc5915 100644 --- a/lib/middleware/session.js +++ b/lib/middleware/session.js @@ -258,10 +258,7 @@ function session(options){ // in case of rolling session, always reset the cookie if (!rollingSessions) { - - // long expires, handle expiry server-side - if (!isNew && cookie.hasLongExpires) return debug('already set cookie'); - + // browser-session length cookie if (null == cookie.expires) { if (!isNew) return debug('already set browser-session cookie'); @@ -269,7 +266,7 @@ function session(options){ } else if (originalHash == hash(req.session) && originalId == req.session.id) { return debug('unmodified session'); } - + } var val = 's:' + signature.sign(req.sessionID, secret); diff --git a/lib/middleware/session/cookie.js b/lib/middleware/session/cookie.js index cdce2a5e6..3125410d8 100644 --- a/lib/middleware/session/cookie.js +++ b/lib/middleware/session/cookie.js @@ -104,18 +104,6 @@ Cookie.prototype = { } }, - /** - * Check if the cookie has a reasonably large max-age. - * - * @return {Boolean} - * @api private - */ - - get hasLongExpires() { - var week = 604800000; - return this.maxAge > (4 * week); - }, - /** * Return a serialized cookie string. *