Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Don't mangle the options object in res.cookie #1458

Merged
merged 1 commit into from

2 participants

@gmethvin

Make a copy of the cookie options before mutating it to pass to
cookie.serialize. This prevents unexpected things from happening when
we try to use the same options object multiple times.

Also add a test to verify that the options object does not change
after a request is made.

This fixes #1449. I had been doing clones to work around this in my
own code, but I much prefer this solution.

@gmethvin gmethvin Don't mangle the options object in res.cookie
Make a copy of the cookie options before mutating it to pass to
cookie.serialize. This prevents unexpected things from happening when
we try to use the same options object multiple times.

Also add a test to verify that the options object does not change
after a request is made.
39ee6f8
@tj tj merged commit d4e56c1 into from
@gmethvin gmethvin deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 3, 2013
  1. @gmethvin

    Don't mangle the options object in res.cookie

    gmethvin authored
    Make a copy of the cookie options before mutating it to pass to
    cookie.serialize. This prevents unexpected things from happening when
    we try to use the same options object multiple times.
    
    Also add a test to verify that the options object does not change
    after a request is made.
This page is out of date. Refresh to see the latest.
Showing with 25 additions and 3 deletions.
  1. +5 −3 lib/response.js
  2. +20 −0 test/res.cookie.js
View
8 lib/response.js
@@ -573,15 +573,17 @@ res.clearCookie = function(name, options){
*/
res.cookie = function(name, val, options){
- options = options || {};
+ options = utils.merge({}, options);
var secret = this.req.secret;
var signed = options.signed;
if (signed && !secret) throw new Error('connect.cookieParser("secret") required for signed cookies');
if ('object' == typeof val) val = 'j:' + JSON.stringify(val);
if (signed) val = 's:' + sign(val, secret);
- if ('maxAge' in options) options.expires = new Date(Date.now() + options.maxAge);
+ if ('maxAge' in options) {
+ options.expires = new Date(Date.now() + options.maxAge);
+ options.maxAge /= 1000;
+ }
if (null == options.path) options.path = '/';
- options.maxAge /= 1000;
this.set('Set-Cookie', cookie.serialize(name, String(val), options));
return this;
};
View
20 test/res.cookie.js
@@ -1,6 +1,7 @@
var express = require('../')
, request = require('./support/http')
+ , utils = require('connect').utils
, cookie = require('cookie');
describe('res', function(){
@@ -108,6 +109,25 @@ describe('res', function(){
done();
})
})
+
+ it('should not mutate the options object', function(done){
+ var app = express();
+
+ var options = { maxAge: 1000 };
+ var optionsCopy = utils.merge({}, options);
+
+ app.use(function(req, res){
+ res.cookie('name', 'tobi', options)
+ res.end();
+ });
+
+ request(app)
+ .get('/')
+ .end(function(err, res){
+ options.should.eql(optionsCopy);
+ done();
+ })
+ })
})
describe('signed', function(){
Something went wrong with that request. Please try again.