Start refactoring of the HTTP header handling

runtime/bin/http_impl.dart

Index: runtime/bin/http_impl.dart
diff --git a/runtime/bin/http_impl.dart b/runtime/bin/http_impl.dart
index 784ad824070f285038a447b988ce56804dd4596f..6faa9ff3c5a69249fcb176b72fa385f2ba92c794 100644
--- a/runtime/bin/http_impl.dart
+++ b/runtime/bin/http_impl.dart
@@ -2,17 +2,131 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+class _HttpHeaders implements HttpHeaders {
+  _HttpHeaders() : _headers = new Map<String, List<String>>();
+
+  List<String> operator[](String name) {
+    name = name.toLowerCase();
+    return _headers[name];
+  }
+
+  void add(String name, value) {
+    // TODO(sgjesse): Add immutable state throw HttpException is immutable.
+    if (name.toLowerCase() == "expires") {
+      if (value is Date) {
+        expires = value;
+      } else if (value is String) {
+        expires = _HttpUtils.parseDate(value);
+      } else {
+        throw new HttpException("Unexpected type for header named $name");
+      }
+    } else if (name.toLowerCase() == "host") {
+      int pos = value.indexOf(":");
+      if (pos == -1) {
+        _host = value;
+        _port = HttpClient.DEFAULT_HTTP_PORT;
+      } else {
+        _host = value.substring(0, pos);
+        if (pos + 1 == value.length) {
+          _port = HttpClient.DEFAULT_HTTP_PORT;
+        } else {
+          _port = Math.parseInt(value.substring(pos + 1));
+        }
+      }
+      _updateHostHeader();
+    } else {
+      _add(name, value.toString());
+    }
+  }
+
+  void set(String name, value) {
+    removeAll(name);
+    add(name, value);
+  }
+
+  void remove(String name, value) {
+    name = name.toLowerCase();
+    List<String> values = _headers[name];
+    if (values != null) {
+      int index = values.indexOf(value);
+      if (index != -1) {
+        values.removeRange(index, 1);
+      }
+    }
+  }
+
+  void removeAll(String name) {
+    name = name.toLowerCase();
+    _headers.remove(name);
+  }
+
+  String get host() => _host;
+  void set host(String host) {
+    _host = host;
+    _updateHostHeader();
+  }
+
+  int get port() => _port;
+  void set port(int port) {
+    _port = port;
+    _updateHostHeader();
+  }
+
+  Date get expires() {

[Anders Johnsen, 2012/04/13 11:47:53]

expires is not defined in interface. Should it? Same with host and port.

[Søren Gjesse, 2012/04/16 11:37:16]

The interface has

Date expires;
String host;
int port;

which maps to these getters and setters.

[Anders Johnsen, 2012/04/16 11:50:46]

Ahh, I see.

+    if (_expires == null) {
+      List<String> values = _headers["expires"];
+      if (values != null) {
+        _expires = _HttpUtils.parseDate(values[0]);
+      }
+    }
+    return _expires;
+  }
+
+  void set expires(Date expires) {
+    _expires = expires;
+    // Format "Expires" header with date in Greenwich Mean Time (GMT).
+    String formatted =
+        _HttpUtils.formatDate(_expires.changeTimeZone(new TimeZone.utc()));
+    _set("expires", formatted);
+  }
+
+  void _add(String name, String value) {
+    name = name.toLowerCase();
+    List<String> values = _headers[name];
+    if (values == null) {
+      values = new List<String>();
+      _headers[name] = values;
+    }
+    values.add(value);
+  }
+
+  void _set(String name, String value) {
+    name = name.toLowerCase();
+    List<String> values = new List<String>();
+    _headers[name] = values;
+    values.add(value);
+  }
+
+  _updateHostHeader() {
+    String portPart = _port == HttpClient.DEFAULT_HTTP_PORT ? "" : ":$_port";
+    _set("host", "$host$portPart");
+  }
+
+  Map<String, List<String>> _headers;
+
+  String _host;
+  int _port;
+  Date _expires;

[Anders Johnsen, 2012/04/13 11:47:53]

Can we do with keeping a String in _headers, and not have any custom fields?

[Søren Gjesse, 2012/04/16 11:37:16]

We could, but I am not sure it would be less complicated. _expires is just a
cache for the parsed date. For setting host and port one would have to parse and
split the Host header each time.

[Anders Johnsen, 2012/04/16 11:50:46]

Okay, let's keep as is then. 

+}
+
+
 class _HttpRequestResponseBase {
   _HttpRequestResponseBase(_HttpConnectionBase this._httpConnection)
       : _contentLength = -1,
-        _headers = new Map();
+        _headers = new _HttpHeaders();
 
   int get contentLength() => _contentLength;
-  Map get headers() => _headers;
-
-  void _setHeader(String name, String value) {
-    _headers[name.toLowerCase()] = value;
-  }
+  HttpHeaders get headers() => _headers;
 
   bool _write(List<int> data, bool copyBuffer) {
     bool allWritten = true;
@@ -59,13 +173,19 @@ class _HttpRequestResponseBase {
     List<int> data;
 
     // Format headers.
-    _headers.forEach((String name, String value) {
+    _headers._headers.forEach((String name, List<String> values) {
       data = name.charCodes();
       _httpConnection._write(data);
       data = ": ".charCodes();
       _httpConnection._write(data);
-      data = value.charCodes();
-      _httpConnection._write(data);
+      for (int i = 0; i < values.length; i++) {

[Anders Johnsen, 2012/04/13 11:47:53]

We could move this to the _HttpHeader class, with some private members, say
parse and stringify?

[Søren Gjesse, 2012/04/16 11:37:16]

Added a _write method which takes a _HttpConnectionBase. Also added toString for
debugging.

[Anders Johnsen, 2012/04/16 11:50:46]

Thank you!

+        if (i > 0) {
+          data = ", ".charCodes();
+          _httpConnection._write(data);
+        }
+        data = values[i].charCodes();
+        _httpConnection._write(data);
+      }
       _writeCRLF();
     });
     // Terminate header.
@@ -97,7 +217,7 @@ class _HttpRequestResponseBase {
   }
 
   _HttpConnectionBase _httpConnection;
-  Map<String, String> _headers;
+  _HttpHeaders _headers;
 
   // Length of the content body. If this is set to -1 (default value)
   // when starting to send data chunked transfer encoding will be
@@ -130,7 +250,7 @@ class _HttpRequest extends _HttpRequestResponseBase implements HttpRequest {
   }
 
   void _onHeaderReceived(String name, String value) {
-    _setHeader(name, value);
+    _headers.add(name, value);
   }
 
   void _onHeadersComplete() {
@@ -219,27 +339,6 @@ class _HttpResponse extends _HttpRequestResponseBase implements HttpResponse {
     _reasonPhrase = reasonPhrase;
   }
 
-  Date get expires() => _expires;
-  void set expires(Date expires) {
-    if (_outputStream != null) throw new HttpException("Header already sent");
-    _expires = expires;
-    // Format "Expires" header with date in Greenwich Mean Time (GMT).
-    String formatted =
-        _HttpUtils.formatDate(_expires.changeTimeZone(new TimeZone.utc()));
-    _setHeader("Expires", formatted);
-  }
-
-  // Set a header on the response. NOTE: If the same header is set
-  // more than once only the last one will be part of the response.
-  void setHeader(String name, String value) {
-    if (_outputStream != null) return new HttpException("Header already sent");
-    if (name.toLowerCase() == "expires") {
-      expires = _HttpUtils.parseDate(value);
-    } else {
-      _setHeader(name, value);
-    }
-  }
-
   OutputStream get outputStream() {
     if (_state == DONE) throw new HttpException("Response closed");
     if (_outputStream == null) {
@@ -363,16 +462,16 @@ class _HttpResponse extends _HttpRequestResponseBase implements HttpResponse {
 
     // Determine the value of the "Connection" header.
     if (_protocolVersion == "1.1" && !_persistentConnection) {
-      setHeader("Connection", "close");
+      _headers.set("Connection", "close");
     } else if (_protocolVersion == "1.0" && _persistentConnection) {
-      setHeader("Connection", "keep-alive");
+      _headers.set("Connection", "keep-alive");
     }
     // Determine the value of the "Transfer-Encoding" header based on
     // whether the content length is known.
     if (_contentLength >= 0) {
-      setHeader("Content-Length", _contentLength.toString());
+      _headers.set("Content-Length", _contentLength.toString());
     } else {
-      setHeader("Transfer-Encoding", "chunked");
+      _headers.set("Transfer-Encoding", "chunked");
     }
 
     // Write headers.
@@ -385,7 +484,6 @@ class _HttpResponse extends _HttpRequestResponseBase implements HttpResponse {
   int _statusCode;
   String _reasonPhrase;
   String _protocolVersion;
-  Date _expires;
   bool _persistentConnection;
   _HttpOutputStream _outputStream;
   int _state;
@@ -733,39 +831,6 @@ class _HttpClientRequest
 
   void set contentLength(int contentLength) => _contentLength = contentLength;
 
-  String get host() => _host;
-  void set host(String host) {
-    _host = host;
-    _updateHostHeader();
-  }
-
-  int get port() => _port;
-  void set port(int port) {
-    _port = port;
-    _updateHostHeader();
-  }
-
-  void setHeader(String name, String value) {
-    if (_state != START) throw new HttpException("Header already sent");
-    if (name.toLowerCase() == "host") {
-      int pos = value.indexOf(":");
-      if (pos == -1) {
-        _host = value;
-        _port = HttpClient.DEFAULT_HTTP_PORT;
-      } else {
-        _host = value.substring(0, pos);
-        if (pos + 1 == value.length) {
-          _port = HttpClient.DEFAULT_HTTP_PORT;
-        } else {
-          _port = Math.parseInt(value.substring(pos + 1));
-        }
-      }
-      _updateHostHeader();
-      return;
-    }
-    _setHeader(name, value);
-  }
-
   OutputStream get outputStream() {
     if (_state == DONE) throw new HttpException("Request closed");
     if (_outputStream == null) {
@@ -778,11 +843,6 @@ class _HttpClientRequest
     return _outputStream;
   }
 
-  _updateHostHeader() {
-    String portPart = _port == HttpClient.DEFAULT_HTTP_PORT ? "" : ":$_port";
-    _setHeader("Host", "$host$portPart");
-  }
-
   // Delegate functions for the HttpOutputStream implementation.
   bool _streamWrite(List<int> buffer, bool copyBuffer) {
     return _write(buffer, copyBuffer);
@@ -830,9 +890,9 @@ class _HttpClientRequest
     // Determine the value of the "Transfer-Encoding" header based on
     // whether the content length is known.
     if (_contentLength >= 0) {
-      setHeader("Content-Length", _contentLength.toString());
+      _headers.set("Content-Length", _contentLength.toString());
     } else {
-      setHeader("Transfer-Encoding", "chunked");
+      _headers.set("Transfer-Encoding", "chunked");
     }
 
     // Write headers.
@@ -842,8 +902,6 @@ class _HttpClientRequest
 
   String _method;
   String _uri;
-  String _host;
-  int _port;
   _HttpClientConnection _connection;
   _HttpOutputStream _outputStream;
   int _state;
@@ -861,14 +919,6 @@ class _HttpClientResponse
   int get statusCode() => _statusCode;
   String get reasonPhrase() => _reasonPhrase;
 
-  Date get expires() {
-    String str = _headers["expires"];
-    if (str == null) return null;
-    return _HttpUtils.parseDate(str);
-  }
-
-  Map get headers() => _headers;
-
   InputStream get inputStream() {
     if (_inputStream == null) {
       _inputStream = new _HttpInputStream(this);
@@ -886,7 +936,7 @@ class _HttpClientResponse
   }
 
   void _onHeaderReceived(String name, String value) {
-    _setHeader(name, value);
+    _headers.add(name, value);
   }
 
   void _onHeadersComplete() {
@@ -1149,8 +1199,8 @@ class _HttpClient implements HttpClient {
                            _HttpClientConnection connection) {
       connection._connectionEstablished(socketConn);
       HttpClientRequest request = connection.open(method, path);
-      request.host = host;
-      request.port = port;
+      request.headers.host = host;
+      request.headers.port = port;
       if (connection._onRequest != null) {
         connection._onRequest(request);
       } else {