mirror of
https://github.com/dart-lang/sdk
synced 2024-10-04 17:04:56 +00:00
Disable security-related headers on redirect.
TESTED=updated unit tests Bug: https://github.com/dart-lang/sdk/issues/45410 Change-Id: I7c555a4818fd719d42748b6a18780e3d9b3ee147 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/229947 Reviewed-by: Alexander Aprelev <aam@google.com> Commit-Queue: Brian Quinlan <bquinlan@google.com>
This commit is contained in:
parent
ac38a9df1c
commit
f5bb0b33f5
|
@ -19,6 +19,11 @@ The `Platform.packageRoot` API has been removed. It had been marked deprecated
|
|||
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):
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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]!);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -7,7 +7,7 @@ import "package:expect/expect.dart";
|
|||
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 @@ Future<HttpServer> 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<HttpServer> 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<HttpServer> setupServer() {
|
|||
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 @@ 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();
|
||||
|
|
|
@ -9,7 +9,7 @@ import "package:expect/expect.dart";
|
|||
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 @@ Future<HttpServer> 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<HttpServer> 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<HttpServer> setupServer() {
|
|||
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 @@ 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();
|
||||
|
|
Loading…
Reference in a new issue