Revised fix for nested array bug in Set-Cookie headers

[drplokta]

I think this one works, by not changing the Set-Cookie header if it's already an array.

[tj]

Well no, now you simply cannot add to it

[tj]

Can't just guess :p. What are the steps to reproduce the bug again?

[drplokta]

Here's the simplest example I can come up with (you'll need an index.jade and layout.jade in the views directory as well, but it doesn't really matter what they are). Run this and load http://:3000/ and then check your cookies. You'll have test=test and test=test,connect.sid=...., and no connect.sid cookie.

var express = require('express');
var mongoStore = require('connect-mongodb');
var jade = require('jade');
app = module.exports = express.createServer();

app.configure(function () {
  app.set('views', __dirname + '/views');
  app.set('view engine', 'jade');
  app.use(express.bodyParser());
  app.use(express.methodOverride());
  app.use(express.static(__dirname + '/public'));
  app.use(express.cookieParser());
  app.use(express.session({ store: mongoStore({url: app.set('connstring')}), 
                            secret: 'secret',
                            cookie: {expires: false }}));
  app.use(app.router);
  app.use(express.logger({ format: '\x1b[1m:method\x1b[0m \x1b[33m:url\x1b[0m :response-time ms' }))
});

app.get('/', function (req, res) {
  res.cookie('test', 'test');
  res.render('index', { title: 'Cookie Test' })
});

if (!module.parent) {
  app.listen(3000);
  console.log("Express server listening on port %d", app.address().port);
}

[tj]

looks fine to me, I get both as expected

[tj]

ps logger() should be at the top above bodyParser() so that it can enclose itself over the entire response to tell you what the response time was

[tj]

ps logger() should be at the top above bodyParser() so that it can enclose itself over the entire response to tell you what the response time was

[donpark]

I just ran into this when I'm setting two cookies in a single response using express's res.cookie function. It looks like res.setHeader gets patched twice somehow so that calling setHeader calls the patch twice before calling the original.

[donpark]

Re 'patched twice' observation, it might be due to a node-inspector flaw because log traces don't show the same.

What I do see in outgoing response header is:

uc=4dcb1043b87f12c9eb000005%3A123%3A1305228680081%3AH%2BADQZmGGER4KKUZdf7xCFydaYY%3D; path=/; expires=Fri, 11 May 2012 19:31:20 GMT; httpOnly; secure
uc=4dcb1043b87f12c9eb000005%3A123%3A1305228680081%3AH%2BADQZmGGER4KKUZdf7xCFydaYY%3D; path=/; expires=Fri, 11 May 2012 19:31:20 GMT; httpOnly; secure,
sc=IOi0g1qazTmYzMnaIjcDREWV.N8VoMqiP1XTYZNJPtYZ8jhB%2FBul2WT64WwMPnvE7fw0; path=/; expires=Thu, 12 May 2011 20:31:20 GMT; httpOnly; secure

where uc is the cookie I've explicitly set with maxAge of 1 yr and sc is the session cookie.

Originally, one of the two uc values had different expires value when I used expires instead of maxAge. Pretty weird.

[tj]

@donpark can you gist the snippet causing the issue?

[donpark]

Finally cornered the bastard. It happens when connect-mongodb module is used which I just noticed @drplokta is also using. So it seems to be a conflict between the patch and whatever connect-mongodb is doing. dang.

[donpark]

Oops. It turned out not to be connect-mongodb's fault but the way NPM 1.0 works.

If connect-mongodb module is installed before connect, then connect-mongodb will install connect in its own node_modules folder. So if connect is installed afterward, then you end up with two copies of connect which means you got double patchs.

Installing connect before connect-mongodb fixed the problem.

[tj]

ah :( fuck, yeah, it kinda sucks that npm doesn't give you the same exports when it's possible for the single module to satisfy the dependency. It cases lots of gotchas, I've seen tons already but yeah that's a funky one. For "plugin-like" modules I typically leave out the dependency on say connect or express and just assume they will be present

[donpark]

well, I think NPM should play nice and remove extra copies of same module if origin matches.

speaking of origin, another NPM gotcha is that it caches even local version of modules and refuses to install fresh copy off network. so I have to remember to clean NPM cache after playing with a local version of modules.

[tj]

I had this issue with Stylus it rendered instanceof completely useless so I had to remove every instanceof in the project haha