🐛 Make cached connections respect redirections

plugins/http2_adapter/lib/src/connection_manager.dart

@@ -18,7 +18,10 @@ abstract class ConnectionManager {
       );
 
   /// Get the connection(may reuse) for each request.
-  Future<ClientTransportConnection> getConnection(RequestOptions options);
+  Future<ClientTransportConnection> getConnection(
+    RequestOptions options,
+    List<RedirectRecord> redirects,
+  );
 
   void removeConnection(ClientTransportConnection transport);
 

plugins/http2_adapter/lib/src/connection_manager_imp.dart

@@ -34,47 +34,60 @@ class _ConnectionManager implements ConnectionManager {
   @override
   Future<ClientTransportConnection> getConnection(
     RequestOptions options,
+    List<RedirectRecord> redirects,
   ) async {
     if (_closed) {
       throw Exception(
         "Can't establish connection after [ConnectionManager] closed!",
       );
     }
-    final uri = options.uri;
-    final domain = '${uri.host}:${uri.port}';
-    _ClientTransportConnectionState? transportState = _transportsMap[domain];
+    Uri uri = options.uri;
+    if (redirects.isNotEmpty) {
+      uri = Http2Adapter.resolveRedirectUri(uri, redirects.last.location);
+    }
+    // Identify whether the connection can be reused.
+    // [Uri.scheme] is required when redirecting from non-TLS to TLS connection.
+    final transportCacheKey = '${uri.scheme}://${uri.host}:${uri.port}';
+    _ClientTransportConnectionState? transportState =
+        _transportsMap[transportCacheKey];
     if (transportState == null) {
       Future<_ClientTransportConnectionState>? initFuture =
-          _connectFutures[domain];
+          _connectFutures[transportCacheKey];
       if (initFuture == null) {
-        _connectFutures[domain] = initFuture = _connect(options);
+        _connectFutures[transportCacheKey] =
+            initFuture = _connect(options, redirects);
       }
       try {
         transportState = await initFuture;
       } catch (e) {
-        _connectFutures.remove(domain);
+        _connectFutures.remove(transportCacheKey);
         rethrow;
       }
       if (_forceClosed) {
         transportState.dispose();
       } else {
-        _transportsMap[domain] = transportState;
-        final _ = _connectFutures.remove(domain);
+        _transportsMap[transportCacheKey] = transportState;
+        final _ = _connectFutures.remove(transportCacheKey);
       }
     } else {
       // Check whether the connection is terminated, if it is, reconnecting.
       if (!transportState.transport.isOpen) {
         transportState.dispose();
-        _transportsMap[domain] = transportState = await _connect(options);
+        _transportsMap[transportCacheKey] =
+            transportState = await _connect(options, redirects);
       }
     }
     return transportState.activeTransport;
   }
 
   Future<_ClientTransportConnectionState> _connect(
     RequestOptions options,
+    List<RedirectRecord> redirects,
   ) async {
-    final uri = options.uri;
+    Uri uri = options.uri;
+    if (redirects.isNotEmpty) {
+      uri = Http2Adapter.resolveRedirectUri(uri, redirects.last.location);
+    }
     final domain = '${uri.host}:${uri.port}';
     final clientConfig = ClientSetting();
     if (onClientCreate != null) {
@@ -279,18 +292,18 @@ class _ConnectionManager implements ConnectionManager {
 class _ClientTransportConnectionState {
   _ClientTransportConnectionState(this.transport);
 
-  ClientTransportConnection transport;
+  final ClientTransportConnection transport;
+
+  bool isActive = true;
+  late DateTime latestIdleTimeStamp;
+  Timer? _timer;
 
   ClientTransportConnection get activeTransport {
     isActive = true;
     latestIdleTimeStamp = DateTime.now();
     return transport;
   }
 
-  bool isActive = true;
-  late DateTime latestIdleTimeStamp;
-  Timer? _timer;
-
   void delayClose(Duration idleTimeout, void Function() callback) {
     const duration = Duration(milliseconds: 100);
     idleTimeout = idleTimeout < duration ? duration : idleTimeout;

plugins/http2_adapter/lib/src/http2_adapter.dart

@@ -6,7 +6,6 @@ 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';
 
@@ -59,7 +58,7 @@ class Http2Adapter implements HttpClientAdapter {
   ) async {
     late final ClientTransportConnection transport;
     try {
-      transport = await connectionManager.getConnection(options);
+      transport = await connectionManager.getConnection(options, redirects);
     } on DioH2NotSupportedException catch (e) {
       // Fallback to use the callback
       // or to another adapter (typically IOHttpClientAdapter)
@@ -253,14 +252,8 @@ class Http2Adapter implements HttpClientAdapter {
       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 = resolveRedirectUri(options.uri, uri);
+      redirects.add(RedirectRecord(statusCode, options.method, uri));
+      final String path = resolveRedirectUri(options.uri, uri).toString();
       return _fetch(
         options.copyWith(
           path: path,
@@ -292,16 +285,15 @@ class Http2Adapter implements HttpClientAdapter {
         statusCodes.contains(status);
   }
 
-  @visibleForTesting
-  static String resolveRedirectUri(Uri currentUri, Uri redirectUri) {
+  static Uri 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 a full URL which has to be redirected to as is.
+      return redirectUri;
     }
 
-    /// This is relative with or without leading slash and is
-    /// resolved against the URL of the original request.
-    return currentUri.resolveUri(redirectUri).toString();
+    // This is relative with or without leading slash and is resolved against
+    // the URL of the original request.
+    return currentUri.resolveUri(redirectUri);
   }
 
   @override
@@ -310,8 +302,7 @@ class Http2Adapter implements HttpClientAdapter {
   }
 }
 
-/// The exception when a connected socket for the [uri]
-/// does not support HTTP/2.
+/// The exception when a connected socket for the [uri] does not support HTTP/2.
 class DioH2NotSupportedException extends SocketException {
   const DioH2NotSupportedException(
     this.uri,

plugins/http2_adapter/test/redirect_test.dart

@@ -2,15 +2,14 @@ import 'package:dio_http2_adapter/dio_http2_adapter.dart';
 import 'package:test/test.dart';
 
 void main() {
-  group(Http2Adapter.resolveRedirectUri, () {
+  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());
+      expect(result.toString(), current.toString());
     });
 
     test('relative location 1', () async {
@@ -19,16 +18,15 @@ void main() {
         Uri.parse('/bar'),
       );
 
-      expect(result, 'https://example.com/bar');
+      expect(result.toString(), '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');
+      expect(result.toString(), 'https://example.com/bar');
     });
 
     test('different location', () async {
@@ -38,8 +36,7 @@ void main() {
         current,
         Uri.parse(target),
       );
-
-      expect(result, target);
+      expect(result.toString(), target);
     });
   });
 }