diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5ebe99a7b..d5fdcdcec 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -60,8 +60,8 @@ jobs: - name: '[Verify step] Test Dart packages [VM]' run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --no-flutter -- "MELOS_ROOT_PATH/scripts/dart_test.sh --platform=vm" - name: '[Verify step] Test Dart packages [Chrome]' - run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --no-flutter -- "MELOS_ROOT_PATH/scripts/dart_test.sh --platform=chrome" + run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --ignore="*http2*" --no-flutter -- "MELOS_ROOT_PATH/scripts/dart_test.sh --platform=chrome" - name: '[Verify step] Test Dart packages [Firefox]' - run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --no-flutter -- "MELOS_ROOT_PATH/scripts/dart_test.sh --platform=firefox" + run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --ignore="*http2*" --no-flutter -- "MELOS_ROOT_PATH/scripts/dart_test.sh --platform=firefox" - name: '[Verify step] Test Flutter packages' run: melos exec $(eval echo $IGNORED_PACKAGES) --ignore="*example*" --flutter -- "flutter test" diff --git a/plugins/http2_adapter/lib/src/http2_adapter.dart b/plugins/http2_adapter/lib/src/http2_adapter.dart index 0dfc3177c..d6b14fbdd 100644 --- a/plugins/http2_adapter/lib/src/http2_adapter.dart +++ b/plugins/http2_adapter/lib/src/http2_adapter.dart @@ -6,6 +6,7 @@ import 'dart:typed_data'; import 'package:dio/dio.dart'; import 'package:dio/io.dart'; import 'package:http2/http2.dart'; +import 'package:meta/meta.dart'; part 'client_setting.dart'; @@ -219,12 +220,29 @@ class Http2Adapter implements HttpClientAdapter { // Handle redirection. if (needRedirect) { + if (responseHeaders['location'] == null) { + // Redirect without location is illegal. + throw DioException.connectionError( + requestOptions: options, + reason: 'Received redirect without location header.', + ); + } final url = responseHeaders.value('location'); + // An empty `location` header is considered a self redirect. + final uri = Uri.parse(url ?? ''); redirects.add( - RedirectRecord(statusCode, options.method, Uri.parse(url ?? '')), + RedirectRecord( + statusCode, + options.method, + uri, + ), ); + final String path = resolveRedirectUri(options.uri, uri); return _fetch( - options.copyWith(path: url, maxRedirects: --options.maxRedirects), + options.copyWith( + path: path, + maxRedirects: --options.maxRedirects, + ), list != null ? Stream.fromIterable(list) : null, cancelFuture, redirects, @@ -251,6 +269,18 @@ class Http2Adapter implements HttpClientAdapter { statusCodes.contains(status); } + @visibleForTesting + static String resolveRedirectUri(Uri currentUri, Uri redirectUri) { + if (redirectUri.hasScheme) { + /// This is a full URL which has to be redirected to as is. + return redirectUri.toString(); + } + + /// This is relative with or without leading slash and is + /// resolved against the URL of the original request. + return currentUri.resolveUri(redirectUri).toString(); + } + @override void close({bool force = false}) { connectionManager.close(force: force); diff --git a/plugins/http2_adapter/pubspec.yaml b/plugins/http2_adapter/pubspec.yaml index 88788cffc..a2e91a118 100644 --- a/plugins/http2_adapter/pubspec.yaml +++ b/plugins/http2_adapter/pubspec.yaml @@ -16,6 +16,7 @@ environment: dependencies: http2: ^2.1.0 + meta: ^1.9.1 dio: ^5.2.0 dev_dependencies: diff --git a/plugins/http2_adapter/test/http2_test.dart b/plugins/http2_adapter/test/http2_test.dart index c1bb40452..a936f962e 100644 --- a/plugins/http2_adapter/test/http2_test.dart +++ b/plugins/http2_adapter/test/http2_test.dart @@ -1,4 +1,3 @@ -@TestOn('vm') import 'package:dio/dio.dart'; import 'package:dio_http2_adapter/dio_http2_adapter.dart'; import 'package:test/test.dart'; @@ -173,13 +172,58 @@ void main() { await dio.get('/drip?delay=1&numbytes=1'); }); - test('request with redirect', () async { + group('redirects', () { final dio = Dio() ..options.baseUrl = 'https://httpbun.com/' ..httpClientAdapter = Http2Adapter(ConnectionManager()); - final res = await dio.get('absolute-redirect/2'); - expect(res.statusCode, 200); + test('single', () async { + final response = await dio.get( + '/redirect', + queryParameters: {'url': 'https://httpbun.com/get'}, + ); + expect(response.isRedirect, isTrue); + expect(response.redirects.length, 1); + + final ri = response.redirects.first; + expect(ri.statusCode, 302); + expect(ri.location.path, '/get'); + expect(ri.method, 'GET'); + }); + + test( + 'empty location', + () async { + final response = await dio.get( + '/redirect', + ); + expect(response.isRedirect, isTrue); + expect(response.redirects.length, 1); + + final ri = response.redirects.first; + expect(ri.statusCode, 302); + expect(ri.location.path, '/get'); + expect(ri.method, 'GET'); + }, + skip: 'Httpbun does not support empty location redirects', + ); + + test('multiple', () async { + final response = await dio.get( + '/redirect/3', + ); + expect(response.isRedirect, isTrue); + expect(response.redirects.length, 3); + + final ri = response.redirects.first; + expect(ri.statusCode, 302); + expect(ri.method, 'GET'); + }); + + test('request with redirect', () async { + final res = await dio.get('absolute-redirect/2'); + expect(res.statusCode, 200); + }); }); test('header value types implicit support', () async { diff --git a/plugins/http2_adapter/test/pinning_test.dart b/plugins/http2_adapter/test/pinning_test.dart index b669e5c8e..2843aab16 100644 --- a/plugins/http2_adapter/test/pinning_test.dart +++ b/plugins/http2_adapter/test/pinning_test.dart @@ -1,4 +1,3 @@ -@TestOn('vm') import 'dart:io'; import 'package:crypto/crypto.dart'; diff --git a/plugins/http2_adapter/test/redirect_test.dart b/plugins/http2_adapter/test/redirect_test.dart new file mode 100644 index 000000000..d81d4d3ab --- /dev/null +++ b/plugins/http2_adapter/test/redirect_test.dart @@ -0,0 +1,45 @@ +import 'package:dio_http2_adapter/dio_http2_adapter.dart'; +import 'package:test/test.dart'; + +void main() { + group(Http2Adapter.resolveRedirectUri, () { + test('empty location', () async { + final current = Uri.parse('https://example.com'); + final result = Http2Adapter.resolveRedirectUri( + current, + Uri.parse(''), + ); + + expect(result, current.toString()); + }); + + test('relative location 1', () async { + final result = Http2Adapter.resolveRedirectUri( + Uri.parse('https://example.com/foo'), + Uri.parse('/bar'), + ); + + expect(result, 'https://example.com/bar'); + }); + + test('relative location 2', () async { + final result = Http2Adapter.resolveRedirectUri( + Uri.parse('https://example.com/foo'), + Uri.parse('../bar'), + ); + + expect(result, 'https://example.com/bar'); + }); + + test('different location', () async { + final current = Uri.parse('https://example.com/foo'); + final target = 'https://somewhere.com/bar'; + final result = Http2Adapter.resolveRedirectUri( + current, + Uri.parse(target), + ); + + expect(result, target); + }); + }); +} diff --git a/plugins/http2_adapter/test/request_test.dart b/plugins/http2_adapter/test/request_test.dart index b1043004b..dd8ab872d 100644 --- a/plugins/http2_adapter/test/request_test.dart +++ b/plugins/http2_adapter/test/request_test.dart @@ -1,4 +1,3 @@ -@TestOn('vm') import 'dart:io'; import 'package:dio/dio.dart'; diff --git a/plugins/http2_adapter/test/timeout_test.dart b/plugins/http2_adapter/test/timeout_test.dart index 3c2a198d3..21ebbfced 100644 --- a/plugins/http2_adapter/test/timeout_test.dart +++ b/plugins/http2_adapter/test/timeout_test.dart @@ -1,4 +1,3 @@ -@TestOn('vm') import 'dart:async'; import 'package:dio/dio.dart';