From 316e2833f017150d58392f5ee82d86438ec22d5f Mon Sep 17 00:00:00 2001
From: Ryan <ry@tinyclouds.org>
Date: Sun, 23 Aug 2009 12:20:25 +0200
Subject: [PATCH] Use flat object instead of array-of-arrays for HTTP headers.

E.G. { "Content-Length": 10, "Content-Type": "text/html" } instead of
[["Content-Length", 10], ["Content-Type", "text/html"]].
The main reason for this change is object-creation efficiency.

This still needs testing and some further changes (like when receiving
multiple header lines with the same field-name, they are concatenated with a
comma but some headers ("Content-Length") should not be concatenated ; the
new header line should replace the old value).

Various thoughts on this subject:
http://groups.google.com/group/nodejs/browse_thread/thread/9a67bb32706d9efc#
http://four.livejournal.com/979640.html
http://mail.gnome.org/archives/libsoup-list/2009-March/msg00015.html
---
 benchmark/http_simple.js                  |  6 +--
 benchmark/static_http_server.js           |  8 ++--
 src/http.js                               | 77 +++++++++++++++++++------------
 test/mjsunit/disabled/test-http-stress.js |  2 +-
 test/mjsunit/test-http-client-race.js     |  7 ++-
 test/mjsunit/test-http-client-upload.js   |  2 +-
 test/mjsunit/test-http-proxy.js           |  4 +-
 test/mjsunit/test-http-server.js          |  2 +-
 test/mjsunit/test-http.js                 | 11 ++++-
 website/api.txt                           | 32 ++++++-------
 website/index.html                        |  2 +-
 11 files changed, 87 insertions(+), 66 deletions(-)

