From 9589eb845b20d3ebaccc751babd76986aba60985 Mon Sep 17 00:00:00 2001 From: kaiion <43284426+kai-ion@users.noreply.github.com> Date: Mon, 31 Aug 2026 16:15:51 +0000 Subject: [PATCH 1/4] Wire clock-skew correction into the legacy and smithy client pipelines --- .changelog/feature-clock-skew-correction.json | 6 ++ .../include/aws/core/client/AWSClient.h | 8 +- .../include/aws/core/http/HttpRequest.h | 13 +++ .../include/smithy/client/AwsSmithyClient.h | 5 -- .../AwsSmithyClientAsyncRequestContext.h | 4 + .../smithy/client/AwsSmithyClientBase.h | 9 +- .../client/common/AwsSmithyRequestSigning.h | 82 ------------------- .../identity/signer/built-in/SigV4aSigner.h | 3 +- .../signer/AWSAuthEventStreamV4Signer.cpp | 2 +- .../source/auth/signer/AWSAuthV4Signer.cpp | 5 +- .../source/client/AWSClient.cpp | 58 +++++++------ .../source/client/ClientConfiguration.cpp | 12 +++ .../smithy/client/AwsSmithyClientBase.cpp | 51 ++++++++++-- .../aws/client/AWSClientTest.cpp | 21 +++-- .../monitoring/MonitoringTest.cpp | 6 +- .../testing/mocks/aws/client/MockAWSClient.h | 26 ++++++ 16 files changed, 175 insertions(+), 136 deletions(-) create mode 100644 .changelog/feature-clock-skew-correction.json diff --git a/.changelog/feature-clock-skew-correction.json b/.changelog/feature-clock-skew-correction.json new file mode 100644 index 000000000000..0acbae5fc1e2 --- /dev/null +++ b/.changelog/feature-clock-skew-correction.json @@ -0,0 +1,6 @@ +{ + "type": "feature", + "category": "aws-cpp-sdk-core", + "contributor": "kaiion", + "description": "Add clock skew correction: the SDK adjusts request signing timestamps by the observed client-to-service skew and retries signature errors caused by skew, so requests keep working when the client clock is off. Disable with AWS_DISABLE_CLOCK_SKEW_CORRECTION." +} diff --git a/src/aws-cpp-sdk-core/include/aws/core/client/AWSClient.h b/src/aws-cpp-sdk-core/include/aws/core/client/AWSClient.h index 1815b88abd9f..238535033580 100644 --- a/src/aws-cpp-sdk-core/include/aws/core/client/AWSClient.h +++ b/src/aws-cpp-sdk-core/include/aws/core/client/AWSClient.h @@ -60,6 +60,11 @@ namespace Aws class AmazonWebServiceRequest; + namespace Internal + { + class ClientSkew; + } + namespace Client { template @@ -345,7 +350,7 @@ namespace Aws * Try to adjust signer's clock * return true if signer's clock is adjusted, false otherwise. */ - bool AdjustClockSkew(HttpResponseOutcome& outcome, const char* signerName) const; + bool AdjustClockSkew(HttpResponseOutcome& outcome, const Aws::Utils::DateTime& timeRequestSent, const Aws::Utils::DateTime& timeResponseReceived, std::chrono::milliseconds attemptSkew) const; void AddHeadersToRequest(const std::shared_ptr& httpRequest, const Http::HeaderValueCollection& headerValues) const; void AddContentBodyToRequest(const std::shared_ptr& httpRequest, const std::shared_ptr& body, bool needsContentMd5 = false, bool isChunked = false) const; @@ -359,6 +364,7 @@ namespace Aws std::shared_ptr m_hash; long m_requestTimeoutMs; bool m_enableClockSkewAdjustment; + mutable std::shared_ptr m_clientSkew; Aws::String m_serviceName = "AWSBaseClient"; Aws::Client::RequestCompressionConfig m_requestCompressionConfig; std::shared_ptr m_userAgentInterceptor; diff --git a/src/aws-cpp-sdk-core/include/aws/core/http/HttpRequest.h b/src/aws-cpp-sdk-core/include/aws/core/http/HttpRequest.h index cb21d943d253..020425e82948 100644 --- a/src/aws-cpp-sdk-core/include/aws/core/http/HttpRequest.h +++ b/src/aws-cpp-sdk-core/include/aws/core/http/HttpRequest.h @@ -13,6 +13,8 @@ #include #include #include +#include +#include #include #include #include @@ -558,6 +560,16 @@ namespace Aws */ inline void SetSigningRegion(const Aws::String& region) { m_signingRegion = region; } + /** + * Gets the per-attempt signing timestamp override set for clock-skew correction, if any. + */ + inline const Aws::Crt::Optional& GetSigningTimestampOverride() const { return m_signingTimestampOverride; } + /** + * Sets an explicit signing timestamp for this attempt (now() + AttemptSkew). The signer uses it + * instead of its own clock; set per attempt so concurrent operations don't share skew state. + */ + inline void SetSigningTimestampOverride(const Aws::Utils::DateTime& signingTime) { m_signingTimestampOverride = signingTime; } + /** * Add a request metric * @param key, HttpClientMetricsKey defined in HttpClientMetrics.cpp @@ -618,6 +630,7 @@ namespace Aws DataSentEventHandler m_onDataSent; ContinueRequestHandler m_continueRequest; Aws::String m_signingRegion; + Aws::Crt::Optional m_signingTimestampOverride; Aws::String m_signingAccessKey; Aws::String m_resolvedRemoteHost; Aws::Monitoring::HttpClientMetricsCollection m_httpRequestMetrics; diff --git a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h index b5760248c530..fb5e388a9242 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClient.h @@ -229,11 +229,6 @@ namespace client return AwsClientRequestSigning::SignEventMessage(message, seed, ctx, m_authSchemes); } - bool AdjustClockSkew(HttpResponseOutcome& outcome, const AuthSchemeOption& authSchemeOption) const override - { - return AwsClientRequestSigning::AdjustClockSkew(outcome, authSchemeOption, m_authSchemes); - } - IdentityOutcome ResolveIdentity(const AwsSmithyClientAsyncRequestContext& ctx) const override { return AwsClientRequestSigning::ResolveIdentity(ctx, m_authSchemes); } diff --git a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientAsyncRequestContext.h b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientAsyncRequestContext.h index 1796f541916c..603feb589daf 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientAsyncRequestContext.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientAsyncRequestContext.h @@ -66,6 +66,10 @@ namespace smithy Aws::Crt::Optional m_lastError; + std::chrono::milliseconds m_attemptSkew{0}; + Aws::Utils::DateTime m_timeRequestSent; + Aws::Utils::DateTime m_timeResponseReceived; + size_t m_retryCount; Aws::Vector m_monitoringContexts; diff --git a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h index b60c808d9f80..2300276c0e76 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/AwsSmithyClientBase.h @@ -58,6 +58,11 @@ namespace Aws } class AmazonWebServiceRequest; + + namespace Internal + { + class ClientSkew; + } } namespace Aws @@ -222,7 +227,8 @@ namespace client virtual ResolveEndpointOutcome ResolveEndpoint(const Aws::Endpoint::EndpointParameters& endpointParameters, EndpointUpdateCallback&& epCallback) const = 0; virtual SelectAuthSchemeOptionOutcome SelectAuthSchemeOption(const AwsSmithyClientAsyncRequestContext& ctx) const = 0; virtual SigningOutcome SignHttpRequest(std::shared_ptr httpRequest, const AwsSmithyClientAsyncRequestContext& ctx) const = 0; - virtual bool AdjustClockSkew(HttpResponseOutcome& outcome, const AuthSchemeOption& authSchemeOption) const = 0; + bool AdjustClockSkew(HttpResponseOutcome& outcome, const AwsSmithyClientAsyncRequestContext& ctx) const; + void RecordClockSkew(const Aws::Http::HttpResponse& response, const AwsSmithyClientAsyncRequestContext& ctx) const; virtual IdentityOutcome ResolveIdentity(const AwsSmithyClientAsyncRequestContext& ctx) const = 0; virtual GetContextEndpointParametersOutcome GetContextEndpointParameters(const AwsSmithyClientAsyncRequestContext& ctx) const = 0; AwsSmithyClientBase::ResolveEndpointOutcome ResolveEndpointFromRequest( @@ -241,6 +247,7 @@ namespace client std::shared_ptr m_errorMarshaller; Aws::Vector> m_interceptors{}; std::shared_ptr m_userAgentInterceptor; + mutable std::shared_ptr m_clientSkew; private: void UpdateAuthSchemeFromEndpoint(const Aws::Endpoint::AWSEndpoint& endpoint, AuthSchemeOption& authscheme) const; diff --git a/src/aws-cpp-sdk-core/include/smithy/client/common/AwsSmithyRequestSigning.h b/src/aws-cpp-sdk-core/include/smithy/client/common/AwsSmithyRequestSigning.h index 682646a6afd6..3ed803c40658 100644 --- a/src/aws-cpp-sdk-core/include/smithy/client/common/AwsSmithyRequestSigning.h +++ b/src/aws-cpp-sdk-core/include/smithy/client/common/AwsSmithyRequestSigning.h @@ -26,10 +26,6 @@ namespace smithy { static const char AWS_SMITHY_CLIENT_SIGNING_TAG[] = "AwsClientRequestSigning"; - //4 Minutes - static const std::chrono::milliseconds TIME_DIFF_MAX = std::chrono::minutes(4); - //-4 Minutes - static const std::chrono::milliseconds TIME_DIFF_MIN = std::chrono::minutes(-4); template class AwsClientRequestSigning @@ -142,28 +138,6 @@ namespace smithy return {authScheme}; } - static bool AdjustClockSkew(HttpResponseOutcome& outcome, const AuthSchemeOption& authSchemeOption, - const Aws::UnorderedMap& authSchemes) - { - assert(!outcome.IsSuccess()); - AWS_LOGSTREAM_WARN(AWS_SMITHY_CLIENT_SIGNING_TAG, "If the signature check failed. This could be because of a time skew. Attempting to adjust the signer."); - - using DateTime = Aws::Utils::DateTime; - DateTime serverTime = smithy::client::Utils::GetServerTimeFromError(outcome.GetError()); - - auto authSchemeOutcome = ResolveAuthScheme(authSchemeOption, authSchemes); - if (!authSchemeOutcome.IsSuccess()) - { - return false; - } - - ClockSkewVisitor visitor(outcome, serverTime, authSchemeOption); - AuthSchemesVariantT authScheme = authSchemeOutcome.GetResult().value(); - authScheme.Visit(visitor); - - return visitor.m_resultShouldWait; - } - protected: struct IdentityVisitor @@ -360,61 +334,5 @@ namespace smithy return std::move(*visitor.result); } - struct ClockSkewVisitor - { - using DateTime = Aws::Utils::DateTime; - using DateFormat = Aws::Utils::DateFormat; - using ClientError = Aws::Client::AWSError; - - ClockSkewVisitor(HttpResponseOutcome& outcome, const DateTime& serverTime, const AuthSchemeOption& targetAuthSchemeOption) - : m_outcome(outcome), m_serverTime(serverTime), m_targetAuthSchemeOption(targetAuthSchemeOption) - { - } - - bool m_resultShouldWait = false; - HttpResponseOutcome& m_outcome; - const Aws::Utils::DateTime& m_serverTime; - const AuthSchemeOption& m_targetAuthSchemeOption; - - template - void operator()(AuthSchemeAlternativeT& authScheme) - { - // Auth Scheme Variant alternative contains the requested auth option - assert(strcmp(authScheme.schemeId, m_targetAuthSchemeOption.schemeId) == 0); - - using IdentityT = typename std::remove_reference::type::IdentityT; - using Signer = AwsSignerBase; - - std::shared_ptr signer = authScheme.signer(); - if (!signer) - { - AWS_LOGSTREAM_ERROR(AWS_SMITHY_CLIENT_SIGNING_TAG, "Failed to adjust signing clock skew. Signer is null."); - return; - } - - const auto signingTimestamp = signer->GetSigningTimestamp(); - if (!m_serverTime.WasParseSuccessful() || m_serverTime == DateTime()) - { - AWS_LOGSTREAM_DEBUG(AWS_SMITHY_CLIENT_SIGNING_TAG, "Date header was not found in the response, can't attempt to detect clock skew"); - return; - } - - AWS_LOGSTREAM_DEBUG(AWS_SMITHY_CLIENT_SIGNING_TAG, "Server time is " << m_serverTime.ToGmtString(DateFormat::RFC822) << ", while client time is " << DateTime::Now().ToGmtString(DateFormat::RFC822)); - auto diff = DateTime::Diff(m_serverTime, signingTimestamp); - //only try again if clock skew was the cause of the error. - if (diff >= TIME_DIFF_MAX || diff <= TIME_DIFF_MIN) - { - diff = DateTime::Diff(m_serverTime, DateTime::Now()); - AWS_LOGSTREAM_INFO(AWS_SMITHY_CLIENT_SIGNING_TAG, "Computed time difference as " << diff.count() << " milliseconds. Adjusting signer with the skew."); - signer->SetClockSkew(diff); - ClientError newError(m_outcome.GetError()); - newError.SetRetryableType(Aws::Client::RetryableType::RETRYABLE); - - m_outcome = std::move(newError); - m_resultShouldWait = true; - } - } - }; - }; } \ No newline at end of file diff --git a/src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SigV4aSigner.h b/src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SigV4aSigner.h index 0e93d7e6a1a3..cba522d93992 100644 --- a/src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SigV4aSigner.h +++ b/src/aws-cpp-sdk-core/include/smithy/identity/signer/built-in/SigV4aSigner.h @@ -97,7 +97,8 @@ namespace smithy { awsSigningConfig.SetSignatureType(signatureType); awsSigningConfig.SetRegion(serviceName.c_str()); awsSigningConfig.SetService(region.c_str()); - awsSigningConfig.SetSigningTimepoint(GetSigningTimestamp().UnderlyingTimestamp()); + const Aws::Utils::DateTime sigV4aSigningTime = request.GetSigningTimestampOverride() ? request.GetSigningTimestampOverride().value() : GetSigningTimestamp(); + awsSigningConfig.SetSigningTimepoint(sigV4aSigningTime.UnderlyingTimestamp()); awsSigningConfig.SetUseDoubleUriEncode(m_urlEscape); awsSigningConfig.SetShouldNormalizeUriPath(true); awsSigningConfig.SetOmitSessionToken(false); diff --git a/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthEventStreamV4Signer.cpp b/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthEventStreamV4Signer.cpp index cf39123eebf7..2d938a7daf06 100644 --- a/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthEventStreamV4Signer.cpp +++ b/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthEventStreamV4Signer.cpp @@ -79,7 +79,7 @@ bool AWSAuthEventStreamV4Signer::SignRequestWithCreds(Http::HttpRequest& request request.SetHeaderValue(Aws::Auth::AWSAuthHelper::X_AMZ_CONTENT_SHA256, EVENT_STREAM_CONTENT_SHA256); //calculate date header to use in internal signature (this also goes into date header). - DateTime now = GetSigningTimestamp(); + DateTime now = request.GetSigningTimestampOverride() ? request.GetSigningTimestampOverride().value() : GetSigningTimestamp(); Aws::String dateHeaderValue = now.ToGmtString(DateFormat::ISO_8601_BASIC); request.SetHeaderValue(AWS_DATE_HEADER, dateHeaderValue); diff --git a/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp b/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp index 7ebf65689524..841b12822ff9 100644 --- a/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp +++ b/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp @@ -96,7 +96,8 @@ bool AWSAuthV4Signer::SignRequestWithSigV4a(Aws::Http::HttpRequest& request, con awsSigningConfig.SetSignatureType(signatureType); awsSigningConfig.SetRegion(region); awsSigningConfig.SetService(serviceName); - awsSigningConfig.SetSigningTimepoint(GetSigningTimestamp().UnderlyingTimestamp()); + const DateTime sigV4aSigningTime = request.GetSigningTimestampOverride() ? request.GetSigningTimestampOverride().value() : GetSigningTimestamp(); + awsSigningConfig.SetSigningTimepoint(sigV4aSigningTime.UnderlyingTimestamp()); awsSigningConfig.SetUseDoubleUriEncode(m_urlEscapePath); awsSigningConfig.SetShouldNormalizeUriPath(true); awsSigningConfig.SetOmitSessionToken(false); @@ -254,7 +255,7 @@ bool AWSAuthV4Signer::SignRequestWithCreds(Aws::Http::HttpRequest& request, cons } //calculate date header to use in internal signature (this also goes into date header). - DateTime now = GetSigningTimestamp(); + DateTime now = request.GetSigningTimestampOverride() ? request.GetSigningTimestampOverride().value() : GetSigningTimestamp(); Aws::String dateHeaderValue = now.ToGmtString(DateFormat::ISO_8601_BASIC); request.SetHeaderValue(AWS_DATE_HEADER, dateHeaderValue); diff --git a/src/aws-cpp-sdk-core/source/client/AWSClient.cpp b/src/aws-cpp-sdk-core/source/client/AWSClient.cpp index c0253901d1ed..0c1c40a250f0 100644 --- a/src/aws-cpp-sdk-core/source/client/AWSClient.cpp +++ b/src/aws-cpp-sdk-core/source/client/AWSClient.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -68,11 +69,6 @@ static const char AWS_CLIENT_LOG_TAG[] = "AWSClient"; static const char AWS_LAMBDA_FUNCTION_NAME[] = "AWS_LAMBDA_FUNCTION_NAME"; static const char X_AMZN_TRACE_ID[] = "_X_AMZN_TRACE_ID"; -//4 Minutes -static const std::chrono::milliseconds TIME_DIFF_MAX = std::chrono::minutes(4); -//-4 Minutes -static const std::chrono::milliseconds TIME_DIFF_MIN = std::chrono::minutes(-4); - CoreErrors AWSClient::GuessBodylessErrorType(Aws::Http::HttpResponseCode responseCode) { switch (responseCode) @@ -138,6 +134,7 @@ AWSClient::AWSClient(const Aws::Client::ClientConfiguration& configuration, m_hash(Aws::Utils::Crypto::CreateMD5Implementation()), m_requestTimeoutMs(configuration.requestTimeoutMs), m_enableClockSkewAdjustment(configuration.enableClockSkewAdjustment), + m_clientSkew(Aws::MakeShared(AWS_CLIENT_LOG_TAG, std::chrono::milliseconds(0))), m_requestCompressionConfig(configuration.requestCompressionConfig), m_userAgentInterceptor{Aws::MakeShared(AWS_CLIENT_LOG_TAG, configuration, m_retryStrategy->GetStrategyName(), m_serviceName)}, m_interceptors{Aws::MakeShared(AWS_CLIENT_LOG_TAG), Aws::MakeShared(AWS_CLIENT_LOG_TAG, @@ -169,6 +166,7 @@ AWSClient::AWSClient(const Aws::Client::ClientConfiguration& configuration, m_hash(Aws::Utils::Crypto::CreateMD5Implementation()), m_requestTimeoutMs(configuration.requestTimeoutMs), m_enableClockSkewAdjustment(configuration.enableClockSkewAdjustment), + m_clientSkew(Aws::MakeShared(AWS_CLIENT_LOG_TAG, std::chrono::milliseconds(0))), m_requestCompressionConfig(configuration.requestCompressionConfig), m_userAgentInterceptor{Aws::MakeShared(AWS_CLIENT_LOG_TAG, configuration, m_retryStrategy->GetStrategyName(), m_serviceName)}, m_interceptors{Aws::MakeShared(AWS_CLIENT_LOG_TAG, configuration), Aws::MakeShared(AWS_CLIENT_LOG_TAG, @@ -228,30 +226,16 @@ static DateTime GetServerTimeFromError(const AWSError error) } } -bool AWSClient::AdjustClockSkew(HttpResponseOutcome& outcome, const char* signerName) const +bool AWSClient::AdjustClockSkew(HttpResponseOutcome& outcome, const Aws::Utils::DateTime& timeRequestSent, const Aws::Utils::DateTime& timeResponseReceived, std::chrono::milliseconds attemptSkew) const { if (m_enableClockSkewAdjustment) { - auto signer = GetSignerByName(signerName); - //detect clock skew and try to correct. - AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "If the signature check failed. This could be because of a time skew. Attempting to adjust the signer."); - - DateTime serverTime = GetServerTimeFromError(outcome.GetError()); - const auto signingTimestamp = signer->GetSigningTimestamp(); - if (!serverTime.WasParseSuccessful() || serverTime == DateTime()) - { - AWS_LOGSTREAM_DEBUG(AWS_CLIENT_LOG_TAG, "Date header was not found in the response, can't attempt to detect clock skew"); - return false; - } - - AWS_LOGSTREAM_DEBUG(AWS_CLIENT_LOG_TAG, "Server time is " << serverTime.ToGmtString(DateFormat::RFC822) << ", while client time is " << DateTime::Now().ToGmtString(DateFormat::RFC822)); - auto diff = DateTime::Diff(serverTime, signingTimestamp); - //only try again if clock skew was the cause of the error. - if (diff >= TIME_DIFF_MAX || diff <= TIME_DIFF_MIN) + const auto measurement = Aws::Internal::MakeClockSkewMeasurement(outcome.GetError().GetResponseHeaders(), timeRequestSent, timeResponseReceived); + const auto adjustment = m_clientSkew->EvaluateFailure(measurement, attemptSkew); + // Force a retry only when the error is a clock-skew error code and the skew exceeds the threshold. + if (Aws::Internal::IsClockSkewError(outcome.GetError()) && adjustment.skewExceedsThreshold) { - diff = DateTime::Diff(serverTime, DateTime::Now()); - AWS_LOGSTREAM_INFO(AWS_CLIENT_LOG_TAG, "Computed time difference as " << diff.count() << " milliseconds. Adjusting signer with the skew."); - signer->SetClockSkew(diff); + AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "Signature check likely failed due to clock skew; adjusting the signing timestamp and retrying."); AWSError newError( outcome.GetError().GetErrorType(), outcome.GetError().GetExceptionName(), outcome.GetError().GetMessage(), true); newError.SetResponseHeaders(outcome.GetError().GetResponseHeaders()); @@ -290,6 +274,8 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, httpRequest->SetHeaderValue(Http::SDK_REQUEST_HEADER, requestInfo); AppendRecursionDetectionHeader(httpRequest); + // AttemptSkew: seeded from the client-level skew, updated after each attempt. + std::chrono::milliseconds attemptSkew = m_clientSkew->Load(); for (long retries = 0;; retries++) { if(!m_retryStrategy->HasSendToken()) @@ -303,7 +289,10 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, httpRequest->SetEventStreamRequest(request.IsEventStreamRequest()); httpRequest->SetHasEventStreamResponse(request.HasEventStreamResponse()); + const DateTime attemptSentTime = DateTime::Now(); + httpRequest->SetSigningTimestampOverride(attemptSentTime + attemptSkew); outcome = AttemptOneRequest(httpRequest, request, signerName, signerRegion, signerServiceNameOverride); + const DateTime timeResponseReceived = DateTime::Now(); outcome.SetRetryCount(retries); if (retries == 0) { @@ -320,6 +309,10 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, {TracingUtils::SMITHY_SERVICE_DIMENSION, this->GetServiceClientName()}}); if (outcome.IsSuccess()) { + if (m_enableClockSkewAdjustment && outcome.GetResult()) + { + m_clientSkew->RecordResponse(Aws::Internal::MakeClockSkewMeasurement(outcome.GetResult()->GetHeaders(), attemptSentTime, timeResponseReceived)); + } Aws::Monitoring::OnRequestSucceeded(this->GetServiceClientName(), request.GetServiceRequestName(), httpRequest, outcome, coreMetrics, contexts); AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Request successful returning."); break; @@ -362,7 +355,8 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, {{TracingUtils::SMITHY_METHOD_DIMENSION, request.GetServiceRequestName()},{TracingUtils::SMITHY_SERVICE_DIMENSION, this->GetServiceClientName()}}); //AdjustClockSkew returns true means clock skew was the problem and skew was adjusted, false otherwise. //sleep if clock skew and region was NOT the problem. AdjustClockSkew may update error inside outcome. - bool shouldSleep = !AdjustClockSkew(outcome, signerName) && !retryWithCorrectRegion; + bool shouldSleep = !AdjustClockSkew(outcome, attemptSentTime, timeResponseReceived, attemptSkew) && !retryWithCorrectRegion; + attemptSkew = m_clientSkew->Load(); if (!retryWithCorrectRegion && !m_retryStrategy->ShouldRetry(outcome.GetError(), retries)) { @@ -464,6 +458,8 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, httpRequest->SetHeaderValue(Http::SDK_REQUEST_HEADER, requestInfo); AppendRecursionDetectionHeader(httpRequest); + // AttemptSkew: seeded from the client-level skew, updated after each attempt. + std::chrono::milliseconds attemptSkew = m_clientSkew->Load(); for (long retries = 0;; retries++) { if(!m_retryStrategy->HasSendToken()) @@ -474,7 +470,10 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, false/*retryable*/)); }; + const DateTime attemptSentTime = DateTime::Now(); + httpRequest->SetSigningTimestampOverride(attemptSentTime + attemptSkew); outcome = AttemptOneRequest(httpRequest, signerName, requestName, signerRegion, signerServiceNameOverride); + const DateTime timeResponseReceived = DateTime::Now(); outcome.SetRetryCount(retries); if (retries == 0) { @@ -490,6 +489,10 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, {{TracingUtils::SMITHY_METHOD_DIMENSION, requestName},{TracingUtils::SMITHY_SERVICE_DIMENSION, this->GetServiceClientName()}}); if (outcome.IsSuccess()) { + if (m_enableClockSkewAdjustment && outcome.GetResult()) + { + m_clientSkew->RecordResponse(Aws::Internal::MakeClockSkewMeasurement(outcome.GetResult()->GetHeaders(), attemptSentTime, timeResponseReceived)); + } Aws::Monitoring::OnRequestSucceeded(this->GetServiceClientName(), requestName, httpRequest, outcome, coreMetrics, contexts); AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Request successful returning."); break; @@ -532,7 +535,8 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, {{TracingUtils::SMITHY_METHOD_DIMENSION, requestName},{TracingUtils::SMITHY_SERVICE_DIMENSION, this->GetServiceClientName()}}); //AdjustClockSkew returns true means clock skew was the problem and skew was adjusted, false otherwise. //sleep if clock skew and region was NOT the problem. AdjustClockSkew may update error inside outcome. - bool shouldSleep = !AdjustClockSkew(outcome, signerName) && !retryWithCorrectRegion; + bool shouldSleep = !AdjustClockSkew(outcome, attemptSentTime, timeResponseReceived, attemptSkew) && !retryWithCorrectRegion; + attemptSkew = m_clientSkew->Load(); if (!retryWithCorrectRegion && !m_retryStrategy->ShouldRetry(outcome.GetError(), retries)) { diff --git a/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp b/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp index 78f3168866c6..ac41987427f7 100644 --- a/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp +++ b/src/aws-cpp-sdk-core/source/client/ClientConfiguration.cpp @@ -39,6 +39,8 @@ static const char* REQUEST_MIN_COMPRESSION_SIZE_BYTES_CONFIG_VAR = "request_min_ static const char* AWS_EXECUTION_ENV = "AWS_EXECUTION_ENV"; static const char* DISABLE_IMDSV1_CONFIG_VAR = "AWS_EC2_METADATA_V1_DISABLED"; static const char* DISABLE_IMDSV1_ENV_VAR = "ec2_metadata_v1_disabled"; +static const char* DISABLE_CLOCK_SKEW_CORRECTION_ENV_VAR = "AWS_DISABLE_CLOCK_SKEW_CORRECTION"; +static const char* DISABLE_CLOCK_SKEW_CORRECTION_CONFIG_VAR = "disable_clock_skew_correction"; static const char* AWS_ACCOUNT_ID_ENDPOINT_MODE_ENVIRONMENT_VARIABLE = "AWS_ACCOUNT_ID_ENDPOINT_MODE"; static const char* AWS_ACCOUNT_ID_ENDPOINT_MODE_CONFIG_FILE_OPTION = "account_id_endpoint_mode"; static const char* AWS_METADATA_SERVICE_TIMEOUT_ENV_VAR = "AWS_METADATA_SERVICE_TIMEOUT"; @@ -336,6 +338,16 @@ void setConfigFromEnvOrProfile(ClientConfiguration &config) config.credentialProviderConfig.imdsConfig.disableImdsV1 = true; } + // Knob is a "disable" flag (AWS ..._DISABLED convention); the config field is "enable" and defaults on. + const bool disableClockSkewCorrection = ClientConfiguration::LoadConfigFromEnvOrProfile(DISABLE_CLOCK_SKEW_CORRECTION_ENV_VAR, + config.profileName, + DISABLE_CLOCK_SKEW_CORRECTION_CONFIG_VAR, + {"true", "false"}, + "false") == "true"; + if (disableClockSkewCorrection) { + config.enableClockSkewAdjustment = false; + } + // accountId is intentionally not set here: AWS_ACCOUNT_ID env variable may not match the provided credentials. // it must be set by an auth provider / identity resolver or by an SDK user. config.accountIdEndpointMode = ClientConfiguration::LoadConfigFromEnvOrProfile(AWS_ACCOUNT_ID_ENDPOINT_MODE_ENVIRONMENT_VARIABLE, diff --git a/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp b/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp index d925a2efd68d..ace89ff4ab80 100644 --- a/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp +++ b/src/aws-cpp-sdk-core/source/smithy/client/AwsSmithyClientBase.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -69,6 +70,7 @@ void createFromFactoriesIfPresent(T& entity, std::function& factory) { void AwsSmithyClientBase::baseInit() { AWS_CHECK_PTR(AWS_SMITHY_CLIENT_LOG, m_clientConfig); + m_clientSkew = Aws::MakeShared(AWS_SMITHY_CLIENT_LOG, std::chrono::milliseconds(0)); createFromFactories(m_clientConfig->retryStrategy, m_clientConfig->configFactories.retryStrategyCreateFn); createFromFactories(m_clientConfig->executor, m_clientConfig->configFactories.executorCreateFn); createFromFactories(m_clientConfig->writeRateLimiter, m_clientConfig->configFactories.writeRateLimiterCreateFn); @@ -85,6 +87,7 @@ void AwsSmithyClientBase::baseInit() { void AwsSmithyClientBase::baseCopyInit() { AWS_CHECK_PTR(AWS_SMITHY_CLIENT_LOG, m_clientConfig); + m_clientSkew = Aws::MakeShared(AWS_SMITHY_CLIENT_LOG, std::chrono::milliseconds(0)); createFromFactoriesIfPresent(m_clientConfig->retryStrategy, m_clientConfig->configFactories.retryStrategyCreateFn); createFromFactoriesIfPresent(m_clientConfig->executor, m_clientConfig->configFactories.executorCreateFn); createFromFactoriesIfPresent(m_clientConfig->writeRateLimiter, m_clientConfig->configFactories.writeRateLimiterCreateFn); @@ -118,6 +121,7 @@ void AwsSmithyClientBase::baseCopyAssign(const AwsSmithyClientBase& other, } void AwsSmithyClientBase::baseMoveAssign(AwsSmithyClientBase&& other) { + m_clientSkew = Aws::MakeShared(AWS_SMITHY_CLIENT_LOG, std::chrono::milliseconds(0)); m_serviceName = std::move(other.m_serviceName); m_serviceUserAgentName = std::move(other.m_serviceUserAgentName); m_httpClient = std::move(other.m_httpClient); @@ -305,6 +309,7 @@ void AwsSmithyClientBase::MakeRequestAsync(Aws::AmazonWebServiceRequest const* c return; } pRequestCtx->m_requestInfo.attempt = 1; + pRequestCtx->m_attemptSkew = m_clientSkew->Load(); pRequestCtx->m_requestInfo.maxAttempts = Aws::Environment::GetEnv("AWS_NEW_RETRIES_2026") == "true" ? m_clientConfig->retryStrategy->GetMaxAttempts() @@ -410,6 +415,8 @@ void AwsSmithyClientBase::AttemptOneRequestAsync(std::shared_ptrm_timeRequestSent = Aws::Utils::DateTime::Now(); + pRequestCtx->m_httpRequest->SetSigningTimestampOverride(pRequestCtx->m_timeRequestSent + pRequestCtx->m_attemptSkew); SigningOutcome signingOutcome = TracingUtils::MakeCallWithTiming([&]() -> SigningOutcome { return this->SignHttpRequest(pRequestCtx->m_httpRequest, *pRequestCtx); }, @@ -488,11 +495,41 @@ void AwsSmithyClientBase::AttemptOneRequestAsync(std::shared_ptrenableClockSkewAdjustment) + { + m_clientSkew->RecordResponse(Aws::Internal::MakeClockSkewMeasurement(response.GetHeaders(), ctx.m_timeRequestSent, ctx.m_timeResponseReceived)); + } +} + +bool AwsSmithyClientBase::AdjustClockSkew(HttpResponseOutcome& outcome, const AwsSmithyClientAsyncRequestContext& ctx) const +{ + if (!m_clientConfig->enableClockSkewAdjustment) + { + return false; + } + const auto measurement = Aws::Internal::MakeClockSkewMeasurement(outcome.GetError().GetResponseHeaders(), ctx.m_timeRequestSent, ctx.m_timeResponseReceived); + const auto adjustment = m_clientSkew->EvaluateFailure(measurement, ctx.m_attemptSkew); + // Force a retry only when the error is a clock-skew error code and the skew exceeds the threshold. + if (Aws::Internal::IsClockSkewError(outcome.GetError()) && adjustment.skewExceedsThreshold) + { + AWS_LOGSTREAM_WARN(AWS_SMITHY_CLIENT_LOG, "Signature check likely failed due to clock skew; adjusting the signing timestamp and retrying."); + auto newError = outcome.GetError(); + newError.SetRetryableType(Aws::Client::RetryableType::RETRYABLE); + outcome = std::move(newError); + return true; + } + return false; +} + void AwsSmithyClientBase::HandleAsyncReply(std::shared_ptr pRequestCtx, std::shared_ptr httpResponse) const { assert(pRequestCtx && httpResponse); + pRequestCtx->m_timeResponseReceived = Aws::Utils::DateTime::Now(); + pRequestCtx->m_interceptorContext->SetTransmitResponse(httpResponse); for (const auto& interceptor : m_interceptors) { @@ -546,6 +583,10 @@ void AwsSmithyClientBase::HandleAsyncReply(std::shared_ptrGetServiceClientName()}}); if (outcome.IsSuccess()) { + if (outcome.GetResult()) + { + RecordClockSkew(*outcome.GetResult(), *pRequestCtx); + } Aws::Monitoring::OnRequestSucceeded(this->GetServiceClientName(), pRequestCtx->m_requestName, pRequestCtx->m_httpRequest, @@ -612,13 +653,9 @@ void AwsSmithyClientBase::HandleAsyncReply(std::shared_ptrtelemetryProvider->getMeter(this->GetServiceClientName(), {}), {{TracingUtils::SMITHY_METHOD_DIMENSION, pRequestCtx->m_requestName}, {TracingUtils::SMITHY_SERVICE_DIMENSION, this->GetServiceClientName()}}); - bool shouldSleep = !retryWithCorrectRegion; - if (m_clientConfig->enableClockSkewAdjustment) - { - // AdjustClockSkew returns true means clock skew was the problem and skew was adjusted, false otherwise. - // sleep if clock skew and region was NOT the problem. AdjustClockSkew may update error inside outcome. - shouldSleep |= !this->AdjustClockSkew(outcome, pRequestCtx->m_authSchemeOption); - } + // Sleep only if neither clock skew nor region caused the failure. AdjustClockSkew self-gates on the disable knob. + bool shouldSleep = !this->AdjustClockSkew(outcome, *pRequestCtx) && !retryWithCorrectRegion; + pRequestCtx->m_attemptSkew = m_clientSkew->Load(); if (!retryWithCorrectRegion && !m_clientConfig->retryStrategy->ShouldRetry(outcome.GetError(), static_cast(pRequestCtx->m_retryCount))) { diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp index 11695b9f7602..56e20c5e99cc 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp @@ -69,7 +69,7 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite protected: std::shared_ptr mockHttpClient; std::shared_ptr mockHttpClientFactory; - Aws::UniquePtr client; + Aws::UniquePtr client; virtual void SetUp() { @@ -84,7 +84,7 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config); + client = Aws::MakeUnique(ALLOCATION_TAG, config); } void TearDown() @@ -167,7 +167,7 @@ class XMLClientTestSuite : public AWSClientTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config, Aws::MakeShared("xmlErrorMarshaller")); + client = Aws::MakeUnique(ALLOCATION_TAG, config, Aws::MakeShared("xmlErrorMarshaller")); } }; @@ -215,6 +215,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewOutsideAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -227,6 +228,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewWithinAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::minutes(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 minutes + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code, but skew is below threshold AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); auto outcome = client->MakeRequest(request); @@ -239,6 +241,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewConsecutiveRequests) // first request should set the skew offset and retry, but following requests should not HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -248,14 +251,14 @@ TEST_F(AWSClientTestSuite, TestClockSkewConsecutiveRequests) QueueMockResponse(HttpResponseCode::UNAUTHORIZED, responseHeaders); outcome = client->MakeRequest(request); - ASSERT_FALSE(outcome.IsSuccess()); // should _not_ attempt to adjust clock skew and retry the request. + ASSERT_FALSE(outcome.IsSuccess()); // skew already applied; the offset now matches, so no retry. ASSERT_EQ(HttpResponseCode::UNAUTHORIZED, outcome.GetError().GetResponseCode()); ASSERT_STREQ("127.0.0.1", outcome.GetError().GetRemoteHostIpAddress().c_str()); ASSERT_EQ(0, client->GetRequestAttemptedRetries()); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); outcome = client->MakeRequest(request); - ASSERT_FALSE(outcome.IsSuccess()); // should _not_ attempt to adjust clock skew and retry the request. + ASSERT_FALSE(outcome.IsSuccess()); // skew already applied; the offset now matches, so no retry. ASSERT_EQ(HttpResponseCode::FORBIDDEN, outcome.GetError().GetResponseCode()); ASSERT_STREQ("127.0.0.1", outcome.GetError().GetRemoteHostIpAddress().c_str()); ASSERT_EQ(0, client->GetRequestAttemptedRetries()); @@ -270,6 +273,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make an initial request so that a skew adjustment is set HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -280,6 +284,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock skewed even further responseHeaders.clear(); responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 hours + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); outcome = client->MakeRequest(request); @@ -289,6 +294,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock in sync with the server responseHeaders.clear(); responseHeaders.emplace("Date", DateTime::Now().ToGmtString(DateFormat::RFC822)); // server is in sync with client + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); outcome = client->MakeRequest(request); @@ -300,10 +306,10 @@ TEST_F(AWSClientTestSuite, TestRetryHeaders) { // The first server time is ahead of us by 1 hour. DateTime serverTime1 = DateTime::Now() + std::chrono::hours(1); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("RequestTimeTooSkewed"))}); // The second server time is ahead of us by 2 hour. DateTime serverTime2 = DateTime::Now() + std::chrono::hours(2); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("RequestTimeTooSkewed"))}); // The third server time is ahead of us by 3 hour. DateTime serverTime3 = DateTime::Now() + std::chrono::hours(3); QueueMockResponse(HttpResponseCode::OK, HeaderValueCollection{std::make_pair("Date", serverTime3.ToGmtString(DateFormat::RFC822))}); @@ -345,6 +351,7 @@ TEST_F(AWSClientTestSuite, TestRetryURIs) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); URI uri("http://www.uri.com/path with space/to/res"); diff --git a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp index bcfdc6e41284..5180345e5d70 100644 --- a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp @@ -188,7 +188,7 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite protected: std::shared_ptr mockHttpClient; std::shared_ptr mockHttpClientFactory; - Aws::UniquePtr client; + Aws::UniquePtr client; void SetUp() { @@ -203,7 +203,7 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config); + client = Aws::MakeUnique(ALLOCATION_TAG, config); Aws::Monitoring::CleanupMonitoring(); std::vector factoryFunctions; @@ -249,6 +249,7 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); // BAD_REQUEST is not retryable, but since this is triggered by clock skew, it's set to mandatory retryable. @@ -271,6 +272,7 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; + responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); diff --git a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h index cff1a22c630f..07f5e6a5910e 100644 --- a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h +++ b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -246,3 +247,28 @@ class MockAWSClientWithStandardRetryStrategy : Aws::Client::AWSClient return err; } }; + +// A MockAWSClient whose error responses carry a service error code via the x-amzn-errortype header +// (the header a JSON error marshaller reads), so a clock-skew retry can be exercised end to end. +class ClockSkewMockAWSClient : public MockAWSClient +{ +public: + using MockAWSClient::MockAWSClient; + +protected: + Aws::Client::AWSError BuildAWSError(const std::shared_ptr& response) const override + { + const auto& headers = response->GetHeaders(); + const auto it = headers.find("x-amzn-errortype"); + if (it == headers.end()) + { + return MockAWSClient::BuildAWSError(response); + } + Aws::Client::AWSError error( + Aws::Client::CoreErrorsMapper::GetErrorForName(it->second.c_str()).GetErrorType(), it->second, "", false); + error.SetResponseHeaders(headers); + error.SetResponseCode(response->GetResponseCode()); + error.SetRemoteHostIpAddress(response->GetOriginatingRequest().GetResolvedRemoteHost()); + return error; + } +}; From e5b808e2e07362e9bb9225d2af78bcaed99941a8 Mon Sep 17 00:00:00 2001 From: kai lin Date: Wed, 2 Sep 2026 10:49:24 -0400 Subject: [PATCH 2/4] remove rule of 5 and change the test to directly use the error --- .../include/aws/core/internal/ClockSkew.h | 17 ------- .../aws/client/AWSClientTest.cpp | 22 ++++---- .../monitoring/MonitoringTest.cpp | 7 +-- .../testing/mocks/aws/client/MockAWSClient.h | 50 ++++++++++++------- 4 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h b/src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h index 4b0dda5c4419..2f921b83bac4 100644 --- a/src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h +++ b/src/aws-cpp-sdk-core/include/aws/core/internal/ClockSkew.h @@ -110,23 +110,6 @@ namespace Aws public: explicit ClientSkew(std::chrono::milliseconds initial) : m_skew(initial) {} - ClientSkew(const ClientSkew& other) : m_skew(other.m_skew.load()) {} - ClientSkew(ClientSkew&& other) noexcept : m_skew(other.m_skew.load()) {} - ClientSkew& operator=(const ClientSkew& other) - { - if (this != &other) - { - m_skew = other.m_skew.load(); - } - return *this; - } - ClientSkew& operator=(ClientSkew&& other) noexcept - { - m_skew = other.m_skew.load(); - return *this; - } - ~ClientSkew() = default; - std::chrono::milliseconds Load() const { return m_skew.load(); } // Runs on every response; a surviving candidate is stored, so a stale value self-heals. diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp index 56e20c5e99cc..57be3bbc7b4a 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp @@ -102,7 +102,9 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers) { - QueueMockResponse(code, headers, "ss"); + // JSON-protocol error responses carry a JSON body; the JSON error marshaller only reads the + // error type from the x-amzn-errortype header when the body parses, so default to empty JSON. + QueueMockResponse(code, headers, "{}"); } void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers, const Aws::String& body) @@ -215,7 +217,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewOutsideAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -228,7 +230,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewWithinAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::minutes(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 minutes - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code, but skew is below threshold + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code, but skew is below threshold AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); auto outcome = client->MakeRequest(request); @@ -241,7 +243,7 @@ TEST_F(AWSClientTestSuite, TestClockSkewConsecutiveRequests) // first request should set the skew offset and retry, but following requests should not HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -273,7 +275,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make an initial request so that a skew adjustment is set HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); @@ -284,7 +286,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock skewed even further responseHeaders.clear(); responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 hours - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); outcome = client->MakeRequest(request); @@ -294,7 +296,7 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock in sync with the server responseHeaders.clear(); responseHeaders.emplace("Date", DateTime::Now().ToGmtString(DateFormat::RFC822)); // server is in sync with client - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); outcome = client->MakeRequest(request); @@ -306,10 +308,10 @@ TEST_F(AWSClientTestSuite, TestRetryHeaders) { // The first server time is ahead of us by 1 hour. DateTime serverTime1 = DateTime::Now() + std::chrono::hours(1); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("RequestTimeTooSkewed"))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("SignatureDoesNotMatch"))}); // The second server time is ahead of us by 2 hour. DateTime serverTime2 = DateTime::Now() + std::chrono::hours(2); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("RequestTimeTooSkewed"))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("SignatureDoesNotMatch"))}); // The third server time is ahead of us by 3 hour. DateTime serverTime3 = DateTime::Now() + std::chrono::hours(3); QueueMockResponse(HttpResponseCode::OK, HeaderValueCollection{std::make_pair("Date", serverTime3.ToGmtString(DateFormat::RFC822))}); @@ -351,7 +353,7 @@ TEST_F(AWSClientTestSuite, TestRetryURIs) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); URI uri("http://www.uri.com/path with space/to/res"); diff --git a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp index 5180345e5d70..ac53990a46f2 100644 --- a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp @@ -235,7 +235,8 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite HttpMethod::HTTP_GET, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); auto httpResponse = Aws::MakeShared(ALLOCATION_TAG, httpRequest); httpResponse->SetResponseCode(code); - httpResponse->GetResponseBody() << ""; + // JSON body so the JSON error marshaller parses it and reads the x-amzn-errortype header. + httpResponse->GetResponseBody() << "{}"; for(auto&& header : headers) { httpResponse->AddHeader(header.first, header.second); @@ -249,7 +250,7 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); // BAD_REQUEST is not retryable, but since this is triggered by clock skew, it's set to mandatory retryable. @@ -272,7 +273,7 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; - responseHeaders.emplace("x-amzn-errortype", "RequestTimeTooSkewed"); // clock-skew error code + responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); diff --git a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h index 07f5e6a5910e..73f7a2ff9963 100644 --- a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h +++ b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h @@ -5,8 +5,8 @@ #include #include +#include #include -#include #include #include #include @@ -248,27 +248,39 @@ class MockAWSClientWithStandardRetryStrategy : Aws::Client::AWSClient } }; -// A MockAWSClient whose error responses carry a service error code via the x-amzn-errortype header -// (the header a JSON error marshaller reads), so a clock-skew retry can be exercised end to end. -class ClockSkewMockAWSClient : public MockAWSClient +// JSON client for the clock-skew pipeline tests: builds errors via the real JsonErrorMarshaller (no +// bespoke BuildAWSError). Use a non-retryable clock-skew code so retries come only from clock skew. +class ClockSkewMockAWSClient : public Aws::Client::AWSJsonClient { public: - using MockAWSClient::MockAWSClient; + ClockSkewMockAWSClient(const Aws::Client::ClientConfiguration& config, + const std::shared_ptr& errorMarshaller) + : Aws::Client::AWSJsonClient(config, + Aws::MakeShared("ClockSkewMockAWSClient", + Aws::MakeShared("ClockSkewMockAWSClient", + MockAWSClient::GetMockAccessKey(), MockAWSClient::GetMockSecretAccessKey()), + "service", config.region.empty() ? Aws::Region::US_EAST_1 : config.region), + errorMarshaller), + m_countedRetryStrategy(std::static_pointer_cast(config.retryStrategy)) { } + + ClockSkewMockAWSClient(const Aws::Client::ClientConfiguration& config) + : ClockSkewMockAWSClient(config, Aws::MakeShared("ClockSkewMockAWSClient")) { } -protected: - Aws::Client::AWSError BuildAWSError(const std::shared_ptr& response) const override + Aws::Client::HttpResponseOutcome MakeRequest(const Aws::AmazonWebServiceRequest& request) { - const auto& headers = response->GetHeaders(); - const auto it = headers.find("x-amzn-errortype"); - if (it == headers.end()) - { - return MockAWSClient::BuildAWSError(response); - } - Aws::Client::AWSError error( - Aws::Client::CoreErrorsMapper::GetErrorForName(it->second.c_str()).GetErrorType(), it->second, "", false); - error.SetResponseHeaders(headers); - error.SetResponseCode(response->GetResponseCode()); - error.SetRemoteHostIpAddress(response->GetOriginatingRequest().GetResolvedRemoteHost()); - return error; + return MakeRequest(Aws::Http::URI("domain.com/something"), request); } + + Aws::Client::HttpResponseOutcome MakeRequest(const Aws::Http::URI& uri, const Aws::AmazonWebServiceRequest& request) + { + m_countedRetryStrategy->ResetAttemptedRetriesCount(); + return AttemptExhaustively(uri, request, Aws::Http::HttpMethod::HTTP_GET, Aws::Auth::SIGV4_SIGNER); + } + + long GetRequestAttemptedRetries() { return m_countedRetryStrategy->GetAttemptedRetriesCount(); } + + inline const char* GetServiceClientName() const override { return "MockAWSClient"; } + +private: + std::shared_ptr m_countedRetryStrategy; }; From 8060910e9c509297244ed8fb7336025727d3f62e Mon Sep 17 00:00:00 2001 From: kai lin Date: Wed, 2 Sep 2026 16:13:00 -0400 Subject: [PATCH 3/4] added new apis to update response headers, added a real s3 client test with clock skew --- .../aws/client/AWSClientTest.cpp | 70 +++++++++++-------- .../monitoring/MonitoringTest.cpp | 33 ++++++--- .../aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp | 33 +++++++++ .../testing/mocks/aws/client/MockAWSClient.h | 41 +---------- 4 files changed, 100 insertions(+), 77 deletions(-) diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp index 57be3bbc7b4a..a2dc3da34c98 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp @@ -69,7 +69,7 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite protected: std::shared_ptr mockHttpClient; std::shared_ptr mockHttpClientFactory; - Aws::UniquePtr client; + Aws::UniquePtr client; virtual void SetUp() { @@ -84,7 +84,7 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config); + client = Aws::MakeUnique(ALLOCATION_TAG, config); } void TearDown() @@ -102,9 +102,7 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers) { - // JSON-protocol error responses carry a JSON body; the JSON error marshaller only reads the - // error type from the x-amzn-errortype header when the body parses, so default to empty JSON. - QueueMockResponse(code, headers, "{}"); + QueueMockResponse(code, headers, "ss"); } void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers, const Aws::String& body) @@ -139,6 +137,25 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClient->AddResponseToReturn(httpResponse); } + // Stage a response whose error carries a specific service error code (via the client-error + // channel) plus an HTTP status code, remote host, and headers, so BuildAWSError yields a typed + // error with those headers/code -- enough to drive clock-skew detection end to end. + void QueueMockResponse(HttpResponseCode code, CoreErrors errorType, const HeaderValueCollection& headers) + { + auto httpRequest = CreateHttpRequest(URI("http://www.uri.com/path/to/res"), + HttpMethod::HTTP_GET, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); + httpRequest->SetResolvedRemoteHost("127.0.0.1"); + auto httpResponse = Aws::MakeShared(ALLOCATION_TAG, httpRequest); + httpResponse->SetResponseCode(code); + httpResponse->SetClientErrorType(errorType); + httpResponse->GetResponseBody() << ""; + for(auto&& header : headers) + { + httpResponse->AddHeader(header.first, header.second); + } + mockHttpClient->AddResponseToReturn(httpResponse); + } + Aws::String ExtractFromRequestInfo(const Aws::String& requestInfo, const Aws::String& key) { auto iter = requestInfo.find(key + "="); @@ -169,7 +186,7 @@ class XMLClientTestSuite : public AWSClientTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config, Aws::MakeShared("xmlErrorMarshaller")); + client = Aws::MakeUnique(ALLOCATION_TAG, config, Aws::MakeShared("xmlErrorMarshaller")); } }; @@ -217,10 +234,9 @@ TEST_F(AWSClientTestSuite, TestClockSkewOutsideAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); auto outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); @@ -230,9 +246,8 @@ TEST_F(AWSClientTestSuite, TestClockSkewWithinAcceptableRange) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::minutes(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 minutes - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code, but skew is below threshold AmazonWebServiceRequestMock request; - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); auto outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(0, client->GetRequestAttemptedRetries()); @@ -243,22 +258,21 @@ TEST_F(AWSClientTestSuite, TestClockSkewConsecutiveRequests) // first request should set the skew offset and retry, but following requests should not HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); auto outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); - QueueMockResponse(HttpResponseCode::UNAUTHORIZED, responseHeaders); + QueueMockResponse(HttpResponseCode::UNAUTHORIZED, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); // skew already applied; the offset now matches, so no retry. ASSERT_EQ(HttpResponseCode::UNAUTHORIZED, outcome.GetError().GetResponseCode()); ASSERT_STREQ("127.0.0.1", outcome.GetError().GetRemoteHostIpAddress().c_str()); ASSERT_EQ(0, client->GetRequestAttemptedRetries()); - QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); + QueueMockResponse(HttpResponseCode::FORBIDDEN, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); // skew already applied; the offset now matches, so no retry. ASSERT_EQ(HttpResponseCode::FORBIDDEN, outcome.GetError().GetResponseCode()); @@ -275,10 +289,9 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make an initial request so that a skew adjustment is set HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code AmazonWebServiceRequestMock request; - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); auto outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); @@ -286,9 +299,8 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock skewed even further responseHeaders.clear(); responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(2)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 2 hours - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); - QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); - QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); + QueueMockResponse(HttpResponseCode::FORBIDDEN, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::FORBIDDEN, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); @@ -296,9 +308,8 @@ TEST_F(AWSClientTestSuite, TestClockChangesAfterSkewHasBeenSet) // make another request with the clock in sync with the server responseHeaders.clear(); responseHeaders.emplace("Date", DateTime::Now().ToGmtString(DateFormat::RFC822)); // server is in sync with client - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); - QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); - QueueMockResponse(HttpResponseCode::FORBIDDEN, responseHeaders); + QueueMockResponse(HttpResponseCode::FORBIDDEN, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::FORBIDDEN, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); @@ -308,10 +319,10 @@ TEST_F(AWSClientTestSuite, TestRetryHeaders) { // The first server time is ahead of us by 1 hour. DateTime serverTime1 = DateTime::Now() + std::chrono::hours(1); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("SignatureDoesNotMatch"))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, CoreErrors::SIGNATURE_DOES_NOT_MATCH, HeaderValueCollection{std::make_pair("Date", serverTime1.ToGmtString(DateFormat::RFC822))}); // The second server time is ahead of us by 2 hour. DateTime serverTime2 = DateTime::Now() + std::chrono::hours(2); - QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822)), std::make_pair("x-amzn-errortype", Aws::String("SignatureDoesNotMatch"))}); + QueueMockResponse(HttpResponseCode::REQUEST_NOT_MADE, CoreErrors::SIGNATURE_DOES_NOT_MATCH, HeaderValueCollection{std::make_pair("Date", serverTime2.ToGmtString(DateFormat::RFC822))}); // The third server time is ahead of us by 3 hour. DateTime serverTime3 = DateTime::Now() + std::chrono::hours(3); QueueMockResponse(HttpResponseCode::OK, HeaderValueCollection{std::make_pair("Date", serverTime3.ToGmtString(DateFormat::RFC822))}); @@ -353,9 +364,8 @@ TEST_F(AWSClientTestSuite, TestRetryURIs) { HeaderValueCollection responseHeaders; responseHeaders.emplace("Date", (DateTime::Now() + std::chrono::hours(1)).ToGmtString(DateFormat::RFC822)); // server is ahead of us by 1 hour - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code - QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); - QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, responseHeaders); + QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::INTERNAL_SERVER_ERROR, CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); URI uri("http://www.uri.com/path with space/to/res"); AmazonWebServiceRequestMock request; auto outcome = client->MakeRequest(uri, request); diff --git a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp index ac53990a46f2..8a91505bbd6b 100644 --- a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp @@ -188,7 +188,7 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite protected: std::shared_ptr mockHttpClient; std::shared_ptr mockHttpClientFactory; - Aws::UniquePtr client; + Aws::UniquePtr client; void SetUp() { @@ -203,7 +203,7 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClientFactory = Aws::MakeShared(ALLOCATION_TAG); mockHttpClientFactory->SetClient(mockHttpClient); SetHttpClientFactory(mockHttpClientFactory); - client = Aws::MakeUnique(ALLOCATION_TAG, config); + client = Aws::MakeUnique(ALLOCATION_TAG, config); Aws::Monitoring::CleanupMonitoring(); std::vector factoryFunctions; @@ -235,8 +235,25 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite HttpMethod::HTTP_GET, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); auto httpResponse = Aws::MakeShared(ALLOCATION_TAG, httpRequest); httpResponse->SetResponseCode(code); - // JSON body so the JSON error marshaller parses it and reads the x-amzn-errortype header. - httpResponse->GetResponseBody() << "{}"; + httpResponse->GetResponseBody() << ""; + for(auto&& header : headers) + { + httpResponse->AddHeader(header.first, header.second); + } + mockHttpClient->AddResponseToReturn(httpResponse); + } + + // Stage a response carrying a specific service error code (client-error channel) so BuildAWSError + // yields a typed error with the response headers, enough to drive clock-skew detection. + void QueueMockResponse(HttpResponseCode code, Aws::Client::CoreErrors errorType, const HeaderValueCollection& headers) + { + auto httpRequest = CreateHttpRequest(URI(URI_STRING), + HttpMethod::HTTP_GET, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); + httpRequest->SetResolvedRemoteHost("127.0.0.1"); + auto httpResponse = Aws::MakeShared(ALLOCATION_TAG, httpRequest); + httpResponse->SetResponseCode(code); + httpResponse->SetClientErrorType(errorType); + httpResponse->GetResponseBody() << ""; for(auto&& header : headers) { httpResponse->AddHeader(header.first, header.second); @@ -250,11 +267,10 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); // BAD_REQUEST is not retryable, but since this is triggered by clock skew, it's set to mandatory retryable. - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, Aws::Client::CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); QueueMockResponse(HttpResponseCode::OK, responseHeaders); auto outcome = client->MakeRequest(request); AWS_ASSERT_SUCCESS(outcome); @@ -273,11 +289,10 @@ TEST_F(MonitoringTestSuite, TestMonitoringListenersAreCalledCorrectlyWithRetryAn HeaderValueCollection responseHeaders, requestHeaders; responseHeaders.emplace("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is ahead of us by 1 hour AmazonWebServiceRequestMock request; - responseHeaders.emplace("x-amzn-errortype", "SignatureDoesNotMatch"); // clock-skew error code requestHeaders.emplace("X-Amz-Date", Aws::Utils::DateTime::Now().ToGmtString(Aws::Utils::DateFormat::ISO_8601)); request.SetHeaders(requestHeaders); - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); - QueueMockResponse(HttpResponseCode::BAD_REQUEST, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, Aws::Client::CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); + QueueMockResponse(HttpResponseCode::BAD_REQUEST, Aws::Client::CoreErrors::SIGNATURE_DOES_NOT_MATCH, responseHeaders); auto outcome = client->MakeRequest(request); ASSERT_FALSE(outcome.IsSuccess()); ASSERT_EQ(1, client->GetRequestAttemptedRetries()); diff --git a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp index dd748b0d9ee5..9d28db2fe828 100644 --- a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp +++ b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp @@ -1,6 +1,9 @@ #include #include #include +#include +#include +#include #include #include #include @@ -312,6 +315,36 @@ TEST_F(S3UnitTest, S3EmbeddedErrorTest) { EXPECT_EQ("656c76696e6727732072657175657374", response.GetError().GetRequestId()); } +// Clock skew end to end through a real client: a SignatureDoesNotMatch error with the server clock an hour ahead drives a signing-time adjustment and one retry. +TEST_F(S3UnitTest, ClockSkewAdjustmentRetries) { + AWSCredentials credentials{"mock", "credentials"}; + const auto epProvider = Aws::MakeShared(ALLOCATION_TAG); + S3ClientConfiguration s3Config; + s3Config.region = "us-east-1"; + s3Config.retryStrategy = Aws::MakeShared(ALLOCATION_TAG); // fixture default is NoRetry + S3TestClient retryingClient{credentials, epProvider, s3Config}; + + auto makeSkewResponse = []() -> std::shared_ptr { + auto mockRequest = Aws::MakeShared(ALLOCATION_TAG, "mockuri", HttpMethod::HTTP_GET); + mockRequest->SetResponseStreamFactory(Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); + auto mockResponse = Aws::MakeShared(ALLOCATION_TAG, mockRequest); + mockResponse->SetResponseCode(HttpResponseCode::FORBIDDEN); + // Write the body (so tellp() > 0 and the XML error marshaller parses it rather than treating it as bodyless). + mockResponse->GetResponseBody() << "SignatureDoesNotMatchclock skew"; + mockResponse->AddHeader("Date", (Aws::Utils::DateTime::Now() + std::chrono::hours(1)).ToGmtString(Aws::Utils::DateFormat::RFC822)); // server is 1 hour ahead + return mockResponse; + }; + + _mockHttpClient->Reset(); + _mockHttpClient->AddResponseToReturn(makeSkewResponse()); + _mockHttpClient->AddResponseToReturn(makeSkewResponse()); + + const auto response = retryingClient.CopyObject(CopyObjectRequest().WithBucket("b").WithKey("k").WithCopySource("s")); + EXPECT_FALSE(response.IsSuccess()); + EXPECT_STREQ("SignatureDoesNotMatch", response.GetError().GetExceptionName().c_str()); // real XML marshaller mapped the code + EXPECT_EQ(2u, _mockHttpClient->GetAllRequestsMade().size()); // clock skew drove exactly one retry +} + class MockRequest : public Aws::AmazonWebServiceRequest { public: diff --git a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h index 73f7a2ff9963..28b835c87005 100644 --- a/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h +++ b/tests/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h @@ -5,7 +5,6 @@ #include #include -#include #include #include #include @@ -162,6 +161,9 @@ class MockAWSClient : Aws::Client::AWSClient { bool retryable = response->GetClientErrorType() == Aws::Client::CoreErrors::NETWORK_CONNECTION ? true : false; error = Aws::Client::AWSError(response->GetClientErrorType(), "", response->GetClientErrorMessage(), retryable); + error.SetResponseHeaders(response->GetHeaders()); + error.SetResponseCode(response->GetResponseCode()); + error.SetRemoteHostIpAddress(response->GetOriginatingRequest().GetResolvedRemoteHost()); return error; } error = Aws::Client::AWSError(Aws::Client::CoreErrors::INVALID_ACTION, false); @@ -247,40 +249,3 @@ class MockAWSClientWithStandardRetryStrategy : Aws::Client::AWSClient return err; } }; - -// JSON client for the clock-skew pipeline tests: builds errors via the real JsonErrorMarshaller (no -// bespoke BuildAWSError). Use a non-retryable clock-skew code so retries come only from clock skew. -class ClockSkewMockAWSClient : public Aws::Client::AWSJsonClient -{ -public: - ClockSkewMockAWSClient(const Aws::Client::ClientConfiguration& config, - const std::shared_ptr& errorMarshaller) - : Aws::Client::AWSJsonClient(config, - Aws::MakeShared("ClockSkewMockAWSClient", - Aws::MakeShared("ClockSkewMockAWSClient", - MockAWSClient::GetMockAccessKey(), MockAWSClient::GetMockSecretAccessKey()), - "service", config.region.empty() ? Aws::Region::US_EAST_1 : config.region), - errorMarshaller), - m_countedRetryStrategy(std::static_pointer_cast(config.retryStrategy)) { } - - ClockSkewMockAWSClient(const Aws::Client::ClientConfiguration& config) - : ClockSkewMockAWSClient(config, Aws::MakeShared("ClockSkewMockAWSClient")) { } - - Aws::Client::HttpResponseOutcome MakeRequest(const Aws::AmazonWebServiceRequest& request) - { - return MakeRequest(Aws::Http::URI("domain.com/something"), request); - } - - Aws::Client::HttpResponseOutcome MakeRequest(const Aws::Http::URI& uri, const Aws::AmazonWebServiceRequest& request) - { - m_countedRetryStrategy->ResetAttemptedRetriesCount(); - return AttemptExhaustively(uri, request, Aws::Http::HttpMethod::HTTP_GET, Aws::Auth::SIGV4_SIGNER); - } - - long GetRequestAttemptedRetries() { return m_countedRetryStrategy->GetAttemptedRetriesCount(); } - - inline const char* GetServiceClientName() const override { return "MockAWSClient"; } - -private: - std::shared_ptr m_countedRetryStrategy; -}; From 0db04b0f391a4ba7e83b2bb4585050c691a133d5 Mon Sep 17 00:00:00 2001 From: kai lin Date: Wed, 2 Sep 2026 16:28:44 -0400 Subject: [PATCH 4/4] removed comments --- tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp | 3 --- tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp | 2 -- 2 files changed, 5 deletions(-) diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp index a2dc3da34c98..e5ff090eb727 100644 --- a/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp @@ -137,9 +137,6 @@ class AWSClientTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClient->AddResponseToReturn(httpResponse); } - // Stage a response whose error carries a specific service error code (via the client-error - // channel) plus an HTTP status code, remote host, and headers, so BuildAWSError yields a typed - // error with those headers/code -- enough to drive clock-skew detection end to end. void QueueMockResponse(HttpResponseCode code, CoreErrors errorType, const HeaderValueCollection& headers) { auto httpRequest = CreateHttpRequest(URI("http://www.uri.com/path/to/res"), diff --git a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp index 8a91505bbd6b..e308dee6246f 100644 --- a/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp +++ b/tests/aws-cpp-sdk-core-tests/monitoring/MonitoringTest.cpp @@ -243,8 +243,6 @@ class MonitoringTestSuite : public Aws::Testing::AwsCppSdkGTestSuite mockHttpClient->AddResponseToReturn(httpResponse); } - // Stage a response carrying a specific service error code (client-error channel) so BuildAWSError - // yields a typed error with the response headers, enough to drive clock-skew detection. void QueueMockResponse(HttpResponseCode code, Aws::Client::CoreErrors errorType, const HeaderValueCollection& headers) { auto httpRequest = CreateHttpRequest(URI(URI_STRING),