diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js index 650b054926..a8788c5fbd 100644 --- a/lib/_http_incoming.js +++ b/lib/_http_incoming.js @@ -154,40 +154,32 @@ IncomingMessage.prototype._addHeaderLine = function(field, value, dest) { } break; - // Comma separate. Maybe make these arrays? - case 'accept': - case 'accept-charset': - case 'accept-encoding': - case 'accept-language': - case 'connection': - case 'cookie': - case 'pragma': - case 'link': - case 'www-authenticate': - case 'proxy-authenticate': - case 'sec-websocket-extensions': - case 'sec-websocket-protocol': - if (!util.isUndefined(dest[field])) { - dest[field] += ', ' + value; - } else { + // list is taken from: + // https://mxr.mozilla.org/mozilla/source/netwerk/protocol/http/src/nsHttpHeaderArray.cpp + case 'content-type': + case 'content-length': + case 'user-agent': + case 'referer': + case 'host': + case 'authorization': + case 'proxy-authorization': + case 'if-modified-since': + case 'if-unmodified-since': + case 'from': + case 'location': + case 'max-forwards': + // drop duplicates + if (util.isUndefined(dest[field])) dest[field] = value; - } break; - default: - if (field.slice(0, 2) == 'x-') { - // except for x- - if (!util.isUndefined(dest[field])) { - dest[field] += ', ' + value; - } else { - dest[field] = value; - } - } else { - // drop duplicates - if (util.isUndefined(dest[field])) dest[field] = value; + // make comma-separated list + if (!util.isUndefined(dest[field])) + dest[field] += ', ' + value; + else { + dest[field] = value; } - break; } }; diff --git a/test/simple/test-http-server-multiheaders2.js b/test/simple/test-http-server-multiheaders2.js new file mode 100644 index 0000000000..40674c0fc2 --- /dev/null +++ b/test/simple/test-http-server-multiheaders2.js @@ -0,0 +1,108 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +// Verify that the HTTP server implementation handles multiple instances +// of the same header as per RFC2616: joining the handful of fields by ', ' +// that support it, and dropping duplicates for other fields. + +var common = require('../common'); +var assert = require('assert'); +var http = require('http'); + +var multipleAllowed = [ + 'Accept', + 'Accept-Charset', + 'Accept-Encoding', + 'Accept-Language', + 'Connection', + 'Cookie', + 'DAV', // GH-2750 + 'Pragma', // GH-715 + 'Link', // GH-1187 + 'WWW-Authenticate', // GH-1083 + 'Proxy-Authenticate', // GH-4052 + 'Sec-Websocket-Extensions', // GH-2764 + 'Sec-Websocket-Protocol', // GH-2764 + 'Via', // GH-6660 + + // not a special case, just making sure it's parsed correctly + 'X-Forwarded-For', + + // make sure that unspecified headers is treated as multiple + 'Some-Random-Header', + 'X-Some-Random-Header', +]; + +var multipleForbidden = [ + 'Content-Type', + 'User-Agent', + 'Referer', + 'Host', + 'Authorization', + 'Proxy-Authorization', + 'If-Modified-Since', + 'If-Unmodified-Since', + 'From', + 'Location', + 'Max-Forwards', + + // special case, tested differently + //'Content-Length', +]; + +var srv = http.createServer(function(req, res) { + multipleForbidden.forEach(function(header) { + assert.equal(req.headers[header.toLowerCase()], 'foo', 'header parsed incorrectly: ' + header); + }); + multipleAllowed.forEach(function(header) { + assert.equal(req.headers[header.toLowerCase()], 'foo, bar', 'header parsed incorrectly: ' + header); + }); + assert.equal(req.headers['content-length'], 0); + + res.writeHead(200, {'Content-Type' : 'text/plain'}); + res.end('EOF'); + + srv.close(); +}); + +function makeHeader(value) { + return function(header) { + return [header, value]; + } +} + +var headers = [] + .concat(multipleAllowed.map(makeHeader('foo'))) + .concat(multipleForbidden.map(makeHeader('foo'))) + .concat(multipleAllowed.map(makeHeader('bar'))) + .concat(multipleForbidden.map(makeHeader('bar'))) + // content-length is a special case since node.js + // is dropping connetions with non-numeric headers + .concat([['content-length', 0], ['content-length', 123]]); + +srv.listen(common.PORT, function() { + http.get({ + host: 'localhost', + port: common.PORT, + path: '/', + headers: headers, + }); +});