diff --git a/CHANGELOG.md b/CHANGELOG.md index 533a4bd00b6..0d2bee122cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ - `IdbFactory.supportsDatabaseNames` has been deprecated. It will always return `false`. +#### `dart:io` + +- **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. + ### Tools #### Dart command line diff --git a/sdk/lib/_http/http.dart b/sdk/lib/_http/http.dart index b63102a81af..fdb4de01838 100644 --- a/sdk/lib/_http/http.dart +++ b/sdk/lib/_http/http.dart @@ -1715,8 +1715,39 @@ abstract class HttpClientRequest implements IOSink { /// 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 6c9c786dbf4..622c662e832 100644 --- a/sdk/lib/_http/http_impl.dart +++ b/sdk/lib/_http/http_impl.dart @@ -667,7 +667,7 @@ class _HttpClientResponse extends _HttpInboundMessageListInt } } return _httpClient - ._openUrlFromRequest(method, url, _httpRequest) + ._openUrlFromRequest(method, url, _httpRequest, isRedirect: true) .then((request) { request._responseRedirects ..addAll(redirects) @@ -751,7 +751,8 @@ class _HttpClientResponse extends _HttpInboundMessageListInt 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 @@ class _HttpClient implements HttpClient { }); } + 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 @@ class _HttpClient implements HttpClient { ..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 f68e32f4b5d..1a192226c05 100644 --- a/tests/standalone/io/http_redirect_test.dart +++ b/tests/standalone/io/http_redirect_test.dart @@ -7,7 +7,7 @@ import "package:expect/expect.dart"; import "dart:async"; import "dart:io"; -Future setupServer() { +Future setupServer({Uri? targetServer}) { final completer = new Completer(); HttpServer.bind("127.0.0.1", 0).then((server) { var handlers = new Map(); @@ -128,6 +128,8 @@ Future setupServer() { // 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 @@ Future setupServer() { }); 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 @@ Future setupServer() { return completer.future; } +// A second HTTP server used to validate that redirect requests accross domains +// do *not* include security-related headers. +Future setupTargetServer() { + final completer = new Completer(); + HttpServer.bind("127.0.0.1", 0).then((server) { + var handlers = new Map(); + 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 @@ void testManualRedirectWithHeaders() { .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 @@ void testAutoRedirectWithHeaders() { .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 testAutoRedirectWithHeaders() { }); } +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 @@ main() { 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 e168b11b403..e851045622b 100644 --- a/tests/standalone_2/io/http_redirect_test.dart +++ b/tests/standalone_2/io/http_redirect_test.dart @@ -9,7 +9,7 @@ import "package:expect/expect.dart"; import "dart:async"; import "dart:io"; -Future setupServer() { +Future setupServer({Uri targetServer}) { Completer completer = new Completer(); HttpServer.bind("127.0.0.1", 0).then((server) { var handlers = new Map(); @@ -130,6 +130,8 @@ Future setupServer() { // 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 @@ Future setupServer() { }); 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 @@ Future setupServer() { return completer.future; } +// A second HTTP server used to validate that redirect requests accross domains +// do *not* include security-related headers. +Future setupTargetServer() { + Completer completer = new Completer(); + HttpServer.bind("127.0.0.1", 0).then((server) { + var handlers = new Map(); + 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 @@ void testManualRedirectWithHeaders() { .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 @@ void testAutoRedirectWithHeaders() { .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 testAutoRedirectWithHeaders() { }); } +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 @@ main() { testManualRedirectWithHeaders(); testAutoRedirect(); testAutoRedirectWithHeaders(); + testCrossDomainAutoRedirectWithHeaders(); testAutoRedirect301POST(); testAutoRedirect303POST(); testAutoRedirectLimit();