Skip to content

Commit f5e6ff5

Browse files
PaulAllanSturmCommit Queue
authored andcommitted
Avoid throwing an exception if authentication isn't requested
Only process authentication if there are credentials available or an authenticate callback to get credentials. This prevents an exception being thrown for a malformed www-authenticate header when the client code hasn't asked HttpClient to perform authentication. [email protected] Bug: #46442 Change-Id: I3d3e70cec936f717f822b2fe170ddc526a74ebf9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405280 Reviewed-by: Brian Quinlan <[email protected]> Commit-Queue: Brian Quinlan <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]>
1 parent ae5cc18 commit f5e6ff5

File tree

3 files changed

+125
-3
lines changed

3 files changed

+125
-3
lines changed

sdk/lib/_http/http_impl.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,11 +773,14 @@ class _HttpClientResponse extends _HttpInboundMessageListInt
773773
}
774774

775775
bool get _shouldAuthenticate {
776-
// Only try to authenticate if there is a challenge in the response.
776+
// Only try to authenticate if there is a challenge in the response and
777+
// the client has credentials or an authentication function.
777778
List<String>? challenge = headers[HttpHeaders.wwwAuthenticateHeader];
778779
return statusCode == HttpStatus.unauthorized &&
779780
challenge != null &&
780-
challenge.length == 1;
781+
challenge.length == 1 &&
782+
(_httpClient._credentials.isNotEmpty ||
783+
_httpClient._authenticate != null);
781784
}
782785

