Three cookie problems

2,138 views
Skip to first unread message

Mike Scott

unread,
Apr 15, 2011, 10:45:58 AM4/15/11
to Express
Express 2.2.2 with Connect 1.3.0, Node.js v0.4.6.

First, res.clearCookie('cookieName') isn't working for me. The problem
seems to be that it doesn't set a path, and there doesn't seem to be
any way to make it set a path (except by hacking the code), so the
browser won't clear the cookie because the path doesn't match (my
browser is Safari v5.0.4 -- I guess different browsers might do this
differently).

Second, the session cookie that Express sets for itself to manage user
sessions isn't a session cookie -- it has an expires date, so the
browser stores it on disk rather than having it go away when the
browser is closed. Is there a configuration option to stop it from
putting expires or maxAge in the session cookie? I can do
req.session.cookie.expires = false, but that's somewhat inelegant, and
I might have to do it in multiple different places.

Third, and perhaps most serious (since I have workarounds for the
other two), when I have a cookie of my own that I'm setting as well as
the session cookie, it's not working properly. Tcpdump of the network
traffic reveals that the cookies are getting set twice in the
response, like this:

Set-Cookie: mycookie=<value>; path=/; expires=<date>
Set-Cookie: mycookie=<value>; path=/;
expires=<date>,connect.sid=<session ID>; path=/; expires=<date>;
httpOnly

So when I set my own cookie, the session cookie is not setting, and
sessions aren't being persisted between pages.

Any ideas?

Thanks
--
Mike Scott

TJ Holowaychuk

unread,
Apr 15, 2011, 12:33:03 PM4/15/11
to expre...@googlegroups.com
the session cookies are configurable, the docs are unfortunately quite poor but feel free to view http://senchalabs.github.com/connect/middleware-session.html

by default they are not session cookies, but you can do this if that is what you prefer, everything is configurable at the session() middleware level, as well as the request-level via req.session.cookie like you were doing

-- 
TJ Holowaychuk
--
You received this message because you are subscribed to the Google Groups "Express" group.
To post to this group, send email to expre...@googlegroups.com.
To unsubscribe from this group, send email to express-js+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/express-js?hl=en.

Mike Scott

unread,
Apr 16, 2011, 1:01:27 AM4/16/11
to Express
I've found a fix for my third problem below, which definitely looks like a bug in Express.

If I'm setting my own cookie values, then there are two different occasions in a single response where res.setHeader is called for a Set-Cookie header, once in Express's response.js (line 280) to set my cookie value, and then again in Connect's session.js (line 230) to set the session cookie value. Node doesn't like you defining the same header twice in a single response -- according to the Node docs you're supposed to put the two values in an array and then call res.setHeader once with the header name and the array.

My fix is that in response.js I don't call res.setHeader, but just save the cookie value for later processing, and then in session.js I set both cookies at once, if required. But that means changing part of Connect, so I don't know if it's a viable fix for Express.

The second problem is fixed; I just needed to add "cookie: {expires: false }" to the properties of express.session in the config.

The first problem also looks like an Express bug, but can be worked around by using:

res.cookie('<cookie name>', '', {expires: new Date(1), path: '/' });

instead of:

res.clearCookie('<cookie name>');


Mike Scott
--
Mike Scott
mi...@plokta.com

Shunsuke Watanabe

unread,
Apr 16, 2011, 11:12:50 PM4/16/11
to Express
Thanks Mike, your response is so helpful.
Why don't you make a pull request on Github?

TJ's responses tend to be superficial.
Maybe he is too busy to maintain his enormous projects.

On 4月16日, 午後2:01, Mike Scott <m...@plokta.com> wrote:
> I've found a fix for my third problem below, which definitely looks like a
> bug in Express.
>
> If I'm setting my own cookie values, then there are two different occasions
> in a single response where res.setHeader is called for a Set-Cookie header,
> once in Express's response.js (line 280) to set my cookie value, and then
> again in Connect's session.js (line 230) to set the session cookie value.
> Node doesn't like you defining the same header twice in a single response --
> according to the Node docs you're supposed to put the two values in an array
> and then call res.setHeader once with the header name and the array.
>
> My fix is that in response.js I don't call res.setHeader, but just save the
> cookie value for later processing, and then in session.js I set both cookies
> at once, if required. But that means changing part of Connect, so I don't
> know if it's a viable fix for Express.
>
> The second problem is fixed; I just needed to add "cookie: {expires: false
>
> }" to the properties of express.session in the config.
>
> The first problem also looks like an Express bug, but can be worked around
> by using:
>
> res.cookie('<cookie name>', '', {expires: new Date(1), path: '/' });
>
> instead of:
>
> res.clearCookie('<cookie name>');
>
> Mike Scott
>
> m...@plokta.com

TJ Holowaychuk

unread,
Apr 16, 2011, 11:16:08 PM4/16/11
to expre...@googlegroups.com
yeah sorry man, I don't have tons of time to respond in great length but I try to at least give hints and whatnot. If there is a bug I'm happy to help, I'll look into this tomorrow

-- 
TJ Holowaychuk

Mike Scott

unread,
Apr 17, 2011, 2:48:09 AM4/17/11
to expre...@googlegroups.com
OK, I've done some more digging in the code.

Connect actually patches the Node res.setHeader method to allow multiple cookie headers to be set at different times by turning the multiple header values into an array. But it looks as though Node has now implemented that as well, so the patch in Connect is now causing a nested array to be sent to the function, which is breaking it. So I've simply removed the patch from Connect altogether, and cookie headers now work properly. I've sent a pull request for this fix.

However, note that the patch in Connect also had a recent addition to set a charset on Content-Type headers -- I'm not sure if this is still needed, and might have to be added back.

Mike


--
You received this message because you are subscribed to the Google Groups "Express" group.
To post to this group, send email to expre...@googlegroups.com.
To unsubscribe from this group, send email to express-js+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/express-js?hl=en.




--
Mike Scott
mi...@plokta.com

Shunsuke Watanabe

unread,
Apr 17, 2011, 12:26:54 PM4/17/11
to Express
Oh, I meant no offence.
I really respect your vigorous works.

On 4月17日, 午後12:16, TJ Holowaychuk <tjholoway...@gmail.com> wrote:
> yeah sorry man, I don't have tons of time to respond in great length but I try to at least give hints and whatnot. If there is a bug I'm happy to help, I'll look into this tomorrow
>
> --

TJ Holowaychuk

unread,
Apr 17, 2011, 2:50:52 PM4/17/11
to expre...@googlegroups.com
interesting, wonder when node fixed that, I don't remember it in the changelogs, and that's pretty bad if they left that out haha, it was definitely a problem with the early 0.4.x releases. Thanks for investigating, I will take a look and merge. The charset thing is still connect-specific so I'll have to leave that in for now

-- 
TJ Holowaychuk
Reply all
Reply to author
Forward
0 new messages