fix: Allow multiple unmerged set-cookie headers.

[lrowe]

Pre-Submission Checklist

• Opened an issue discussing these changes before opening the PR
• Ran the linter and tests via make prepush
• Included comprehensive and convincing tests for changes

Issues

Closes:

• Issue set-cookie headers should not include comma separated values #779 - set-cookie headers should not include comma separated values

Changes

Fixes the fix for #779 to allow multiple set-cookie headers. The RFC only disallows these headers from being merged into a single header, it does not disallow multiple headers.

[DonutEspresso]

Thanks for the PR! Oddly, this is the exact opposite of #779, but appears to have the desired behavior. Wonder if something in Node core changed where it knows about how to set multiple headers for certain headers (like cookies), rather than universally transforming multiple values into a comma delimited string.

Maybe @yunong can remember how the original issue was manifesting?

[lrowe]

It looks like @yunong was wanting to avoid set-cookie headers being merged into a single header (as this is not valid per RFC.) Maybe this is something Restify did at one point? It looks like Node has supported Arrays for multiple header lines back to Sep 2010 nodejs/node-v0.x-archive@6560ab9 yet I see recent bugs like nodejs/node#3591 reporting that header merging is happening.

I've run this test script back to v0.10.48 and see an appropriate response with two separate Set-Cookie headers.

"use strict";

var http = require('http');
var port = 3000;

function requestHandler(request, response) {
  console.log(request.url);
  response.setHeader("Set-Cookie", ["a=1", "b=2"]);
  response.end('Hello Node.js Server!');
}

var server = http.createServer(requestHandler);

server.listen(port, function (err) {
  if (err) {
    return console.log('something bad happened', err);
  }

  console.log('server is listening on ' + port);
})

[retrohacker]

@lrowe has extensively tested this across many versions of Node and it gives the desired behaviour. We have a test case in place to catch a regression from Node core if one happens.

If there are cases where Node core collapses Set-Cookie values into a comma separated list we can add a separate test for that in the future.

[DonutEspresso]

LGTM