Re-implement support for client redirects.

sdk/lib/io/http_impl.dart

Index: sdk/lib/io/http_impl.dart
diff --git a/sdk/lib/io/http_impl.dart b/sdk/lib/io/http_impl.dart
index add73898c7b8b8869724672e32d27512cb859e94..39d3c9802495eeb342973226b74f3b73c62bc7ff 100644
--- a/sdk/lib/io/http_impl.dart
+++ b/sdk/lib/io/http_impl.dart
@@ -11,8 +11,7 @@ class _HttpIncomingConnection extends StreamController<List<int>> {
   final SignalCompleter _messageCompleter = new SignalCompleter();
 
   // Common properties.
-  _HttpHeaders headers;
-  int contentLength;
+  final _HttpHeaders headers;
   bool upgraded = false;
 
   // ClientResponse properties.
@@ -23,7 +22,8 @@ class _HttpIncomingConnection extends StreamController<List<int>> {
   String method;
   Uri uri;
 
-  _HttpIncomingConnection(void this._pause(),
+  _HttpIncomingConnection(_HttpHeaders this.headers,
+                          void this._pause(),
                           void this._resume())
       : super.singleSubscription() {
     _pause();
@@ -39,7 +39,7 @@ class _HttpIncomingConnection extends StreamController<List<int>> {
     if (isPaused) {
       _pause();
     } else {
-      _reasume();
+      _resume();
     }
   }
 
@@ -116,12 +116,89 @@ class _HttpRequest extends _HttpIncoming implements HttpRequest {
 
 
 class _HttpClientResponse extends _HttpIncoming implements HttpClientResponse {
-  _HttpClientResponse(_HttpIncomingConnection _incomingConnection)
+  final List<RedirectInfo> redirects;
+
+  // The HttpClient this response belongs to.
+  final _HttpClient _httpClient;
+
+  // The HttpClientRequest of this response.
+  final _HttpClientRequest _httpRequest;
+
+  _HttpClientResponse(_HttpIncomingConnection _incomingConnection,
+                      _HttpClientRequest this._httpRequest,
+                      _HttpClient this._httpClient,
+                      List<RedirectInfo> this.redirects)

[Søren Gjesse, 2013/01/09 08:43:49]

Do you need to pass this? Isn't it just _httpRequest._responseRedirects?

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

       : super(_incomingConnection);
 
   int get statusCode => _incomingConnection.statusCode;
   String get reasonPhrase => _incomingConnection.reasonPhrase;
 
+  bool get isRedirect {
+    if (_httpRequest.method == "GET" || _httpRequest.method == "HEAD") {
+      return statusCode == HttpStatus.MOVED_PERMANENTLY ||
+             statusCode == HttpStatus.FOUND ||
+             statusCode == HttpStatus.SEE_OTHER ||
+             statusCode == HttpStatus.TEMPORARY_REDIRECT;
+    } else if (_httpRequest.method == "POST") {
+      return statusCode == HttpStatus.SEE_OTHER;
+    }
+    return false;
+  }
+
+  /**
+   * Redirect this connection to a new URL. The default value for
+   * [method] is the method for the current request. The default value
+   * for [url] is the value of the [:HttpHeaders.LOCATION:] header of
+   * the current response. All body data must have been read from the
+   * current response before calling [redirect].
+   *
+   * All headers added to the request will be added to the redirection
+   * request(s). However, any body send with the request will not be
+   * part of the redirection request(s).

[Søren Gjesse, 2013/01/09 08:43:49]

No need for this comment in the implementation file.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+   */
+  Future<HttpClientResponse> redirect([String method,
+                                       Uri url,
+                                       bool followLoops]) {
+    if (method == null) {
+      if (statusCode == HttpStatus.SEE_OTHER && _httpRequest.method == "POST") {

[Søren Gjesse, 2013/01/09 08:43:49]

Add comment with reference to RFC 2616 section 10.3.4.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+        method = "GET";
+      } else {
+        method = _httpRequest.method;
+      }
+    }
+    if (url == null) {
+      if (!isRedirect) throw new StateError("Response is not redirecting");

[Søren Gjesse, 2013/01/09 08:43:49]

"Response is not redirecting" => Response has no Location header for redirect"

[Søren Gjesse, 2013/01/09 08:43:49]

We should probably only check for the Location header and not use isRedirect
here.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+      url = new Uri.fromString(headers.value(HttpHeaders.LOCATION));
+    }
+    if (followLoops != true) {
+      for (var redirect in redirects) {
+        if (redirect.location == url) {
+          return new Future.immediateError(
+              new RedirectLoopException(redirects));
+        }
+      }
+    }
+    return _httpClient.openUrl(method, url)
+      .then((request) {
+        // Only follow redirects if initial request did.
+        request.followRedirects = _httpRequest.followRedirects;
+        // Allow one less redirect.

[Søren Gjesse, 2013/01/09 08:43:49]

The "one less" in the comment is not reflected in the code.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+        request.maxRedirects = _httpRequest.maxRedirects;
+        // Copy headers

[Søren Gjesse, 2013/01/09 08:43:49]

Terminate comment with .

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+        for (var header in _httpRequest.headers._headers.keys) {
+          if (header != HttpHeaders.HOST.toLowerCase()) {
+            request.headers.set(header, _httpRequest.headers[header]);
+          }
+        }
+        request.headers.contentLength = 0;
+        var redirects = [];

[Søren Gjesse, 2013/01/09 08:43:49]

Just operate directly on request._responseRedirects.

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+        redirects.addAll(this.redirects);
+        redirects.add(new _RedirectInfo(statusCode, method, url));
+        request._setResponseRedirects(redirects);
+        return request.close();
+      });
+  }
+
   StreamSubscription<List<int>> listen(void onData(List<int> event),
                                        {void onError(AsyncError error),
                                         void onDone(),
@@ -230,9 +307,28 @@ class _HttpResponse extends _HttpOutgoing<HttpResponse>
 
 class _HttpClientRequest extends _HttpOutgoing<HttpClientRequest>
     implements HttpClientRequest {
-  _HttpClientRequest(Uri this.uri,
+  final String method;
+  final Uri uri;
+  final List<Cookie> cookies = new List<Cookie>();
+
+  // The HttpClient this request belongs to.
+  final _HttpClient _httpClient;
+
+  final Completer<HttpClientResponse> _responseCompleter
+      = new Completer<HttpClientResponse>();
+
+
+  // TODO(ajohnsen): Get default value from client?
+  bool _followRedirects = true;
+
+  int _maxRedirects = 5;
+
+  List<RedirectInfo> _responseRedirects = [];
+
+  _HttpClientRequest(_HttpOutgoingConnection outgoing,
+                     Uri this.uri,
                      String this.method,
-                     _HttpOutgoingConnection outgoing)
+                     _HttpClient this._httpClient)
       : super("1.1", outgoing) {
     // GET and HEAD have 'content-length: 0' by default.
     if (method == "GET" || method == "HEAD") {
@@ -240,14 +336,53 @@ class _HttpClientRequest extends _HttpOutgoing<HttpClientRequest>
     }
   }
 
+  Future<HttpClientResponse> get response => _responseCompleter.future;
+
   Future<HttpClientResponse> close() {
     super.close();
     return response;
   }
 
+  int get maxRedirects => _maxRedirects;
+  void set maxRedirects(int maxRedirects) {
+    if (_headersWritten) throw new StateError("Request already sent");
+    _maxRedirects = maxRedirects;
+  }
+
+  bool get followRedirects => _followRedirects;
+  void set followRedirects(bool followRedirects) {
+    if (_headersWritten) throw new StateError("Request already sent");
+    _followRedirects = followRedirects;
+  }
+
   void _onIncoming(_HttpIncomingConnection incoming) {
-    // TODO(ajohnsen): Handle redirect and auth.
-    _responseCompleter.complete(new _HttpClientResponse(incoming));
+    // TODO(ajohnsen): Handle auth.
+    var response = new _HttpClientResponse(incoming,
+                                           this,
+                                          _httpClient,
+                                          _responseRedirects);
+
+    Future<HttpClientResponse> future;
+
+    if (followRedirects &&
+        response.isRedirect) {
+      if (response.redirects.length < maxRedirects) {
+        // Redirect
+        future = response.redirect();
+      } else {
+        // End with exception, too many redirects.
+        future = new Future.immediateError(
+            new RedirectLimitExceededException(response.redirects));
+      }
+    } else {
+      future = new Future<HttpClientResponse>.immediate(response);
+    }
+
+    future.then(
+        _responseCompleter.complete,
+        onError: (e) {
+          _responseCompleter.completeError(e.error, e.stackTrace);
+        });
   }
 
   void _onError(AsyncError error) {
@@ -299,13 +434,9 @@ class _HttpClientRequest extends _HttpOutgoing<HttpClientRequest>
     writeCRLF();
   }
 
-  Future<HttpClientResponse> get response => _responseCompleter.future;
-
-  final String method;
-  final Uri uri;
-  final List<Cookie> cookies = new List<Cookie>();
-  final Completer<HttpClientResponse> _responseCompleter
-      = new Completer<HttpClientResponse>();
+  void _setResponseRedirects(List<RedirectInfo> redirects) {

[Søren Gjesse, 2013/01/09 08:43:49]

Why do you need this method?

[Anders Johnsen, 2013/01/09 10:03:00]

Done.

+    _responseRedirects = redirects;
+  }
 }
 
 
@@ -460,7 +591,7 @@ class _HttpClient implements HttpClient {
                                  String path) {
     // TODO(sgjesse): The path set here can contain both query and
     // fragment. They should be cracked and set correctly.
-    return _open(method, new Uri.fromComponents(
+    return _openUrl(method, new Uri.fromComponents(
         scheme: "http", domain: host, port: port, path: path));
   }
 
@@ -475,7 +606,7 @@ class _HttpClient implements HttpClient {
   }
 
   Future<HttpClientRequest> getUrl(Uri url) {
-    return _open("get", url);
+    return _openUrl("get", url);
   }
 
   Future<HttpClientRequest> post(String host,
@@ -485,7 +616,7 @@ class _HttpClient implements HttpClient {
   }
 
   Future<HttpClientRequest> postUrl(Uri url) {
-    return _open("post", url);
+    return _openUrl("post", url);
   }
 
   void close() {
@@ -501,9 +632,9 @@ class _HttpClient implements HttpClient {
     _activeConnections.clear();
   }
 
-  Future<HttpClientRequest> _open(String method,
-                                  Uri uri,
-                                  [_HttpClientConnection connection]) {
+  Future<HttpClientRequest> _openUrl(String method,
+                                     Uri uri,
+                                     [_HttpClientConnection connection]) {
     if (method == null) {
       throw new ArgumentError(method);
     }
@@ -514,9 +645,9 @@ class _HttpClient implements HttpClient {
 
     // TODO(ajohnsen): Proxy?
     Future future;
+    int port = uri.port;
+    if (port == 0) port = HttpClient.DEFAULT_HTTP_PORT;
     if (connection == null) {
-      int port = uri.port;
-      if (port == 0) port = HttpClient.DEFAULT_HTTP_PORT;
       future = _getConnection(uri.domain, port);
     } else {
       future = new Future.immediate(connection);
@@ -530,8 +661,12 @@ class _HttpClient implements HttpClient {
           // Create new internal outgoing connection.
           var outgoing = new _HttpOutgoingConnection();
           // Create new request object, wrapping the outgoing connection.
-          var request = new _HttpClientRequest(
-              uri, method.toUpperCase(), outgoing);
+          var request = new _HttpClientRequest(outgoing,
+                                               uri,
+                                               method.toUpperCase(),
+                                               this);
+          request.headers.host = uri.domain;
+          request.headers.port = port;
           // Start sending the request (lazy, delayed until the user provides
           // data).
           connection.sendRequest(outgoing, onDone)