Permalink
Browse files

Fixed: set-cookie only once for browser-session cookies

  • Loading branch information...
1 parent 6b5233f commit c0cb44cd2dc7885605e6a86c98ead9baa3904ed2 @tj tj committed Feb 19, 2012
Showing with 43 additions and 14 deletions.
  1. +11 −6 lib/middleware/session.js
  2. +32 −8 test/session.js
@@ -212,7 +212,8 @@ function session(options){
// parse url
var url = parse(req.url)
- , path = url.pathname;
+ , path = url.pathname
+ , sessionIsNew;
// expose store
req.sessionStore = store;
@@ -225,11 +226,14 @@ function session(options){
, tls = req.connection.encrypted || (trustProxy && 'https' == proto)
, secured = cookie.secure && tls;
- debug('secured: %s', !!secured);
- if (secured || !cookie.secure) {
- debug('set %s to %s', key, req.sessionID);
- res.setHeader('Set-Cookie', cookie.serialize(key, req.sessionID));
- }
+ // browser-session cookies only set-cookie once
+ if (null == cookie.expires && !sessionIsNew) return;
+
+ // only send secure cookies via https
+ if (cookie.secure && !secured) return debug('not secured');
+
+ debug('set %s to %s', key, req.sessionID);
+ res.setHeader('Set-Cookie', cookie.serialize(key, req.sessionID));
});
// proxy end() to commit the session
@@ -247,6 +251,7 @@ function session(options){
// generate the session
function generate() {
+ sessionIsNew = true;
store.generate(req);
}
View
@@ -2,6 +2,8 @@
var connect = require('../')
, assert = require('assert');
+var min = 60 * 1000;
+
function respond(req, res) {
res.end();
}
@@ -16,7 +18,7 @@ function expires(res) {
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session())
+ .use(connect.session({ cookie: { maxAge: min }}))
.use(respond);
describe('connect.session()', function(){
@@ -31,7 +33,7 @@ describe('connect.session()', function(){
it('should trust X-Forwarded-Proto', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session({ proxy: true, cookie: { secure: true }}))
+ .use(connect.session({ proxy: true, cookie: { secure: true, maxAge: 5 }}))
.use(respond);
app.request()
@@ -48,7 +50,7 @@ describe('connect.session()', function(){
it('should not trust X-Forwarded-Proto', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session({ cookie: { secure: true }}))
+ .use(connect.session({ cookie: { secure: true, maxAge: min }}))
.use(respond);
app.request()
@@ -76,7 +78,7 @@ describe('connect.session()', function(){
it('should allow overriding', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session({ key: 'sid' }))
+ .use(connect.session({ key: 'sid', cookie: { maxAge: min }}))
.use(respond);
app.request()
@@ -143,7 +145,7 @@ describe('connect.session()', function(){
it('should persist', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session())
+ .use(connect.session({ cookie: { maxAge: min }}))
.use(function(req, res, next){
req.session.count = req.session.count || 0;
req.session.count++;
@@ -192,7 +194,7 @@ describe('connect.session()', function(){
it('should destroy/replace the previous session', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session())
+ .use(connect.session({ cookie: { maxAge: min }}))
.use(function(req, res, next){
var id = req.session.id;
req.session.regenerate(function(err){
@@ -223,7 +225,7 @@ describe('connect.session()', function(){
it('should serialize as parameters', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))
- .use(connect.session({ proxy: true }))
+ .use(connect.session({ proxy: true, cookie: { maxAge: min }}))
.use(function(req, res, next){
req.session.cookie.httpOnly = false;
req.session.cookie.secure = true;
@@ -245,7 +247,6 @@ describe('connect.session()', function(){
.use(connect.cookieParser('keyboard cat'))
.use(connect.session({ cookie: { path: '/admin' }}))
.use(function(req, res, next){
- req.session.cookie.secure = false;
res.end();
});
@@ -258,6 +259,29 @@ describe('connect.session()', function(){
});
})
+ it('should Set-Cookie only once for browser-session cookies', function(done){
+ var app = connect()
+ .use(connect.cookieParser('keyboard cat'))
+ .use(connect.session({ cookie: { path: '/admin' }}))
+ .use(function(req, res, next){
+ res.end();
+ });
+
+ app.request()
+ .get('/')
+ .end(function(res){
+ res.headers.should.have.property('set-cookie');
+
+ app.request()
+ .get('/')
+ .set('Cookie', 'connect.sid=' + sid(res))
+ .end(function(res){
+ res.headers.should.not.have.property('set-cookie');
+ done();
+ })
+ });
+ })
+
it('should override defaults', function(done){
var app = connect()
.use(connect.cookieParser('keyboard cat'))

0 comments on commit c0cb44c

Please sign in to comment.