From 2c58c2a06c85b7e68253c2c0899064749ba5edeb Mon Sep 17 00:00:00 2001 From: Ashique P S Date: Tue, 12 Apr 2022 23:57:35 +0530 Subject: [PATCH] Fixed the invocation of refresh! method on SharedCredentials (#2686) --- gems/aws-sdk-core/CHANGELOG.md | 2 ++ .../lib/aws-sdk-core/plugins/retry_errors.rb | 14 ++++++++++++-- .../spec/aws/plugins/retry_errors_legacy_spec.rb | 9 +++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 0d05641ffe8..e8c2f05fbe4 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Don't call `refresh!` on non-refreshable `Credentials` when retrying errors (#2685). + 3.130.0 (2022-03-11) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index a9c3631665a..e7bb4f5cd98 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -313,12 +313,17 @@ def exponential_backoff(retries) def retry_request(context, error) context.retries += 1 - context.config.credentials.refresh! if error.expired_credentials? + context.config.credentials.refresh! if refresh_credentials?(context, error) context.http_request.body.rewind context.http_response.reset call(context) end + def refresh_credentials?(context, error) + error.expired_credentials? && + context.config.credentials.respond_to?(:refresh!) + end + def add_retry_headers(context) request_pairs = { 'attempt' => context.retries, @@ -383,7 +388,7 @@ def retry_if_possible(response, error_inspector) def retry_request(context, error) delay_retry(context) context.retries += 1 - context.config.credentials.refresh! if error.expired_credentials? + context.config.credentials.refresh! if refresh_credentials?(context, error) context.http_request.body.rewind context.http_response.reset call(context) @@ -399,6 +404,11 @@ def should_retry?(context, error) response_truncatable?(context) end + def refresh_credentials?(context, error) + error.expired_credentials? && + context.config.credentials.respond_to?(:refresh!) + end + def retry_limit(context) context.config.retry_limit end diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb index 795868baf14..067231e8004 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative '../../spec_helper' +require_relative '../../retry_errors_helper' module Aws module Plugins @@ -207,12 +208,20 @@ module Plugins end it 'retries if creds expire and are refreshable' do + # Note: this adds the refresh! method to credentials expect(credentials).to receive(:refresh!).exactly(3).times resp.error = RetryErrorsSvc::Errors::AuthFailure.new(nil, nil) handle { |_context| resp } expect(resp.context.retries).to eq(3) end + it 'does not call refresh! when error is expired credentials and clock skew' do + resp.error = RetryErrorsSvc::Errors::RequestExpired.new(nil, nil) + resp.context.http_response.headers['date'] = (Time.now + 10*60).iso8601 + handle { |_context| resp } + expect(resp.context.retries).to eq(1) + end + it 'retries if endpoint discovery error is detected' do config.api.endpoint_operation = :describe_endpoints DescribeEndpointsRequest = Seahorse::Model::Shapes::StructureShape.new(