From 51bd1b4483f60d85ee19274aab98505a0b5c9105 Mon Sep 17 00:00:00 2001
From: Peter Griess <pg@std.in>
Date: Mon, 21 Jun 2010 13:53:17 -0500
Subject: [PATCH] Only concatenate some incoming HTTP headers.

- Concatenate 'accept', 'accept-charset', 'accept-encoding',
  'accept-language', 'connection', 'cookie', and 'x-*' headers.
- For all others, drop duplicates.
---
 lib/http.js                                  | 32 ++++++++++++++---
 test/simple/test-http-server-multiheaders.js | 36 ++++++++++++++++++++
 2 files changed, 63 insertions(+), 5 deletions(-)
 create mode 100644 test/simple/test-http-server-multiheaders.js

diff --git a/lib/http.js b/lib/http.js
index 5f051ed96b8..e3ebf75a8b3 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -227,13 +227,35 @@ IncomingMessage.prototype.resume = function () {
   this.socket.resume();
 };
 
+// Add the given (field, value) pair to the message
+//
+// Per RFC2616, section 4.2 it is acceptable to join multiple instances of the
+// same header with a ', ' if the header in question supports specification of
+// multiple values this way. If not, we declare the first instance the winner
+// and drop the second. Extended header fields (those beginning with 'x-') are
+// always joined.
 IncomingMessage.prototype._addHeaderLine = function (field, value) {
-  if (field in this.headers) {
-    // TODO Certain headers like 'Content-Type' should not be concatinated.
-    // See https://www.google.com/reader/view/?tab=my#overview-page
-    this.headers[field] += ", " + value;
-  } else {
+  if (!(field in this.headers)) {
     this.headers[field] = value;
+    return;
+  }
+
+  // If this field already exists in the request, use duplicate-resolution
+  // logic from RFC2616.
+  switch (field) {
+  case 'accept':
+  case 'accept-charset':
+  case 'accept-encoding':
+  case 'accept-language':
+  case 'connection':
+  case 'cookie':
+    this.headers[field] += ', ' + value;
+    break;
+
+  default:
+    if (field[0] !== 'x' || field[1] !== '-') break;
+    this.headers[field] += ', ' + value;
+    break;
   }
 };
 
diff --git a/test/simple/test-http-server-multiheaders.js b/test/simple/test-http-server-multiheaders.js
new file mode 100644
index 00000000000..3133c67f897
--- /dev/null
+++ b/test/simple/test-http-server-multiheaders.js
@@ -0,0 +1,36 @@
+// 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.
+
+require('../common');
+var http = require('http');
+
+var srv = http.createServer(function(req, res) {
+  assert.equal(req.headers.accept, 'abc, def, ghijklmnopqrst');
+  assert.equal(req.headers.host, 'foo');
+  assert.equal(req.headers['x-foo'], 'bingo');
+  assert.equal(req.headers['x-bar'], 'banjo, bango');
+
+  res.writeHead(200, {'Content-Type' : 'text/plain'});
+  res.end('EOF');
+
+  srv.close();
+});
+
+srv.listen(PORT, function () {
+  var hc = http.createClient(PORT, 'localhost');
+  var hr = hc.request('/',
+    [
+      ['accept', 'abc'],
+      ['accept', 'def'],
+      ['Accept', 'ghijklmnopqrst'],
+      ['host', 'foo'],
+      ['Host', 'bar'],
+      ['hOst', 'baz'],
+      ['x-foo', 'bingo'],
+      ['x-bar', 'banjo'],
+      ['x-bar', 'bango']
+    ]
+  );
+  hr.end();
+});