Skip to content

Commit f5bb0b3

Browse files
brianquinlanwhesse
brianquinlan
authored and
whesse
committed
Disable security-related headers on redirect.
TESTED=updated unit tests Bug: #45410 Change-Id: I7c555a4818fd719d42748b6a18780e3d9b3ee147 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/229947 Reviewed-by: Alexander Aprelev <[email protected]> Commit-Queue: Brian Quinlan <[email protected]>
1 parent ac38a9d commit f5bb0b3

File tree

5 files changed

+224
-8
lines changed

5 files changed

+224
-8
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ The `Platform.packageRoot` API has been removed. It had been marked deprecated
1919
in 2018, as it doesn't work with any Dart 2.x release.
2020
- Add optional `sourcePort` parameter to `Socket.connect`, `Socket.startConnect`, `RawSocket.connect` and `RawSocket.startConnect`
2121

22+
- **Breaking Change** [#45410](https://github.com/dart-lang/sdk/issues/45410):
23+
`HttpClient` no longer transmits some headers (i.e. `authorization`,
24+
`www-authenticate`, `cookie`, `cookie2`) when processing redirects to
25+
a different domain.
26+
2227
#### `dart:isolate`
2328

2429
- **Breaking Change** [#47769](https://github.com/dart-lang/sdk/issues/47769):

‎sdk/lib/_http/http.dart‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,8 +1715,39 @@ abstract class HttpClientRequest implements IOSink {
17151715
/// following the redirect.
17161716
///
17171717
/// All headers added to the request will be added to the redirection
1718-
/// request(s). However, any body send with the request will not be
1719-
/// part of the redirection request(s).
1718+
/// request(s) except when forwarding sensitive headers like
1719+
/// "Authorization", "WWW-Authenticate", and "Cookie". Those headers
1720+
/// will be skipped if following a redirect to a domain that is not a
1721+
/// subdomain match or exact match of the initial domain.
1722+
/// For example, a redirect from "foo.com" to either "foo.com" or
1723+
/// "sub.foo.com" will forward the sensitive headers, but a redirect to
1724+
/// "bar.com" will not.
1725+
///
1726+
/// Any body send with the request will not be part of the redirection
1727+
/// request(s).
1728+
///
1729+
/// For precise control of redirect handling, set this property to `false`
1730+
/// and make a separate HTTP request to process the redirect. For example:
1731+
///
1732+
/// ```dart
1733+
/// final client = HttpClient();
1734+
/// var uri = Uri.parse("http://localhost/");
1735+
/// var request = await client.getUrl(uri);
1736+
/// request.followRedirects = false;
1737+
/// var response = await request.close();
1738+
/// while (response.isRedirect) {
1739+
/// response.drain();
1740+
/// final location = response.headers.value(HttpHeaders.locationHeader);
1741+
/// if (location != null) {
1742+
/// uri = uri.resolve(location);
1743+
/// request = await client.getUrl(uri);
1744+
/// // Set the body or headers as desired.
1745+
/// request.followRedirects = false;
1746+
/// response = await request.close();
1747+
/// }
1748+
/// }
1749+
/// // Do something with the final response.
1750+
/// ```
17201751
bool followRedirects = true;
17211752

17221753
/// Set this property to the maximum number of redirects to follow

‎sdk/lib/_http/http_impl.dart‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ class _HttpClientResponse extends _HttpInboundMessageListInt
667667
}
668668
}
669669
return _httpClient
670-
._openUrlFromRequest(method, url, _httpRequest)
670+
._openUrlFromRequest(method, url, _httpRequest, isRedirect: true)
671671
.then((request) {
672672
request._responseRedirects
673673
..addAll(redirects)
@@ -751,7 +751,8 @@ class _HttpClientResponse extends _HttpInboundMessageListInt
751751
return drain().then((_) {
752752
return _httpClient
753753
._openUrlFromRequest(
754-
_httpRequest.method, _httpRequest.uri, _httpRequest)
754+
_httpRequest.method, _httpRequest.uri, _httpRequest,
755+
isRedirect: false)
755756
.then((request) => request.close());
756757
});
757758
}
@@ -2715,8 +2716,31 @@ class _HttpClient implements HttpClient {
27152716
});
27162717
}
27172718

