fix(setCookie): calling several times Set-Cookie is now working

[marg51]

Hey,

not sure what is the goal of the first block of setCookie, but I got weird Set-Cookie headers when calling it several times.

---

Calling setCookie three times (foulDeviceUID, foulDeviceUID2, foulSessionUID) would result in these wrong headers

Set-Cookie: foulDeviceUID=device_x
Set-Cookie: foulDeviceUID=device_x,foulDeviceUID2=device_x
Set-Cookie: foulSessionUID=session_x
Set-Cookie: foulDeviceUID=device_x,foulDeviceUID=device_x,foulDeviceUID2=device_x,foulSessionUID=session_x

The value of foulDeviceUID is now device_x,foulDeviceUID=device_x,foulDeviceUID2=device_x,foulSessionUID=session_x

Removing the first block of setCookie yield this result:

Set-Cookie: foulDeviceUID=device_x
Set-Cookie: foulDeviceUID2=device_x
Set-Cookie: foulSessionUID=session_x

and now works as expected.

[nathschmidt]

Thanks for submitting this PR. Unfortunately, I'm not sure how your code is working. What version of node and restify are you using?

The underlying node HTTP Response class won't actually allow you to set the same header multiple times using the underlying setHeader method, the user is meant to instead use an array to set multiple copies of the same header fields. So, to get multiple Set-Cookie headers, instead of:

function respond(req, res, next){
    res.header('Set-Cookie', cookie.serialize("hello", "world"));
    res.header('Set-Cookie', cookie.serialize("goodbye", "sunshine"));

    res.send("Done.");
    next();
}

# resulting header
Set-Cookie: goodbye=sunshine;

You need to do:

function respond(req, res, next){
    res.header('Set-Cookie', [
        cookie.serialize("hello", "world"),
        cookie.serialize("goodbye", "sunshine")
    ]);

    res.send("Done.");
    next();
}

# resulting header
Set-Cookie: hello=world;
Set-Cookie: goodbye=sunshine;

So what that first block is doing is checking to see if the header has been set previously, getting that previous value, turning the header into an array if it isn't already one and then appending the new cookie to the header value array.

The docs for the underlying setHeader function are here.

This is the case in all the versions I've looked at, but I'm going to try both the current master and your master out in postman and see which one looks correct. I've also done up some unit tests (finally) for this module, so check those out too.

[nathschmidt]

I've tried out master and this PR in this gist. One thing to note, make sure you're using version 0.1.1, there where issues with setting multiple cookies in the past.

[marg51]

I'm using node 0.12.2, restify-cookies 0.1.1 and restify 3.0.0 (I should have mentioned it earlier)

There was a few changes about headers "recently", see nodejs/node-v0.x-archive#6821

I'll have a proper look later but it looks like something has changed in the API.

EDIT: I suspect the change in the above pull-request, landed in node 0.11.11 to be the problem.

[marg51]

oh, ok, found.

restify/node-restify@35df9ee

They fixed the issue in this commit, landed in restify 3.0.1, I was using 3.0.0.

Using restify 3.0.0 with your gist yields the weird behaviour I had

curl -ki http://127.0.0.1:25000/set-multi/first/second
HTTP/1.1 200 OK
Set-Cookie: thecookie=first
Set-Cookie: thecookie=first,theothercookie=second
Date: Sun, 26 Apr 2015 09:57:12 GMT
Connection: keep-alive
Transfer-Encoding: chunked

That's good that you have unit-test now though and thanks for your time

[nathschmidt]

No worries. Yeah, it was well past time to throw those together. Good to know they're fixing up the behaviour wrt to headers, I've always hated that it didn't handle multiple headers very well.