diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4a0849e45..0a72dbe28 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -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/setup-dart@v1.3 diff --git a/dio/CHANGELOG.md b/dio/CHANGELOG.md index 53788bfe8..bf2f3a650 100644 --- a/dio/CHANGELOG.md +++ b/dio/CHANGELOG.md @@ -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 diff --git a/dio/dart_test.yaml b/dio/dart_test.yaml index 149442f40..d46838e77 100644 --- a/dio/dart_test.yaml +++ b/dio/dart_test.yaml @@ -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 diff --git a/dio/lib/src/adapters/browser_adapter.dart b/dio/lib/src/adapters/browser_adapter.dart index 9ae672c1a..eab026f17 100644 --- a/dio/lib/src/adapters/browser_adapter.dart +++ b/dio/lib/src/adapters/browser_adapter.dart @@ -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) { @@ -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; diff --git a/dio/test/request_integration_test.dart b/dio/test/request_integration_test.dart index 2b747e5c7..e95b409dd 100644 --- a/dio/test/request_integration_test.dart +++ b/dio/test/request_integration_test.dart @@ -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'); }); } diff --git a/dio/test/utils.dart b/dio/test/utils.dart index 6198cde09..90bd036da 100644 --- a/dio/test/utils.dart +++ b/dio/test/utils.dart @@ -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. @@ -155,6 +157,14 @@ void stopServer() { final Matcher throwsSocketException = throwsA(const TypeMatcher()); +/// A matcher for functions that throw DioException of type connectionError. +final Matcher throwsDioExceptionConnectionError = throwsA( + allOf([ + isA(), + (DioException e) => e.type == DioExceptionType.connectionError, + ]), +); + /// A stream of chunks of bytes representing a single piece of data. class ByteStream extends StreamView> { ByteStream(Stream> stream) : super(stream);