Add redirect support to the HTTP client

Add redirect support to the HTTP client

Redirection is implemented by re-using the same HttpClientConnection
object for several request/response pairs. There is s redirect method
on the HttpClientConnection object for manual redirection. The
HttpClientConnection object also has properties followRedirects and
maxRedirects to configure automatic redirect handling.

When redirection happen the HttpClientConnection will collect the
redirection history for client einspection and for redirect loop
detection.

R=ajohnsen@google.com, ager@google.com

BUG=none
TEST=tests/standalone/io/http_redirect_test.dart

Committed: https://code.google.com/p/dart/source/detail?r=7415

[Mads Ager (google), 2012-05-08 12:48:53]

lgtm

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:517: * [onError] callback will be called with an
[RedirectLimitExceeded]
an -> a?

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:531: * the current response. All body data nost have been
read from the
nost -> must

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
File tests/standalone/io/http_redirect_test.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:38:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/B");
Long lines.

[Anders Johnsen, 2012-05-08 12:58:16]

LGTM, this is awesome!

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:510: * automatically follow redirects. The default is
[:false:].
I can see wget does this by default. Why would we not?

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:525: List<RedirectInfo> get redirects();
Really cool!

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:531: * the current response. All body data nost have been
read from the
nost->most.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
File runtime/bin/http_impl.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1155: statusCode == HttpStatus.TEMPORARY_REDIRECT;
HttpStatus.MOVED_TEMPORARILY is missing here.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1191: _connection._onError(new
RedirectLoop(_connection._redirects));
Is it a problem we don't drain here?

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1197: inputStream.onData = () =>
response.inputStream.read();
remove '() => ' and '()', and below.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
File tests/standalone/io/http_redirect_test.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:38:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/B");
Long line.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:46:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/A");
Long line.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:71: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/1"));
Long line.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:93: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/1"));
Long line.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:112: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/A"));
Long line.

[Søren Gjesse, 2012-05-08 13:32:12]

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart
File runtime/bin/http.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:510: * automatically follow redirects. The default is
[:false:].
On 2012/05/08 12:58:16, ajohnsen wrote:
> I can see wget does this by default. Why would we not?

Well we can - changed the default to true.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:517: * [onError] callback will be called with an
[RedirectLimitExceeded]
On 2012/05/08 12:48:53, Mads Ager wrote:
> an -> a?

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:525: List<RedirectInfo> get redirects();
On 2012/05/08 12:58:16, ajohnsen wrote:
> Really cool!

Thanks.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:531: * the current response. All body data nost have been
read from the
On 2012/05/08 12:48:53, Mads Ager wrote:
> nost -> must

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http.dart#...
runtime/bin/http.dart:531: * the current response. All body data nost have been
read from the
On 2012/05/08 12:58:16, ajohnsen wrote:
> nost->most.

most -> must :-)

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
File runtime/bin/http_impl.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1155: statusCode == HttpStatus.TEMPORARY_REDIRECT;
On 2012/05/08 12:58:16, ajohnsen wrote:
> HttpStatus.MOVED_TEMPORARILY is missing here.

HttpStatus.MOVED_TEMPORARILY is the same status code as HttpStatus.FOUND.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1191: _connection._onError(new
RedirectLoop(_connection._redirects));
On 2012/05/08 12:58:16, ajohnsen wrote:
> Is it a problem we don't drain here?

No it shouldn't be. All error situations will close the socket.

https://chromiumcodereview.appspot.com/10386024/diff/1/runtime/bin/http_impl....
runtime/bin/http_impl.dart:1197: inputStream.onData = () =>
response.inputStream.read();
On 2012/05/08 12:58:16, ajohnsen wrote:
> remove '() => ' and '()', and below.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
File tests/standalone/io/http_redirect_test.dart (right):

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:38:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/B");
On 2012/05/08 12:48:53, Mads Ager wrote:
> Long lines.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:38:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/B");
On 2012/05/08 12:58:16, ajohnsen wrote:
> Long line.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:46:
response.headers.set(HttpHeaders.LOCATION, "http://127.0.0.1:${server.port}/A");
On 2012/05/08 12:58:16, ajohnsen wrote:
> Long line.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:71: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/1"));
On 2012/05/08 12:58:16, ajohnsen wrote:
> Long line.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:93: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/1"));
On 2012/05/08 12:58:16, ajohnsen wrote:
> Long line.

Done.

https://chromiumcodereview.appspot.com/10386024/diff/1/tests/standalone/io/ht...
tests/standalone/io/http_redirect_test.dart:112: HttpClientConnection conn =
client.getUrl(new Uri.fromString("http://127.0.0.1:${server.port}/A"));
On 2012/05/08 12:58:16, ajohnsen wrote:
> Long line.

Done.