Skip to content

Commit

Permalink
Fix empty location redirects and add separate test
Browse files Browse the repository at this point in the history
  • Loading branch information
kuhnroyal committed Feb 19, 2024
1 parent 438ac58 commit 507cbd7
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 21 deletions.
31 changes: 19 additions & 12 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,30 +220,24 @@ class Http2Adapter implements HttpClientAdapter {

// Handle redirection.
if (needRedirect) {
final url = responseHeaders.value('location');
if (url == null) {
if (responseHeaders['location'] == null) {
// Redirect without location is illegal.
throw DioException.connectionError(
requestOptions: options,
reason: 'Received redirect without location header.',
);
}
final uri = Uri.parse(url);
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,
),
);
final String path;
if (uri.hasScheme) {
/// This is a full URL which has to be redirected to as is.
path = uri.toString();
} else {
/// This is relative with or without leading slash and is
/// resolved against the URL of the original request.
path = options.uri.resolveUri(uri).path;
}
final String path = resolveRedirectUri(options.uri, uri);
return _fetch(
options.copyWith(
path: path,
Expand Down Expand Up @@ -274,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
28 changes: 19 additions & 9 deletions plugins/http2_adapter/test/http2_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,30 +182,40 @@ void main() {
final response = await dio.get(
'/redirect',
queryParameters: {'url': 'https://httpbun.com/get'},
onReceiveProgress: (received, total) {
// ignore progress
},
);
expect(response.isRedirect, isTrue);

// Redirects are not supported in web.
// Rhe browser will follow the redirects automatically.
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);

// Redirects are not supported in web.
// The browser will follow the redirects automatically.
expect(response.redirects.length, 3);

final ri = response.redirects.first;
expect(ri.statusCode, 302);
expect(ri.method, 'GET');
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);
});
});
}

0 comments on commit 507cbd7

Please sign in to comment.