783786
Future<HttpClientResponse> _authenticate(bool proxyAuth) {

tests/standalone/io/http_auth_digest_test.dart

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'dart:io';
77

88
import "package:convert/convert.dart";
99
import "package:crypto/crypto.dart";
10+
import "package:expect/async_helper.dart";
1011
import "package:expect/expect.dart";
1112

1213
class Server {
@@ -37,6 +38,15 @@ class Server {
3738
HttpServer.bind("127.0.0.1", 0).then((s) {
3839
server = s;
3940
server.listen((HttpRequest request) {
41+
if (request.uri.path == "/malformedAuthenticate") {
42+
request.response.statusCode = HttpStatus.unauthorized;
43+
// This authenticate header is malformed because of missing commas
44+
request.response.headers.set(HttpHeaders.wwwAuthenticateHeader,
45+
'Digest realm="$realm" nonce="$nonce" domain="/digest/"');
46+
request.response.close();
47+
return;
48+
}
49+
4050
sendUnauthorizedResponse(HttpResponse response, {stale = false}) {
4151
response.statusCode = HttpStatus.unauthorized;
4252
StringBuffer authHeader = new StringBuffer();
@@ -309,6 +319,54 @@ void testNextNonce() {
309319
});
310320
}
311321

322+
void testMalformedAuthenticateHeaderNoAuthHandler() {
323+
Server.start('MD5', 'auth').then((server) async {
324+
HttpClient client = new HttpClient();
325+
final uri = Uri.parse(
326+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
327+
328+
// Request should resolve normally if no authentication is configured
329+
await client.getUrl(uri).then((request) => request.close());
330+
331+
server.shutdown();
332+
client.close();
333+
});
334+
}
335+
336+
void testMalformedAuthenticateHeaderWithAuthHandler() {
337+
Server.start('MD5', 'auth').then((server) async {
338+
HttpClient client = new HttpClient();
339+
final uri = Uri.parse(
340+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
341+
342+
// Request should throw an exception if the authenticate handler is set
343+
client.authenticate =
344+
(Uri url, String scheme, String? realm) async => false;
345+
await asyncExpectThrows<HttpException>(
346+
client.getUrl(uri).then((request) => request.close()));
347+
348+
server.shutdown();
349+
client.close();
350+
});
351+
}
352+
353+
void testMalformedAuthenticateHeaderWithCredentials() {
354+
Server.start('MD5', 'auth').then((server) async {
355+
HttpClient client = new HttpClient();
356+
final uri = Uri.parse(
357+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
358+
359+
// Request should throw an exception if credentials have been added
360+
client.addCredentials(
361+
uri, 'realm', HttpClientDigestCredentials('dart', 'password'));
362+
await asyncExpectThrows<HttpException>(
363+
client.getUrl(uri).then((request) => request.close()));
364+
365+
server.shutdown();
366+
client.close();
367+
});
368+
}
369+
312370
// An Apache virtual directory configuration like this can be used for
313371
// running the local server tests.
314372
//
@@ -375,6 +433,9 @@ main() {
375433
testAuthenticateCallback("MD5", "auth-int");
376434
testStaleNonce();
377435
testNextNonce();
436+
testMalformedAuthenticateHeaderNoAuthHandler();
437+
testMalformedAuthenticateHeaderWithAuthHandler();
438+
testMalformedAuthenticateHeaderWithCredentials();
378439
// These teste are not normally run. They can be used for locally
379440
// testing with another web server (e.g. Apache).
380441
//testLocalServerDigest();

tests/standalone/io/http_auth_test.dart

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:async';
66
import 'dart:convert';
77
import 'dart:io';
88

9+
import "package:expect/async_helper.dart";
910
import "package:expect/expect.dart";
1011

1112
class Server {
@@ -23,7 +24,14 @@ class Server {
2324
response.close();
2425
return;
2526
}
26-
;
27+
28+
if (request.uri.path == "/malformedAuthenticate") {
29+
response.statusCode = HttpStatus.unauthorized;
30+
response.headers.set(HttpHeaders.wwwAuthenticateHeader,
31+
'Basic malformed header no commas');
32+
response.close();
33+
return;
34+
}
2735

2836
String username;
2937
String password;
@@ -204,6 +212,53 @@ void testBasicAuthenticateCallback() {
204212
});
205213
}
206214

215+
void testMalformedAuthenticateHeaderNoAuthHandler() {
216+
setupServer().then((server) async {
217+
HttpClient client = new HttpClient();
218+
final uri = Uri.parse(
219+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
220+
221+
// Request should resolve normally if no authentication is configured
222+
await client.getUrl(uri).then((request) => request.close());
223+
224+
server.shutdown();
225+
client.close();
226+
});
227+
}
228+
229+
void testMalformedAuthenticateHeaderWithAuthHandler() {
230+
setupServer().then((server) async {
231+
HttpClient client = new HttpClient();
232+
final uri = Uri.parse(
233+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
234+
235+
// Request should throw an exception if the authenticate handler is set
236+
client.authenticate = (url, scheme, realm) async => false;
237+
await asyncExpectThrows<HttpException>(
238+
client.getUrl(uri).then((request) => request.close()));
239+
240+
server.shutdown();
241+
client.close();
242+
});
243+
}
244+
245+
void testMalformedAuthenticateHeaderWithCredentials() {
246+
setupServer().then((server) async {
247+
HttpClient client = new HttpClient();
248+
final uri = Uri.parse(
249+
'http://${InternetAddress.loopbackIPv4.address}:${server.port}/malformedAuthenticate');
250+
251+
// Request should throw an exception if credentials have been added
252+
client.addCredentials(
253+
uri, 'realm', HttpClientBasicCredentials('dart', 'password'));
254+
await asyncExpectThrows<HttpException>(
255+
client.getUrl(uri).then((request) => request.close()));
256+
257+
server.shutdown();
258+
client.close();
259+
});
260+
}
261+
207262
void testLocalServerBasic() {
208263
HttpClient client = new HttpClient();
209264

@@ -250,6 +305,9 @@ main() {
250305
testBasicNoCredentials();
251306
testBasicCredentials();
252307
testBasicAuthenticateCallback();
308+
testMalformedAuthenticateHeaderNoAuthHandler();
309+
testMalformedAuthenticateHeaderWithAuthHandler();
310+
testMalformedAuthenticateHeaderWithCredentials();
253311
// These teste are not normally run. They can be used for locally
254312
// testing with another web server (e.g. Apache).
255313
//testLocalServerBasic();

0 commit comments

Comments
 (0)