2719+
static bool _isSubdomain(Uri subdomain, Uri domain) {
2720+
return (subdomain.scheme == domain.scheme &&
2721+
subdomain.port == domain.port &&
2722+
(subdomain.host == domain.host ||
2723+
subdomain.host.endsWith("." + domain.host)));
2724+
}
2725+
2726+
static bool _shouldCopyHeaderOnRedirect(
2727+
String headerKey, Uri originalUrl, Uri redirectUri) {
2728+
if (_isSubdomain(redirectUri, originalUrl)) {
2729+
return true;
2730+
}
2731+
2732+
const nonRedirectHeaders = [
2733+
"authorization",
2734+
"www-authenticate",
2735+
"cookie",
2736+
"cookie2"
2737+
];
2738+
return !nonRedirectHeaders.contains(headerKey.toLowerCase());
2739+
}
2740+
27182741
Future<_HttpClientRequest> _openUrlFromRequest(
2719-
String method, Uri uri, _HttpClientRequest previous) {
2742+
String method, Uri uri, _HttpClientRequest previous,
2743+
{required bool isRedirect}) {
27202744
// If the new URI is relative (to either '/' or some sub-path),
27212745
// construct a full URI from the previous one.
27222746
Uri resolved = previous.uri.resolveUri(uri);
@@ -2728,7 +2752,9 @@ class _HttpClient implements HttpClient {
27282752
..maxRedirects = previous.maxRedirects;
27292753
// Copy headers.
27302754
for (var header in previous.headers._headers.keys) {
2731-
if (request.headers[header] == null) {
2755+
if (request.headers[header] == null &&
2756+
(!isRedirect ||
2757+
_shouldCopyHeaderOnRedirect(header, resolved, previous.uri))) {
27322758
request.headers.set(header, previous.headers[header]!);
27332759
}
27342760
}

‎tests/standalone/io/http_redirect_test.dart‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import "package:expect/expect.dart";
77
import "dart:async";
88
import "dart:io";
99

10-
Future<HttpServer> setupServer() {
10+
Future<HttpServer> setupServer({Uri? targetServer}) {
1111
final completer = new Completer<HttpServer>();
1212
HttpServer.bind("127.0.0.1", 0).then((server) {
1313
var handlers = new Map<String, Function>();
@@ -128,16 +128,33 @@ Future<HttpServer> setupServer() {
128128
// Setup redirect checking headers.
129129
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
130130
Expect.equals("value", request.headers.value("X-Request-Header"));
131+
Expect.isNotNull(request.headers.value("Authorization"),
132+
"expected 'Authorization' header to be set");
131133
response.headers.set(
132134
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
133135
response.statusCode = HttpStatus.movedPermanently;
134136
response.close();
135137
});
136138
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
137139
Expect.equals("value", request.headers.value("X-Request-Header"));
140+
Expect.isNotNull(request.headers.value("Authorization"),
141+
"expected 'Authorization' header to be set");
138142
response.close();
139143
});
140144

145+
if (targetServer != null) {
146+
addRequestHandler("/src-crossdomain",
147+
(HttpRequest request, HttpResponse response) {
148+
Expect.equals("value", request.headers.value("X-Request-Header"));
149+
Expect.isNotNull(request.headers.value("Authorization"),
150+
"expected 'Authorization' header to be set");
151+
response.headers
152+
.set(HttpHeaders.locationHeader, targetServer.toString());
153+
response.statusCode = HttpStatus.movedPermanently;
154+
response.close();
155+
});
156+
}
157+
141158
// Setup redirect for 301 where POST should not redirect.
142159
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
143160
Expect.equals("POST", request.method);
@@ -183,6 +200,36 @@ Future<HttpServer> setupServer() {
183200
return completer.future;
184201
}
185202

203+
// A second HTTP server used to validate that redirect requests accross domains
204+
// do *not* include security-related headers.
205+
Future<HttpServer> setupTargetServer() {
206+
final completer = new Completer<HttpServer>();
207+
HttpServer.bind("127.0.0.1", 0).then((server) {
208+
var handlers = new Map<String, Function>();
209+
addRequestHandler(
210+
String path, void handler(HttpRequest request, HttpResponse response)) {
211+
handlers[path] = handler;
212+
}
213+
214+
server.listen((HttpRequest request) {
215+
if (request.uri.path == "/target") {
216+
Expect.equals("value", request.headers.value("X-Request-Header"));
217+
Expect.isNull(request.headers.value("Authorization"),
218+
"expected 'Authorization' header to be removed on redirect");
219+
request.response.close();
220+
} else {
221+
request.listen((_) {}, onDone: () {
222+
request.response.statusCode = 404;
223+
request.response.close();
224+
});
225+
}
226+
});
227+
228+
completer.complete(server);
229+
});
230+
return completer.future;
231+
}
232+
186233
void checkRedirects(int redirectCount, HttpClientResponse response) {
187234
if (redirectCount < 2) {
188235
Expect.isTrue(response.redirects.isEmpty);
@@ -250,6 +297,7 @@ void testManualRedirectWithHeaders() {
250297
.then((HttpClientRequest request) {
251298
request.followRedirects = false;
252299
request.headers.add("X-Request-Header", "value");
300+
request.headers.add("Authorization", "Basic ...");
253301
return request.close();
254302
}).then(handleResponse);
255303
});
@@ -282,6 +330,7 @@ void testAutoRedirectWithHeaders() {
282330
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
283331
.then((HttpClientRequest request) {
284332
request.headers.add("X-Request-Header", "value");
333+
request.headers.add("Authorization", "Basic ...");
285334
return request.close();
286335
}).then((HttpClientResponse response) {
287336
response.listen((_) => Expect.fail("Response data not expected"),
@@ -294,6 +343,33 @@ void testAutoRedirectWithHeaders() {
294343
});
295344
}
296345

346+
void testCrossDomainAutoRedirectWithHeaders() {
347+
setupTargetServer().then((targetServer) {
348+
setupServer(
349+
targetServer:
350+
Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
351+
.then((server) {
352+
HttpClient client = new HttpClient();
353+
354+
client
355+
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
356+
.then((HttpClientRequest request) {
357+
request.headers.add("X-Request-Header", "value");
358+
request.headers.add("Authorization", "Basic ...");
359+
return request.close();
360+
}).then((HttpClientResponse response) {
361+
response.listen((_) => Expect.fail("Response data not expected"),
362+
onDone: () {
363+
Expect.equals(1, response.redirects.length);
364+
targetServer.close();
365+
server.close();
366+
client.close();
367+
});
368+
});
369+
});
370+
});
371+
}
372+
297373
void testAutoRedirect301POST() {
298374
setupServer().then((server) {
299375
HttpClient client = new HttpClient();
@@ -441,6 +517,7 @@ main() {
441517
testManualRedirectWithHeaders();
442518
testAutoRedirect();
443519
testAutoRedirectWithHeaders();
520+
testCrossDomainAutoRedirectWithHeaders();
444521
testAutoRedirect301POST();
445522
testAutoRedirect303POST();
446523
testAutoRedirectLimit();

‎tests/standalone_2/io/http_redirect_test.dart‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import "package:expect/expect.dart";
99
import "dart:async";
1010
import "dart:io";
1111

12-
Future<HttpServer> setupServer() {
12+
Future<HttpServer> setupServer({Uri targetServer}) {
1313
Completer completer = new Completer<HttpServer>();
1414
HttpServer.bind("127.0.0.1", 0).then((server) {
1515
var handlers = new Map<String, Function>();
@@ -130,16 +130,33 @@ Future<HttpServer> setupServer() {
130130
// Setup redirect checking headers.
131131
addRequestHandler("/src", (HttpRequest request, HttpResponse response) {
132132
Expect.equals("value", request.headers.value("X-Request-Header"));
133+
Expect.isNotNull(request.headers.value("Authorization"),
134+
"expected 'Authorization' header to be set");
133135
response.headers.set(
134136
HttpHeaders.locationHeader, "http://127.0.0.1:${server.port}/target");
135137
response.statusCode = HttpStatus.movedPermanently;
136138
response.close();
137139
});
138140
addRequestHandler("/target", (HttpRequest request, HttpResponse response) {
139141
Expect.equals("value", request.headers.value("X-Request-Header"));
142+
Expect.isNotNull(request.headers.value("Authorization"),
143+
"expected 'Authorization' header to be set");
140144
response.close();
141145
});
142146

147+
if (targetServer != null) {
148+
addRequestHandler("/src-crossdomain",
149+
(HttpRequest request, HttpResponse response) {
150+
Expect.equals("value", request.headers.value("X-Request-Header"));
151+
Expect.isNotNull(request.headers.value("Authorization"),
152+
"expected 'Authorization' header to be set");
153+
response.headers
154+
.set(HttpHeaders.locationHeader, targetServer.toString());
155+
response.statusCode = HttpStatus.movedPermanently;
156+
response.close();
157+
});
158+
}
159+
143160
// Setup redirect for 301 where POST should not redirect.
144161
addRequestHandler("/301src", (HttpRequest request, HttpResponse response) {
145162
Expect.equals("POST", request.method);
@@ -185,6 +202,36 @@ Future<HttpServer> setupServer() {
185202
return completer.future;
186203
}
187204

205+
// A second HTTP server used to validate that redirect requests accross domains
206+
// do *not* include security-related headers.
207+
Future<HttpServer> setupTargetServer() {
208+
Completer completer = new Completer<HttpServer>();
209+
HttpServer.bind("127.0.0.1", 0).then((server) {
210+
var handlers = new Map<String, Function>();
211+
addRequestHandler(
212+
String path, void handler(HttpRequest request, HttpResponse response)) {
213+
handlers[path] = handler;
214+
}
215+
216+
server.listen((HttpRequest request) {
217+
if (request.uri.path == "/target") {
218+
Expect.equals("value", request.headers.value("X-Request-Header"));
219+
Expect.isNull(request.headers.value("Authorization"),
220+
"expected 'Authorization' header to be removed on redirect");
221+
request.response.close();
222+
} else {
223+
request.listen((_) {}, onDone: () {
224+
request.response.statusCode = 404;
225+
request.response.close();
226+
});
227+
}
228+
});
229+
230+
completer.complete(server);
231+
});
232+
return completer.future;
233+
}
234+
188235
void checkRedirects(int redirectCount, HttpClientResponse response) {
189236
if (redirectCount < 2) {
190237
Expect.isTrue(response.redirects.isEmpty);
@@ -252,6 +299,7 @@ void testManualRedirectWithHeaders() {
252299
.then((HttpClientRequest request) {
253300
request.followRedirects = false;
254301
request.headers.add("X-Request-Header", "value");
302+
request.headers.add("Authorization", "Basic ...");
255303
return request.close();
256304
}).then(handleResponse);
257305
});
@@ -284,6 +332,7 @@ void testAutoRedirectWithHeaders() {
284332
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src"))
285333
.then((HttpClientRequest request) {
286334
request.headers.add("X-Request-Header", "value");
335+
request.headers.add("Authorization", "Basic ...");
287336
return request.close();
288337
}).then((HttpClientResponse response) {
289338
response.listen((_) => Expect.fail("Response data not expected"),
@@ -296,6 +345,33 @@ void testAutoRedirectWithHeaders() {
296345
});
297346
}
298347

348+
void testCrossDomainAutoRedirectWithHeaders() {
349+
setupTargetServer().then((targetServer) {
350+
setupServer(
351+
targetServer:
352+
Uri.parse("http://127.0.0.1:${targetServer.port}/target"))
353+
.then((server) {
354+
HttpClient client = new HttpClient();
355+
356+
client
357+
.getUrl(Uri.parse("http://127.0.0.1:${server.port}/src-crossdomain"))
358+
.then((HttpClientRequest request) {
359+
request.headers.add("X-Request-Header", "value");
360+
request.headers.add("Authorization", "Basic ...");
361+
return request.close();
362+
}).then((HttpClientResponse response) {
363+
response.listen((_) => Expect.fail("Response data not expected"),
364+
onDone: () {
365+
Expect.equals(1, response.redirects.length);
366+
targetServer.close();
367+
server.close();
368+
client.close();
369+
});
370+
});
371+
});
372+
});
373+
}
374+
299375
void testAutoRedirect301POST() {
300376
setupServer().then((server) {
301377
HttpClient client = new HttpClient();
@@ -443,6 +519,7 @@ main() {
443519
testManualRedirectWithHeaders();
444520
testAutoRedirect();
445521
testAutoRedirectWithHeaders();
522+
testCrossDomainAutoRedirectWithHeaders();
446523
testAutoRedirect301POST();
447524
testAutoRedirect303POST();
448525
testAutoRedirectLimit();

0 commit comments

Comments
 (0)