diff --git a/README.md b/README.md index 7db673f..cb501cc 100644 --- a/README.md +++ b/README.md @@ -227,11 +227,25 @@ Sometimes you need to retry a request due to different circumstances, an expired ```dart class ExpiredTokenRetryPolicy extends RetryPolicy { + @override + int get maxRetryAttempts => 2; + + @override + bool shouldAttemptRetryOnException(Exception reason, BaseRequest request) { + // Log the exception for debugging + print('Request failed: ${reason.toString()}'); + print('Request URL: ${request.url}'); + + // Retry on network exceptions, but not on client errors + return reason is SocketException || reason is TimeoutException; + } + @override Future shouldAttemptRetryOnResponse(BaseResponse response) async { if (response.statusCode == 401) { // Perform your token refresh here. - + print('Token expired, refreshing...'); + return true; } @@ -242,13 +256,56 @@ class ExpiredTokenRetryPolicy extends RetryPolicy { You can also set the maximum amount of retry attempts with `maxRetryAttempts` property or override the `shouldAttemptRetryOnException` if you want to retry the request after it failed with an exception. +### RetryPolicy Interface + +The `RetryPolicy` abstract class provides the following methods that you can override: + +- **`shouldAttemptRetryOnException(Exception reason, BaseRequest request)`**: Called when an exception occurs during the request. Return `true` to retry, `false` to fail immediately. +- **`shouldAttemptRetryOnResponse(BaseResponse response)`**: Called after receiving a response. Return `true` to retry, `false` to accept the response. +- **`maxRetryAttempts`**: The maximum number of retry attempts (default: 1). +- **`delayRetryAttemptOnException({required int retryAttempt})`**: Delay before retrying after an exception (default: no delay). +- **`delayRetryAttemptOnResponse({required int retryAttempt})`**: Delay before retrying after a response (default: no delay). + +### Using Retry Policies + +To use a retry policy, pass it to the `InterceptedClient` or `InterceptedHttp`: + +```dart +final client = InterceptedClient.build( + interceptors: [WeatherApiInterceptor()], + retryPolicy: ExpiredTokenRetryPolicy(), +); +``` + Sometimes it is helpful to have a cool-down phase between multiple requests. This delay could for example also differ between the first and the second retry attempt as shown in the following example. ```dart class ExpiredTokenRetryPolicy extends RetryPolicy { + @override + int get maxRetryAttempts => 3; + + @override + bool shouldAttemptRetryOnException(Exception reason, BaseRequest request) { + // Only retry on network-related exceptions + return reason is SocketException || reason is TimeoutException; + } + + @override + Future shouldAttemptRetryOnResponse(BaseResponse response) async { + // Retry on server errors (5xx) and authentication errors (401) + return response.statusCode >= 500 || response.statusCode == 401; + } + + @override + Duration delayRetryAttemptOnException({required int retryAttempt}) { + // Exponential backoff for exceptions + return Duration(milliseconds: (250 * math.pow(2.0, retryAttempt - 1)).round()); + } + @override Duration delayRetryAttemptOnResponse({required int retryAttempt}) { - return const Duration(milliseconds: 250) * math.pow(2.0, retryAttempt); + // Exponential backoff for response-based retries + return Duration(milliseconds: (250 * math.pow(2.0, retryAttempt - 1)).round()); } } ``` diff --git a/lib/http/intercepted_client.dart b/lib/http/intercepted_client.dart index 0a9deac..1f19856 100644 --- a/lib/http/intercepted_client.dart +++ b/lib/http/intercepted_client.dart @@ -274,16 +274,89 @@ class InterceptedClient extends BaseClient { /// of the response Future _attemptRequest(BaseRequest request, {bool isStream = false}) async { + _retryCount = 0; // Reset retry count for each new request + return _attemptRequestWithRetries(request, isStream: isStream); + } + + /// Internal method that handles the actual request with retry logic + Future _attemptRequestWithRetries(BaseRequest request, + {bool isStream = false}) async { BaseResponse response; try { // Intercept request final interceptedRequest = await _interceptRequest(request); - var stream = requestTimeout == null - ? await _inner.send(interceptedRequest) - : await _inner - .send(interceptedRequest) - .timeout(requestTimeout!, onTimeout: onRequestTimeout); + StreamedResponse stream; + if (requestTimeout == null) { + stream = await _inner.send(interceptedRequest); + } else { + // Use a completer to properly handle timeout and cancellation + final completer = Completer(); + final Future requestFuture = + _inner.send(interceptedRequest); + + // Set up timeout with proper cleanup + Timer? timeoutTimer; + bool isCompleted = false; + + timeoutTimer = Timer(requestTimeout!, () { + if (!isCompleted) { + isCompleted = true; + if (onRequestTimeout != null) { + // If timeout callback is provided, use it + try { + final timeoutResponse = onRequestTimeout!(); + if (timeoutResponse is Future) { + timeoutResponse.then((response) { + if (!completer.isCompleted) { + completer.complete(response); + } + }).catchError((error) { + if (!completer.isCompleted) { + completer.completeError(error); + } + }); + } else { + if (!completer.isCompleted) { + completer.complete(timeoutResponse); + } + } + } catch (error) { + if (!completer.isCompleted) { + completer.completeError(error); + } + } + } else { + // Default timeout behavior + if (!completer.isCompleted) { + completer.completeError(Exception( + 'Request timeout after ${requestTimeout!.inMilliseconds}ms')); + } + } + } + }); + + // Handle the actual request completion + requestFuture.then((streamResponse) { + timeoutTimer?.cancel(); + if (!isCompleted) { + isCompleted = true; + if (!completer.isCompleted) { + completer.complete(streamResponse); + } + } + }).catchError((error) { + timeoutTimer?.cancel(); + if (!isCompleted) { + isCompleted = true; + if (!completer.isCompleted) { + completer.completeError(error); + } + } + }); + + stream = await completer.future; + } response = isStream ? stream : await Response.fromStream(stream); @@ -293,7 +366,7 @@ class InterceptedClient extends BaseClient { _retryCount += 1; await Future.delayed(retryPolicy! .delayRetryAttemptOnResponse(retryAttempt: _retryCount)); - return _attemptRequest(request, isStream: isStream); + return _attemptRequestWithRetries(request, isStream: isStream); } } on Exception catch (error) { if (retryPolicy != null && @@ -302,13 +375,12 @@ class InterceptedClient extends BaseClient { _retryCount += 1; await Future.delayed(retryPolicy! .delayRetryAttemptOnException(retryAttempt: _retryCount)); - return _attemptRequest(request, isStream: isStream); + return _attemptRequestWithRetries(request, isStream: isStream); } else { rethrow; } } - _retryCount = 0; return response; } diff --git a/lib/models/retry_policy.dart b/lib/models/retry_policy.dart index 2535d35..bab5515 100644 --- a/lib/models/retry_policy.dart +++ b/lib/models/retry_policy.dart @@ -12,8 +12,9 @@ import 'package:http/http.dart'; /// int get maxRetryAttempts => 2; /// /// @override -/// bool shouldAttemptRetryOnException(Exception reason) { +/// bool shouldAttemptRetryOnException(Exception reason, BaseRequest request) { /// log(reason.toString()); +/// log("Request URL: ${request.url}"); /// /// return false; /// } @@ -36,20 +37,45 @@ import 'package:http/http.dart'; abstract class RetryPolicy { /// Defines whether the request should be retried when an Exception occurs /// while making said request to the server. + /// + /// [reason] - The exception that occurred during the request + /// [request] - The original request that failed + /// + /// Returns `true` if the request should be retried, `false` otherwise. FutureOr shouldAttemptRetryOnException( Exception reason, BaseRequest request) => false; /// Defines whether the request should be retried after the request has /// received `response` from the server. + /// + /// [response] - The response received from the server + /// + /// Returns `true` if the request should be retried, `false` otherwise. + /// Common use cases include retrying on 401 (Unauthorized) or 500 (Server Error). FutureOr shouldAttemptRetryOnResponse(BaseResponse response) => false; /// Number of maximum request attempts that can be retried. + /// + /// Default is 1, meaning the original request plus 1 retry attempt. + /// Set to 0 to disable retries, or higher values for more retry attempts. int get maxRetryAttempts => 1; + /// Delay before retrying when an exception occurs. + /// + /// [retryAttempt] - The current retry attempt number (1-based) + /// + /// Returns the delay duration. Default is no delay (Duration.zero). + /// Consider implementing exponential backoff for production use. Duration delayRetryAttemptOnException({required int retryAttempt}) => Duration.zero; + /// Delay before retrying when a response indicates retry is needed. + /// + /// [retryAttempt] - The current retry attempt number (1-based) + /// + /// Returns the delay duration. Default is no delay (Duration.zero). + /// Consider implementing exponential backoff for production use. Duration delayRetryAttemptOnResponse({required int retryAttempt}) => Duration.zero; } diff --git a/lib/utils/query_parameters.dart b/lib/utils/query_parameters.dart index 2dc6ac4..913c827 100644 --- a/lib/utils/query_parameters.dart +++ b/lib/utils/query_parameters.dart @@ -1,12 +1,36 @@ /// Takes a string and appends [parameters] as query parameters of [url]. /// -/// It does not check if [url] is valid, it just appends the parameters. +/// It validates the URL structure and properly encodes both keys and values +/// to prevent URL injection attacks. String buildUrlString(String url, Map? parameters) { // Avoids unnecessary processing. if (parameters == null) return url; // Check if there are parameters to add. if (parameters.isNotEmpty) { + // Validate URL structure to prevent injection + // First check if it looks like a valid HTTP/HTTPS URL + if (!url.startsWith('http://') && !url.startsWith('https://')) { + throw ArgumentError( + 'Invalid URL structure: $url - must be a valid HTTP/HTTPS URL', + ); + } + + try { + final uri = Uri.parse(url); + // Additional validation: ensure it has a host + if (uri.host.isEmpty) { + throw ArgumentError( + 'Invalid URL structure: $url - must have a valid host', + ); + } + } catch (e) { + if (e is ArgumentError) { + rethrow; + } + throw ArgumentError('Invalid URL structure: $url'); + } + // Checks if the string url already has parameters. if (url.contains("?")) { url += "&"; @@ -14,22 +38,26 @@ String buildUrlString(String url, Map? parameters) { url += "?"; } - // Concat every parameter to the string url. + // Concat every parameter to the string url with proper encoding parameters.forEach((key, value) { + // Encode the key to prevent injection + final encodedKey = Uri.encodeQueryComponent(key); + if (value is List) { if (value is List) { for (String singleValue in value) { - url += "$key=${Uri.encodeQueryComponent(singleValue)}&"; + url += "$encodedKey=${Uri.encodeQueryComponent(singleValue)}&"; } } else { for (dynamic singleValue in value) { - url += "$key=${Uri.encodeQueryComponent(singleValue.toString())}&"; + url += + "$encodedKey=${Uri.encodeQueryComponent(singleValue.toString())}&"; } } } else if (value is String) { - url += "$key=${Uri.encodeQueryComponent(value)}&"; + url += "$encodedKey=${Uri.encodeQueryComponent(value)}&"; } else { - url += "$key=${Uri.encodeQueryComponent(value.toString())}&"; + url += "$encodedKey=${Uri.encodeQueryComponent(value.toString())}&"; } }); diff --git a/test/utils/utils_test.dart b/test/utils/utils_test.dart index 5bcd64e..12279b8 100644 --- a/test/utils/utils_test.dart +++ b/test/utils/utils_test.dart @@ -1,5 +1,5 @@ -import 'package:test/test.dart'; import 'package:http_interceptor/utils/utils.dart'; +import 'package:test/test.dart'; main() { group("buildUrlString", () { @@ -12,8 +12,10 @@ main() { String parameterUrl = buildUrlString(url, parameters); // Assert - expect(parameterUrl, - equals("https://www.google.com/helloworld?foo=bar&num=0")); + expect( + parameterUrl, + equals("https://www.google.com/helloworld?foo=bar&num=0"), + ); }); test("Adds parameters to a URL string with parameters", () { // Arrange @@ -25,9 +27,11 @@ main() { // Assert expect( - parameterUrl, - equals( - "https://www.google.com/helloworld?foo=bar&num=0&extra=1&extra2=anotherone")); + parameterUrl, + equals( + "https://www.google.com/helloworld?foo=bar&num=0&extra=1&extra2=anotherone", + ), + ); }); test("Adds parameters with array to a URL string without parameters", () { // Arrange @@ -41,8 +45,60 @@ main() { String parameterUrl = buildUrlString(url, parameters); // Assert - expect(parameterUrl, - equals("https://www.google.com/helloworld?foo=bar&num=0&num=1")); + expect( + parameterUrl, + equals("https://www.google.com/helloworld?foo=bar&num=0&num=1"), + ); + }); + + test("Properly encodes parameter keys to prevent injection", () { + // Arrange + String url = "https://www.google.com/helloworld"; + Map parameters = { + "normal_key": "normal_value", + "key&with=special": "value&with=special", + }; + + // Act + String parameterUrl = buildUrlString(url, parameters); + + // Assert + expect(parameterUrl, contains("normal_key=normal_value")); + expect( + parameterUrl, + contains(Uri.encodeQueryComponent("key&with=special")), + ); + expect( + parameterUrl, + contains(Uri.encodeQueryComponent("value&with=special")), + ); + // Should not contain unencoded special characters that could cause injection + expect(parameterUrl.split('?')[1], isNot(contains("&with=special&"))); + }); + + test("Validates URL structure and throws error for invalid URLs", () { + // Arrange + String invalidUrl = "not a valid url"; + Map parameters = {"key": "value"}; + + // Act & Assert + expect( + () => buildUrlString(invalidUrl, parameters), + throwsA(isA()), + ); + }); + + test("Validates URL structure and throws error for URLs without scheme", + () { + // Arrange + String invalidUrl = "example.com/path"; // No scheme + Map parameters = {"key": "value"}; + + // Act & Assert + expect( + () => buildUrlString(invalidUrl, parameters), + throwsA(isA()), + ); }); }); }