Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce cases in which browsers would trigger a CORS preflight request #1955

Merged
merged 13 commits into from
Sep 15, 2023
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
fail-fast: false
matrix:
sdk: [ 2.15.0, stable, beta ]
platform: [ vm, chrome ]
platform: [ vm, chrome, firefox ]
steps:
- uses: actions/checkout@v3
- uses: dart-lang/[email protected]
Expand Down
3 changes: 3 additions & 0 deletions dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ See the [Migration Guide][] for the complete breaking changes list.**
- Removes the accidentally added `options` argument for `Options.compose`.
- Fix wrong formatting of multi-value header in `BrowserHttpClientAdapter`.
- Add warning in debug mode when trying to send data with a `GET` request in web.
- Reduce cases in which browsers would trigger a CORS preflight request.
- Add warnings in debug mode when using `sendTimeout` and `onSendProgress` with an empty request body.
- Fix `receiveTimeout` not working correctly on web.

## 5.3.2

Expand Down
7 changes: 5 additions & 2 deletions dio/dart_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ file_reporters:
override_platforms:
chrome:
settings:
# disable web security to allow CORS requests
arguments: --disable-web-security
headless: true
firefox:
settings:
# headless argument has to be set explicitly for non-chrome browsers
arguments: --headless
84 changes: 57 additions & 27 deletions dio/lib/src/adapters/browser_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,39 +116,69 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {
);
}

final uploadStopwatch = Stopwatch();
xhr.upload.onProgress.listen((event) {
// This event will only be triggered if a request body exists.
// This code is structured to call `xhr.upload.onProgress.listen` only when
// absolutely necessary, because registering an xhr upload listener prevents
// the request from being classified as a "simple request" by the CORS spec.
// Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests
// Upload progress events only get triggered if the request body exists,
// so we can check it beforehand.
if (requestStream != null) {
if (connectTimeoutTimer != null) {
connectTimeoutTimer!.cancel();
connectTimeoutTimer = null;
xhr.upload.onProgress.listen((event) {
connectTimeoutTimer?.cancel();
connectTimeoutTimer = null;
});
}

final sendTimeout = options.sendTimeout;
if (sendTimeout != null) {
if (!uploadStopwatch.isRunning) {
uploadStopwatch.start();
}
final uploadStopwatch = Stopwatch();
xhr.upload.onProgress.listen((event) {
if (!uploadStopwatch.isRunning) {
uploadStopwatch.start();
}

final duration = uploadStopwatch.elapsed;
if (duration > sendTimeout) {
uploadStopwatch.stop();
completer.completeError(
DioException.sendTimeout(
timeout: sendTimeout,
requestOptions: options,
),
StackTrace.current,
);
xhr.abort();
}
final duration = uploadStopwatch.elapsed;
if (duration > sendTimeout) {
uploadStopwatch.stop();
completer.completeError(
DioException.sendTimeout(
timeout: sendTimeout,
requestOptions: options,
),
StackTrace.current,
);
xhr.abort();
}
});
}
if (options.onSendProgress != null &&
event.loaded != null &&
event.total != null) {
options.onSendProgress!(event.loaded!, event.total!);

final onSendProgress = options.onSendProgress;
if (onSendProgress != null) {
xhr.upload.onProgress.listen((event) {
if (event.loaded != null && event.total != null) {
onSendProgress(event.loaded!, event.total!);
}
});
}
});
} else if (!_kReleaseMode) {
if (options.sendTimeout != null) {
dev.log(
'sendTimeout cannot be used without a request body to send',
level: 900,
name: '🔔 Dio',
stackTrace: StackTrace.current,
);
}
if (options.onSendProgress != null) {
dev.log(
'onSendProgress cannot be used without a request body to send',
level: 900,
name: '🔔 Dio',
stackTrace: StackTrace.current,
);
}
}

final downloadStopwatch = Stopwatch();
xhr.onProgress.listen((event) {
Expand All @@ -159,8 +189,8 @@ class BrowserHttpClientAdapter implements HttpClientAdapter {

final receiveTimeout = options.receiveTimeout;
if (receiveTimeout != null) {
if (!uploadStopwatch.isRunning) {
uploadStopwatch.start();
if (!downloadStopwatch.isRunning) {
downloadStopwatch.start();
}

final duration = downloadStopwatch.elapsed;
Expand Down
81 changes: 81 additions & 0 deletions dio/test/request_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -330,5 +330,86 @@ void main() {
expect(response.data![0], 1);
});
});

// Test that browsers can correctly classify requests as
// either "simple" or "preflighted". Reference:
// https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests
group('CORS preflight', () {
test('empty GET is not preflighted', () async {
// If there is no preflight (OPTIONS) request, the main request
// successfully completes with status 418.
final response = await dio.get(
'/status/418',
options: Options(
validateStatus: (status) => true,
),
);
expect(response.statusCode, 418);
});

test('GET with custom headers is preflighted', () async {
// If there is a preflight (OPTIONS) request, the server fails it
// by responding with status 418. This fails CORS, so the browser
// never sends the main request and this code throws.
expect(() async {
final _ = await dio.get(
'/status/418',
options: Options(
headers: {
'x-request-header': 'value',
},
),
);
}, throwsDioExceptionConnectionError);
});

test('POST with text body is not preflighted', () async {
// If there is no preflight (OPTIONS) request, the main request
// successfully completes with status 418.
final response = await dio.post(
'/status/418',
data: 'body text',
options: Options(
validateStatus: (status) => true,
contentType: Headers.textPlainContentType,
),
);
expect(response.statusCode, 418);
});

test('POST with sendTimeout is preflighted', () async {
// If there is a preflight (OPTIONS) request, the server fails it
// by responding with status 418. This fails CORS, so the browser
// never sends the main request and this code throws.
expect(() async {
final _ = await dio.post(
'/status/418',
data: 'body text',
options: Options(
validateStatus: (status) => true,
contentType: Headers.textPlainContentType,
sendTimeout: Duration(seconds: 1),
),
);
}, throwsDioExceptionConnectionError);
});

test('POST with onSendProgress is preflighted', () async {
// If there is a preflight (OPTIONS) request, the server fails it
// by responding with status 418. This fails CORS, so the browser
// never sends the main request and this code throws.
expect(() async {
final _ = await dio.post(
'/status/418',
data: 'body text',
options: Options(
validateStatus: (status) => true,
contentType: Headers.textPlainContentType,
),
onSendProgress: (_, __) {},
);
}, throwsDioExceptionConnectionError);
});
}, testOn: 'browser');
});
}
10 changes: 10 additions & 0 deletions dio/test/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import 'dart:async';
import 'dart:convert';
import 'dart:io';
import 'dart:typed_data';

import 'package:dio/dio.dart';
import 'package:test/test.dart';

/// The current server instance.
Expand Down Expand Up @@ -155,6 +157,14 @@ void stopServer() {
final Matcher throwsSocketException =
throwsA(const TypeMatcher<SocketException>());

/// A matcher for functions that throw DioException of type connectionError.
final Matcher throwsDioExceptionConnectionError = throwsA(
allOf([
isA<DioException>(),
(DioException e) => e.type == DioExceptionType.connectionError,
]),
);

/// A stream of chunks of bytes representing a single piece of data.
class ByteStream extends StreamView<List<int>> {
ByteStream(Stream<List<int>> stream) : super(stream);
Expand Down