Version 2.16.0-134.6.beta
* Cherry-pick 57db739be0ad4629079bfa94840064f615d35abc to beta
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0e43ad5..b4c038a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,11 @@
in 2018, as it doesn't work with any Dart 2.x release.
- Add optional `sourcePort` parameter to `Socket.connect`, `Socket.startConnect`, `RawSocket.connect` and `RawSocket.startConnect`
+- **Breaking Change** [#45410](https://github.com/dart-lang/sdk/issues/45410):
+ `HttpClient` no longer transmits some headers (i.e. `authorization`,
+ `www-authenticate`, `cookie`, `cookie2`) when processing redirects to
+ a different domain.
+
#### `dart:isolate`
- **Breaking Change** [#47769](https://github.com/dart-lang/sdk/issues/47769):
diff --git a/sdk/lib/_http/http.dart b/sdk/lib/_http/http.dart
index b63102a..fdb4de0 100644
--- a/sdk/lib/_http/http.dart
+++ b/sdk/lib/_http/http.dart
@@ -1715,8 +1715,39 @@
/// following the 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).
+ /// request(s) except when forwarding sensitive headers like
+ /// "Authorization", "WWW-Authenticate", and "Cookie". Those headers
+ /// will be skipped if following a redirect to a domain that is not a
+ /// subdomain match or exact match of the initial domain.
+ /// For example, a redirect from "foo.com" to either "foo.com" or
+ /// "sub.foo.com" will forward the sensitive headers, but a redirect to
+ /// "bar.com" will not.
+ ///
+ /// Any body send with the request will not be part of the redirection
+ /// request(s).
+ ///
+ /// For precise control of redirect handling, set this property to `false`
+ /// and make a separate HTTP request to process the redirect. For example:
+ ///
+ /// ```dart
+ /// final client = HttpClient();
+ /// var uri = Uri.parse("http://localhost/");
+ /// var request = await client.getUrl(uri);
+ /// request.followRedirects = false;
+ /// var response = await request.close();
+ /// while (response.isRedirect) {
+ /// response.drain();
+ /// final location = response.headers.value(HttpHeaders.locationHeader);
+ /// if (location != null) {
+ /// uri = uri.resolve(location);
+ /// request = await client.getUrl(uri);
+ /// // Set the body or headers as desired.
+ /// request.followRedirects = false;
+ /// response = await request.close();
+ /// }
+ /// }
+ /// // Do something with the final response.
+ /// ```
bool followRedirects = true;
/// Set this property to the maximum number of redirects to follow
diff --git a/sdk/lib/_http/http_impl.dart b/sdk/lib/_http/http_impl.dart
index 6c9c786..622c662 100644
--- a/sdk/lib/_http/http_impl.dart
+++ b/sdk/lib/_http/http_impl.dart
@@ -667,7 +667,7 @@
}
}
return _httpClient
- ._openUrlFromRequest(method, url, _httpRequest)
+ ._openUrlFromRequest(method, url, _httpRequest, isRedirect: true)
.then((request) {
request._responseRedirects
..addAll(redirects)
@@ -751,7 +751,8 @@
return drain().then((_) {
return _httpClient
._openUrlFromRequest(
- _httpRequest.method, _httpRequest.uri, _httpRequest)
+ _httpRequest.method, _httpRequest.uri, _httpRequest,
+ isRedirect: false)
.then((request) => request.close());
});
}
@@ -2715,8 +2716,31 @@
});
}
+ static bool _isSubdomain(Uri subdomain, Uri domain) {
+ return (subdomain.scheme == domain.scheme &&
+ subdomain.port == domain.port &&
+ (subdomain.host == domain.host ||
+ subdomain.host.endsWith("." + domain.host)));
+ }
+
+ static bool _shouldCopyHeaderOnRedirect(
+ String headerKey, Uri originalUrl, Uri redirectUri) {
+ if (_isSubdomain(redirectUri, originalUrl)) {
+ return true;
+ }
+
+ const nonRedirectHeaders = [
+ "authorization",
+ "www-authenticate",
+ "cookie",
+ "cookie2"
+ ];
+ return !nonRedirectHeaders.contains(headerKey.toLowerCase());
+ }
+
Future<_HttpClientRequest> _openUrlFromRequest(
- String method, Uri uri, _HttpClientRequest previous) {
+ String method, Uri uri, _HttpClientRequest previous,
+ {required bool isRedirect}) {
// If the new URI is relative (to either '/' or some sub-path),
// construct a full URI from the previous one.
Uri resolved = previous.uri.resolveUri(uri);
@@ -2728,7 +2752,9 @@
..maxRedirects = previous.maxRedirects;
// Copy headers.
for (var header in previous.headers._headers.keys) {
- if (request.headers[header] == null) {
+ if (request.headers[header] == null &&
+ (!isRedirect ||
+ _shouldCopyHeaderOnRedirect(header, resolved, previous.uri))) {
request.headers.set(header, previous.headers[header]!);
}
}
diff --git a/tests/standalone/io/http_redirect_test.dart b/tests/standalone/io/http_redirect_test.dart
index f68e32f..1a19222 100644
--- a/tests/standalone/io/http_redirect_test.dart
+++ b/tests/standalone/io/http_redirect_test.dart
@@ -7,7 +7,7 @@
import "dart:async";
import "dart:io";
-Future<HttpServer> setupServer() {
+Future<HttpServer> setupServer({Uri? targetServer}) {
final completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
@@ -128,6 +128,8 @@
// Setup redirect checking headers.
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
response.headers.set(
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
response.statusCode = HttpStatus.movedPermanently;
@@ -135,9 +137,24 @@
});
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
response.close();
});
+ if (targetServer != null) {
+ addRequestHandler("/src-crossdomain",
+ (HttpRequest request, HttpResponse response) {
+ Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
+ response.headers
+ .set(HttpHeaders.locationHeader, targetServer.toString());
+ response.statusCode = HttpStatus.movedPermanently;
+ response.close();
+ });
+ }
+
// Setup redirect for 301 where POST should not redirect.
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
Expect.equals("POST", request.method);
@@ -183,6 +200,36 @@
return completer.future;
}
+// A second HTTP server used to validate that redirect requests accross domains
+// do *not* include security-related headers.
+Future<HttpServer> setupTargetServer() {
+ final completer = new Completer<HttpServer>();
+ HttpServer.bind("127.0.0.1", 0).then((server) {
+ var handlers = new Map<String, Function>();
+ addRequestHandler(
+ String path, void handler(HttpRequest request, HttpResponse response)) {
+ handlers[path] = handler;
+ }
+
+ server.listen((HttpRequest request) {
+ if (request.uri.path == "/target") {
+ Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be removed on redirect");
+ request.response.close();
+ } else {
+ request.listen((_) {}, onDone: () {
+ request.response.statusCode = 404;
+ request.response.close();
+ });
+ }
+ });
+
+ completer.complete(server);
+ });
+ return completer.future;
+}
+
void checkRedirects(int redirectCount, HttpClientResponse response) {
if (redirectCount < 2) {
Expect.isTrue(response.redirects.isEmpty);
@@ -250,6 +297,7 @@
.then((HttpClientRequest request) {
request.followRedirects = false;
request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
return request.close();
}).then(handleResponse);
});
@@ -282,6 +330,7 @@
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
@@ -294,6 +343,33 @@
});
}
+void testCrossDomainAutoRedirectWithHeaders() {
+ setupTargetServer().then((targetServer) {
+ setupServer(
+ targetServer:
+ Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
+ .then((server) {
+ HttpClient client = new HttpClient();
+
+ client
+ .getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
+ .then((HttpClientRequest request) {
+ request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
+ return request.close();
+ }).then((HttpClientResponse response) {
+ response.listen((_) => Expect.fail("Response data not expected"),
+ onDone: () {
+ Expect.equals(1, response.redirects.length);
+ targetServer.close();
+ server.close();
+ client.close();
+ });
+ });
+ });
+ });
+}
+
void testAutoRedirect301POST() {
setupServer().then((server) {
HttpClient client = new HttpClient();
@@ -441,6 +517,7 @@
testManualRedirectWithHeaders();
testAutoRedirect();
testAutoRedirectWithHeaders();
+ testCrossDomainAutoRedirectWithHeaders();
testAutoRedirect301POST();
testAutoRedirect303POST();
testAutoRedirectLimit();
diff --git a/tests/standalone_2/io/http_redirect_test.dart b/tests/standalone_2/io/http_redirect_test.dart
index e168b11..e851045 100644
--- a/tests/standalone_2/io/http_redirect_test.dart
+++ b/tests/standalone_2/io/http_redirect_test.dart
@@ -9,7 +9,7 @@
import "dart:async";
import "dart:io";
-Future<HttpServer> setupServer() {
+Future<HttpServer> setupServer({Uri targetServer}) {
Completer completer = new Completer<HttpServer>();
HttpServer.bind("127.0.0.1", 0).then((server) {
var handlers = new Map<String, Function>();
@@ -130,6 +130,8 @@
// Setup redirect checking headers.
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
response.headers.set(
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
response.statusCode = HttpStatus.movedPermanently;
@@ -137,9 +139,24 @@
});
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
response.close();
});
+ if (targetServer != null) {
+ addRequestHandler("/src-crossdomain",
+ (HttpRequest request, HttpResponse response) {
+ Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNotNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be set");
+ response.headers
+ .set(HttpHeaders.locationHeader, targetServer.toString());
+ response.statusCode = HttpStatus.movedPermanently;
+ response.close();
+ });
+ }
+
// Setup redirect for 301 where POST should not redirect.
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
Expect.equals("POST", request.method);
@@ -185,6 +202,36 @@
return completer.future;
}
+// A second HTTP server used to validate that redirect requests accross domains
+// do *not* include security-related headers.
+Future<HttpServer> setupTargetServer() {
+ Completer completer = new Completer<HttpServer>();
+ HttpServer.bind("127.0.0.1", 0).then((server) {
+ var handlers = new Map<String, Function>();
+ addRequestHandler(
+ String path, void handler(HttpRequest request, HttpResponse response)) {
+ handlers[path] = handler;
+ }
+
+ server.listen((HttpRequest request) {
+ if (request.uri.path == "/target") {
+ Expect.equals("value", request.headers.value("X-Request-Header"));
+ Expect.isNull(request.headers.value("Authorization"),
+ "expected 'Authorization' header to be removed on redirect");
+ request.response.close();
+ } else {
+ request.listen((_) {}, onDone: () {
+ request.response.statusCode = 404;
+ request.response.close();
+ });
+ }
+ });
+
+ completer.complete(server);
+ });
+ return completer.future;
+}
+
void checkRedirects(int redirectCount, HttpClientResponse response) {
if (redirectCount < 2) {
Expect.isTrue(response.redirects.isEmpty);
@@ -252,6 +299,7 @@
.then((HttpClientRequest request) {
request.followRedirects = false;
request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
return request.close();
}).then(handleResponse);
});
@@ -284,6 +332,7 @@
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
.then((HttpClientRequest request) {
request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
return request.close();
}).then((HttpClientResponse response) {
response.listen((_) => Expect.fail("Response data not expected"),
@@ -296,6 +345,33 @@
});
}
+void testCrossDomainAutoRedirectWithHeaders() {
+ setupTargetServer().then((targetServer) {
+ setupServer(
+ targetServer:
+ Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
+ .then((server) {
+ HttpClient client = new HttpClient();
+
+ client
+ .getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
+ .then((HttpClientRequest request) {
+ request.headers.add("X-Request-Header", "value");
+ request.headers.add("Authorization", "Basic ...");
+ return request.close();
+ }).then((HttpClientResponse response) {
+ response.listen((_) => Expect.fail("Response data not expected"),
+ onDone: () {
+ Expect.equals(1, response.redirects.length);
+ targetServer.close();
+ server.close();
+ client.close();
+ });
+ });
+ });
+ });
+}
+
void testAutoRedirect301POST() {
setupServer().then((server) {
HttpClient client = new HttpClient();
@@ -443,6 +519,7 @@
testManualRedirectWithHeaders();
testAutoRedirect();
testAutoRedirectWithHeaders();
+ testCrossDomainAutoRedirectWithHeaders();
testAutoRedirect301POST();
testAutoRedirect303POST();
testAutoRedirectLimit();
diff --git a/tools/VERSION b/tools/VERSION
index 7e2479b..73354a3 100644
--- a/tools/VERSION
+++ b/tools/VERSION
@@ -28,4 +28,4 @@
MINOR 16
PATCH 0
PRERELEASE 134
-PRERELEASE_PATCH 5
\ No newline at end of file
+PRERELEASE_PATCH 6
\ No newline at end of file
