Add support for multiple HTTP proxies

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 7e71ca814cf62f870d31fd308c67de0fc77ac576..06c3589a58d3cfdf9d8e59a5efd3f3c8e5eb4aeb 100644
--- a/runtime/bin/http_impl.dart
+++ b/runtime/bin/http_impl.dart
@@ -2105,29 +2105,90 @@ class _HttpClient implements HttpClient {
   }
 
   HttpClientConnection _prepareHttpClientConnection(
-    String method,
-    Uri url,
-    [_HttpClientConnection connection]) {
+      String method,
+      Uri url,
+      [_HttpClientConnection connection]) {
+
+    void _establishConnection(String host,

[Anders Johnsen, 2012/10/12 06:40:14]

Should we consider moving this outside of _prepareHttpClientConnection? This is
getting quite nested.

[Søren Gjesse, 2012/10/12 08:15:37]

I was also considering that, but then the argument list would need to include
the arguments passed to _prepareHttpClientConnection above so I decided against
it.

+                              int port,
+                              _ProxyConfiguration proxyConfiguration,
+                              int proxyIndex) {
+
+      void _connectionOpened(_SocketConnection socketConn,
+                             _HttpClientConnection connection,
+                             bool usingProxy) {
+        connection._usingProxy = usingProxy;
+        connection._connectionEstablished(socketConn);
+        HttpClientRequest request = connection.open(method, url);
+        request.headers.host = host;
+        request.headers.port = port;
+        if (connection._onRequest != null) {
+          connection._onRequest(request);
+        } else {
+          request.outputStream.close();
+        }
+      }
 
-    String host = url.domain;
-    int port = url.port == 0 ? HttpClient.DEFAULT_HTTP_PORT : url.port;
+      // Determine the actual host to connect to.
+      String connectHost;
+      int connectPort;
+      _Proxy proxy = proxyConfiguration.proxies[proxyIndex];
+      if (proxy.isDirect) {
+        connectHost = host;
+        connectPort = port;
+      } else {
+        connectHost = proxy.host;
+        connectPort = proxy.port;
+      }
 
-    void _connectionOpened(_SocketConnection socketConn,
-                           _HttpClientConnection connection,
-                           bool usingProxy) {
-      connection._usingProxy = usingProxy;
-      connection._connectionEstablished(socketConn);
-      HttpClientRequest request = connection.open(method, url);
-      request.headers.host = host;
-      request.headers.port = port;
-      if (connection._onRequest != null) {
-        connection._onRequest(request);
+      // If there are active connections for this key get the first one
+      // otherwise create a new one.
+      String key = _connectionKey(connectHost, connectPort);
+      Queue socketConnections = _openSockets[key];
+      if (socketConnections == null || socketConnections.isEmpty()) {
+        Socket socket = new Socket(connectHost, connectPort);
+        // Until the connection is established handle connection errors
+        // here as the HttpClientConnection object is not yet associated
+        // with the socket.
+        socket.onError = (e) {
+          proxyIndex++;
+          if (proxyIndex < proxyConfiguration.proxies.length) {

[Anders Johnsen, 2012/10/12 06:40:14]

An alternative would be to move this line to the start of the recursive
function. Then any caller (if others come one day), will have the correct
behavior if the index is invalid.

[Søren Gjesse, 2012/10/12 08:15:37]

The problem with that approach is that the actual connection exception is not
available at the beginning of the function. Added an assert that proxy index is
valid as this in an internal invariant.

+            // Try the next proxy in the list.
+            _establishConnection(host, port, proxyConfiguration, proxyIndex);
+          } else {
+            // Report the error through the HttpClientConnection object to
+            // the client.
+            connection._onError(e);
+          }
+        };
+        socket.onConnect = () {
+          // When the connection is established, clear the error
+          // callback as it will now be handled by the
+          // HttpClientConnection object which will be associated with
+          // the connected socket.
+          socket.onError = null;

[Anders Johnsen, 2012/10/12 06:40:14]

Not knowing much about http proxy, is the spec saying that a bad connection is
the only time we should try other? Ie. is there a HTTP error code that would
mean we should use the next proxy? If that is that case, we should add a TODO.

[Søren Gjesse, 2012/10/12 08:15:37]

There is no "real" standard here. The page
http://findproxyforurl.com/netscape-documentation/ only talks about
"unresponsive proxy" and "proxy goes down". In that respect I think we are fine,
but there might be other rules implemented in browsers that I don't know about. 

+          _SocketConnection socketConn =
+          new _SocketConnection(connectHost, connectPort, socket);
+          _activeSockets.add(socketConn);
+          _connectionOpened(socketConn, connection, !proxy.isDirect);
+        };
       } else {
-        request.outputStream.close();
+        _SocketConnection socketConn = socketConnections.removeFirst();
+        _activeSockets.add(socketConn);
+        new Timer(0, (ignored) =>
+                  _connectionOpened(socketConn, connection, !proxy.isDirect));
+
+        // Get rid of eviction timer if there are no more active connections.
+        if (socketConnections.isEmpty()) _openSockets.remove(key);
+        if (_openSockets.isEmpty()) _cancelEvictionTimer();
       }
     }
 
-    // Create a new connection if we are not re-using an existing one.
+    // Find the TCP host and port.
+    String host = url.domain;
+    int port = url.port == 0 ? HttpClient.DEFAULT_HTTP_PORT : url.port;
+
+    // Create a new connection object if we are not re-using an existing one.
     if (connection == null) {
       connection = new _HttpClientConnection(this);
     }
@@ -2141,53 +2202,8 @@ class _HttpClient implements HttpClient {
       proxyConfiguration = new _ProxyConfiguration(_findProxy(url));
     }
 
-    // Determine the actual host to connect to.
-    String connectHost;
-    int connectPort;
-    _Proxy proxy = proxyConfiguration.proxies[0];
-    if (proxy.isDirect) {
-      connectHost = host;
-      connectPort = port;
-    } else {
-      connectHost = proxy.host;
-      connectPort = proxy.port;
-    }
-
-    // If there are active connections for this key get the first one
-    // otherwise create a new one.
-    String key = _connectionKey(connectHost, connectPort);
-    Queue socketConnections = _openSockets[key];
-    if (socketConnections == null || socketConnections.isEmpty()) {
-      Socket socket = new Socket(connectHost, connectPort);
-      // Until the connection is established handle connection errors
-      // here as the HttpClientConnection object is not yet associated
-      // with the socket.
-      socket.onError = (e) {
-        // Report the error through the HttpClientConnection object to
-        // the client.
-        connection._onError(e);
-      };
-      socket.onConnect = () {
-        // When the connection is established, clear the error
-        // callback as it will now be handled by the
-        // HttpClientConnection object which will be associated with
-        // the connected socket.
-        socket.onError = null;
-        _SocketConnection socketConn =
-            new _SocketConnection(connectHost, connectPort, socket);
-        _activeSockets.add(socketConn);
-        _connectionOpened(socketConn, connection, !proxy.isDirect);
-      };
-    } else {
-      _SocketConnection socketConn = socketConnections.removeFirst();
-      _activeSockets.add(socketConn);
-      new Timer(0, (ignored) =>
-                _connectionOpened(socketConn, connection, !proxy.isDirect));
-
-      // Get rid of eviction timer if there are no more active connections.
-      if (socketConnections.isEmpty()) _openSockets.remove(key);
-      if (_openSockets.isEmpty()) _cancelEvictionTimer();
-    }
+    // Establish the connection starting with the first proxy configured.
+    _establishConnection(host, port, proxyConfiguration, 0);
 
     return connection;
   }