diff --git a/benchmark/http_simple.js b/benchmark/http_simple.js
index 099a641..d1801e7 100644
--- a/benchmark/http_simple.js
+++ b/benchmark/http_simple.js
@@ -40,9 +40,9 @@ node.http.createServer(function (req, res) {
   var content_length = body.length.toString();
 
   res.sendHeader( status 
-                , [ ["Content-Type", "text/plain"]
-                  , ["Content-Length", content_length]
-                  ]
+                , { "Content-Type": "text/plain"
+                  , "Content-Length": content_length
+                  }
                 );
   res.sendBody(body);
           
diff --git a/benchmark/static_http_server.js b/benchmark/static_http_server.js
index b238e45..debdceb 100644
--- a/benchmark/static_http_server.js
+++ b/benchmark/static_http_server.js
@@ -12,10 +12,10 @@ for (var i = 0; i < bytes; i++) {
 }
 
 var server = node.http.createServer(function (req, res) {
-  res.sendHeader(200, [
-    ["Content-Type", "text/plain"],
-    ["Content-Length", body.length]
-  ]);
+  res.sendHeader(200, {
+    "Content-Type": "text/plain",
+    "Content-Length": body.length
+  });
   res.sendBody(body);
   res.finish();
 })
diff --git a/src/http.js b/src/http.js
index abf4890..f7d903a 100644
--- a/src/http.js
+++ b/src/http.js
@@ -117,7 +117,7 @@ function IncomingMessage (connection) {
 
   this.connection = connection;
   this.httpVersion = null;
-  this.headers = [];
+  this.headers = {};
 
   // request (server) only 
   this.uri = "";
@@ -142,6 +142,15 @@ IncomingMessage.prototype.resume = function () {
   this.connection.readResume();
 };
 
+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 {
+    this.headers[field] = value;
+  }
+};
 
 function OutgoingMessage () {
   node.EventEmitter.call(this);
@@ -162,22 +171,26 @@ OutgoingMessage.prototype.send = function (data, encoding) {
   this.output.push(data);
 };
 
-OutgoingMessage.prototype.sendHeaderLines = function (first_line, header_lines) {
+OutgoingMessage.prototype.sendHeaderLines = function (first_line, headers) {
   var sent_connection_header = false;
   var sent_content_length_header = false;
   var sent_transfer_encoding_header = false;
 
-  header_lines = header_lines || [];
-
   // first_line in the case of request is: "GET /index.html HTTP/1.1\r\n"
   // in the case of response it is: "HTTP/1.1 200 OK\r\n"
-  var header = first_line;
-
-  for (var i = 0; i < header_lines.length; i++) {
-    var field = header_lines[i][0];
-    var value = header_lines[i][1];
+  var message_header = first_line;
+  var field, value;
+  for (var i in headers) {
+    if (headers instanceof Array) {
+      field = headers[i][0];
+      value = headers[i][1];
+    } else {
+      if (!headers.hasOwnProperty(i)) continue;
+      field = i;
+      value = headers[i];
+    }
 
-    header += field + ": " + value + CRLF;
+    message_header += field + ": " + value + CRLF;
     
     if (connection_expression.exec(field)) {
       sent_connection_header = true;
@@ -196,23 +209,23 @@ OutgoingMessage.prototype.sendHeaderLines = function (first_line, header_lines)
   // keep-alive logic 
   if (sent_connection_header == false) {
     if (this.should_keep_alive) {
-      header += "Connection: keep-alive\r\n";
+      message_header += "Connection: keep-alive\r\n";
     } else {
       this.closeOnFinish = true;
-      header += "Connection: close\r\n";
+      message_header += "Connection: close\r\n";
     }
   }
 
   if (sent_content_length_header == false && sent_transfer_encoding_header == false) {
     if (this.use_chunked_encoding_by_default) {
-      header += "Transfer-Encoding: chunked\r\n";
+      message_header += "Transfer-Encoding: chunked\r\n";
       this.chunked_encoding = true;
     }
   }
 
-  header += CRLF;
+  message_header += CRLF;
 
-  this.send(header);
+  this.send(message_header);
   // wait until the first body chunk, or finish(), is sent to flush.
 };
 
@@ -255,7 +268,7 @@ ServerResponse.prototype.sendHeader = function (statusCode, headers) {
 };
 
 
-function ClientRequest (method, uri, header_lines) {
+function ClientRequest (method, uri, headers) {
   OutgoingMessage.call(this);
 
   this.should_keep_alive = false;
@@ -266,7 +279,7 @@ function ClientRequest (method, uri, header_lines) {
   }
   this.closeOnFinish = true;
 
-  this.sendHeaderLines(method + " " + uri + " HTTP/1.1\r\n", header_lines);
+  this.sendHeaderLines(method + " " + uri + " HTTP/1.1\r\n", headers);
 }
 node.inherits(ClientRequest, OutgoingMessage);
 
@@ -282,7 +295,7 @@ function createIncomingMessageStream (connection, incoming_listener) {
   stream.addListener("incoming", incoming_listener);
 
   var incoming;
-  var last_header_was_a_value = false;
+  var field = null, value = null;
 
   connection.addListener("message_begin", function () {
     incoming = new IncomingMessage(connection);
@@ -294,25 +307,31 @@ function createIncomingMessageStream (connection, incoming_listener) {
   });
 
   connection.addListener("header_field", function (data) {
-    if (incoming.headers.length > 0 && last_header_was_a_value == false) {
-      incoming.headers[incoming.headers.length-1][0] += data; 
+    if (value) {
+      incoming._addHeaderLine(field, value);
+      field = null;
+      value = null;
+    }
+    if (field) {
+      field += data;
     } else {
-      incoming.headers.push([data]);
+      field = data;
     }
-    last_header_was_a_value = false;
   });
 
   connection.addListener("header_value", function (data) {
-    var last_pair = incoming.headers[incoming.headers.length-1];
-    if (last_pair.length == 1) {
-      last_pair[1] = data;
-    } else  {
-      last_pair[1] += data;
-    } 
-    last_header_was_a_value = true;
+    if (value) {
+      value += data;
+    } else {
+      value = data;
+    }
   });
 
   connection.addListener("headers_complete", function (info) {
+    if (field && value) {
+      incoming._addHeaderLine(field, value);
+    }
+
     incoming.httpVersion = info.httpVersion;
 
     if (info.method) {
diff --git a/test/mjsunit/disabled/test-http-stress.js b/test/mjsunit/disabled/test-http-stress.js
index 509a1b4..7397f1d 100644
--- a/test/mjsunit/disabled/test-http-stress.js
+++ b/test/mjsunit/disabled/test-http-stress.js
@@ -5,7 +5,7 @@ var request_count = 1000;
 var response_body = '{"ok": true}';
 
 var server = node.http.createServer(function(req, res) {
- res.sendHeader(200, [['Content-Type', 'text/javascript']]);
+ res.sendHeader(200, {'Content-Type': 'text/javascript'});
  res.sendBody(response_body);
  res.finish();
 });
diff --git a/test/mjsunit/test-http-client-race.js b/test/mjsunit/test-http-client-race.js
index e13e6ca..fc3744e 100644
--- a/test/mjsunit/test-http-client-race.js
+++ b/test/mjsunit/test-http-client-race.js
@@ -6,10 +6,9 @@ var body2_s = "22222";
 
 var server = node.http.createServer(function (req, res) {
   var body = req.uri.path === "/1" ? body1_s : body2_s;
-  res.sendHeader(200, [
-    ["Content-Type", "text/plain"],
-    ["Content-Length", body.length]
-  ]);
+  res.sendHeader(200, { "Content-Type": "text/plain"
+                      , "Content-Length": body.length
+                      });
   res.sendBody(body);
   res.finish();
 });
diff --git a/test/mjsunit/test-http-client-upload.js b/test/mjsunit/test-http-client-upload.js
index e5ad377..2ea8f0a 100644
--- a/test/mjsunit/test-http-client-upload.js
+++ b/test/mjsunit/test-http-client-upload.js
@@ -17,7 +17,7 @@ var server = node.http.createServer(function(req, res) {
   req.addListener("complete", function () {
     server_req_complete = true;
     puts("request complete from server");
-    res.sendHeader(200, [['Content-Type', 'text/plain']]);
+    res.sendHeader(200, {'Content-Type': 'text/plain'});
     res.sendBody('hello\n');
     res.finish();
   });
diff --git a/test/mjsunit/test-http-proxy.js b/test/mjsunit/test-http-proxy.js
index 5be03be..2df51dd 100644
--- a/test/mjsunit/test-http-proxy.js
+++ b/test/mjsunit/test-http-proxy.js
@@ -5,7 +5,7 @@ var BACKEND_PORT = 8870;
 
 var backend = node.http.createServer(function (req, res) {
   // node.debug("backend");
-  res.sendHeader(200, [["content-type", "text/plain"]]);
+  res.sendHeader(200, {"content-type": "text/plain"});
   res.sendBody("hello world\n");
   res.finish();
 });
@@ -14,7 +14,7 @@ backend.listen(BACKEND_PORT);
 
 var proxy_client = node.http.createClient(BACKEND_PORT);
 var proxy = node.http.createServer(function (req, res) {
-  // node.debug("proxy req");
+  node.debug("proxy req headers: " + JSON.stringify(req.headers));
   var proxy_req = proxy_client.get(req.uri.path);
   proxy_req.finish(function(proxy_res) {
     res.sendHeader(proxy_res.statusCode, proxy_res.headers);
diff --git a/test/mjsunit/test-http-server.js b/test/mjsunit/test-http-server.js
index 3f84bab..ac04d5b 100644
--- a/test/mjsunit/test-http-server.js
+++ b/test/mjsunit/test-http-server.js
@@ -26,7 +26,7 @@ function onLoad() {
     }
 
     setTimeout(function () {
-      res.sendHeader(200, [["Content-Type", "text/plain"]]);
+      res.sendHeader(200, {"Content-Type": "text/plain"});
       res.sendBody(req.uri.path);
       res.finish();
     }, 1);
diff --git a/test/mjsunit/test-http.js b/test/mjsunit/test-http.js
index 94a1905..d726dd2 100644
--- a/test/mjsunit/test-http.js
+++ b/test/mjsunit/test-http.js
@@ -11,6 +11,13 @@ function onLoad () {
     if (responses_sent == 0) {
       assertEquals("GET", req.method);
       assertEquals("/hello", req.uri.path);
+
+      p(req.headers);
+      assertTrue("Accept" in req.headers);
+      assertEquals("*/*", req.headers["Accept"]);
+
+      assertTrue("Foo" in req.headers);
+      assertEquals("bar", req.headers["Foo"]);
     }
 
     if (responses_sent == 1) {
@@ -20,7 +27,7 @@ function onLoad () {
     }
 
     req.addListener("complete", function () {
-      res.sendHeader(200, [["Content-Type", "text/plain"]]);
+      res.sendHeader(200, {"Content-Type": "text/plain"});
       res.sendBody("The path was " + req.uri.path);
       res.finish();
       responses_sent += 1;
@@ -30,7 +37,7 @@ function onLoad () {
   }).listen(PORT);
 
   var client = node.http.createClient(PORT);
-  var req = client.get("/hello");
+  var req = client.get("/hello", {"Accept": "*/*", "Foo": "bar"});
   req.finish(function (res) {
     assertEquals(200, res.statusCode);
     responses_recvd += 1;
diff --git a/website/api.txt b/website/api.txt
index 266119a..b8aba19 100644
--- a/website/api.txt
+++ b/website/api.txt
@@ -18,7 +18,7 @@ World" after waiting two seconds:
 ----------------------------------------
 node.http.createServer(function (request, response) {
   setTimeout(function () {
-    response.sendHeader(200, [["Content-Type", "text/plain"]]);
+    response.sendHeader(200, {"Content-Type": "text/plain"});
     response.sendBody("Hello World");
     response.finish();
   }, 2000);
@@ -575,15 +575,14 @@ In particular, large, possibly chunk-encoded, messages. The interface is
 careful to never buffer entire requests or responses--the
 user is able to stream data.
 
-HTTP message headers are represented by an array of 2-element
-arrays like this
+HTTP message headers are represented by an object like this
 
 ----------------------------------------
-  [ ["Content-Length", "123"]
-  , ["Content-Type", "text/plain"]
-  , ["Connection", "keep-alive"]
-  , ["Accept", "*/*"]
-  ]
+  { "Content-Length": "123"
+  , "Content-Type": "text/plain"
+  , "Connection": "keep-alive"
+  , "Accept": "*/*"
+  }
 ----------------------------------------
 
 In order to support the full spectrum of possible HTTP applications, Node's
@@ -693,7 +692,6 @@ in the actual HTTP Request.
 
 
 +request.headers+ ::
-The request headers expressed as an array of 2-element arrays.
 Read only.
 
 
@@ -727,17 +725,16 @@ passed as the second parameter to the +"request"+ event.
 +response.sendHeader(statusCode, headers)+ ::
 
 Sends a response header to the request. The status code is a 3-digit HTTP
-status code, like +404+. The second argument, +headers+, should be an array
-of 2-element arrays, representing the response headers. 
+status code, like +404+. The second argument, +headers+ are the response headers. 
 +
 Example:
 +
 ----------------------------------------
 var body = "hello world";
-response.sendHeader(200, [ 
-  ["Content-Length", body.length],
-  ["Content-Type", "text/plain"]
-]);
+response.sendHeader(200, {
+  "Content-Length": body.length,
+  "Content-Type": "text/plain"
+});
 ----------------------------------------
 +
 This method must only be called once on a message and it must
@@ -799,8 +796,7 @@ connection is not established until a request is issued.
 Issues a request; if necessary establishes connection. Returns a +node.http.ClientRequest+ instance.
 +
 +request_headers+ is optional.
-+request_headers+ should be an array of 2-element
-arrays. Additional request headers might be added internally
+Additional request headers might be added internally
 by Node. Returns a +ClientRequest+ object.
 +
 Do remember to include the +Content-Length+ header if you
@@ -894,7 +890,7 @@ After emitted no other events will be emitted on the response.
   +"1.1"+ or +"1.0"+.
 
 +response.headers+ :: 
-  The response headers. An Array of 2-element arrays.
+  The response headers.
 
 +response.setBodyEncoding(encoding)+ :: 
   Set the encoding for the response body. Either +"utf8"+ or +"raw"+.
diff --git a/website/index.html b/website/index.html
index d5347d0..196a702 100644
--- a/website/index.html
+++ b/website/index.html
@@ -43,7 +43,7 @@
       <pre>
 node.http.createServer(function (req, res) {
   setTimeout(function () {
-    res.sendHeader(200, [["Content-Type", "text/plain"]]);
+    res.sendHeader(200, {"Content-Type": "text/plain"});
     res.sendBody("Hello World");
     res.finish();
   }, 2000);
-- 
2.7.4