Skip to content

Commit

Permalink
[http2] Fix Http2Adapter not redirecting for relative URLs (#2101)
Browse files Browse the repository at this point in the history
Fixes a redirect bug discovered during
#2077 where the adapter would fail to
properly redirect if the location header contains a relative URL.


<!-- Write down your pull request descriptions. -->

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

### Additional context and info (if any)

<!-- Provide more context and info about the PR. -->
  • Loading branch information
kuhnroyal authored Feb 28, 2024
1 parent a0304b2 commit 7b65a97
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
34 changes: 32 additions & 2 deletions plugins/http2_adapter/lib/src/http2_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions plugins/http2_adapter/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ environment:

dependencies:
http2: ^2.1.0
meta: ^1.9.1
dio: ^5.2.0

dev_dependencies:
Expand Down
52 changes: 48 additions & 4 deletions plugins/http2_adapter/test/http2_test.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion plugins/http2_adapter/test/pinning_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
@TestOn('vm')
import 'dart:io';

import 'package:crypto/crypto.dart';
Expand Down
45 changes: 45 additions & 0 deletions plugins/http2_adapter/test/redirect_test.dart
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
1 change: 0 additions & 1 deletion plugins/http2_adapter/test/request_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
@TestOn('vm')
import 'dart:io';

import 'package:dio/dio.dart';
Expand Down
1 change: 0 additions & 1 deletion plugins/http2_adapter/test/timeout_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
@TestOn('vm')
import 'dart:async';

import 'package:dio/dio.dart';
Expand Down

0 comments on commit 7b65a97

Please sign in to comment.