From e672a7df2a80014cd35962e40eb17eb25a6d2c90 Mon Sep 17 00:00:00 2001 From: sbiscigl Date: Fri, 30 Jan 2026 11:49:39 -0500 Subject: [PATCH 1/3] add test for partially consumed stream checksum reuse --- .../aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp | 54 +++++++++++++++++++ .../aws/testing/mocks/http/MockHttpClient.h | 21 ++++++++ 2 files changed, 75 insertions(+) diff --git a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp index 3f9a45aa9ae4..87528e58f82e 100644 --- a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp +++ b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include #include #include @@ -609,4 +611,56 @@ TEST_F(S3UnitTest, testLegacyApi) "SignatureV4"); EXPECT_TRUE(outcome2.IsSuccess()); +} + +TEST_F(S3UnitTest, PartiallyConsumedStreamChecksumReuse) { + auto request = PutObjectRequest().WithBucket("(iamthou").WithKey("thouarti"); + // the body has to be over 8K as the checksum is read as we read in chunks, in this case + // we set the chunk size to 8K and we need the body to be larger than that. + const Aws::String bodyString(9216, 'a'); + request.SetBody(Aws::MakeShared(ALLOCATION_TAG, bodyString)); + + const auto errorResponseStream = Aws::MakeShared(ALLOCATION_TAG, "mockuri", HttpMethod::HTTP_POST); + errorResponseStream->SetResponseStreamFactory(Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); + auto errorResponse = Aws::MakeShared(ALLOCATION_TAG, errorResponseStream); + errorResponse->SetResponseCode(HttpResponseCode::REQUEST_TIMEOUT); + _mockHttpClient->AddResponseToReturn( + errorResponse, [](IOStream&) -> void {}, + [](const std::shared_ptr& request) -> void { + // Partially read the buffer, such that the request checksum ends up in a bad state. + Aws::Array tempBuffer; + request->GetContentBody()->read(tempBuffer.data(), 12); + }); + + const auto successResponseStream = Aws::MakeShared(ALLOCATION_TAG, "mockuri", HttpMethod::HTTP_POST); + successResponseStream->SetResponseStreamFactory(Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); + auto successResponse = Aws::MakeShared(ALLOCATION_TAG, errorResponseStream); + successResponse->SetResponseCode(HttpResponseCode::OK); + _mockHttpClient->AddResponseToReturn( + successResponse, [](IOStream&) -> void {}, [](const std::shared_ptr&) -> void {}); + + // The top level test has a no retry policy so we have to create one that retries + const AWSCredentials credentials{"mock", "credentials"}; + S3ClientConfiguration configuration; + configuration.httpClientChunkedMode = HttpClientChunkedMode::DEFAULT; + // Smallest chunk size allowed + configuration.awsChunkedBufferSize = 8192UL; + const S3Client clientWithRetries{credentials, nullptr, configuration}; + + const auto response = clientWithRetries.PutObject(request); + AWS_EXPECT_SUCCESS(response); + + EXPECT_EQ(_mockHttpClient->GetAllRequestsMade().size(), 2UL); + + Aws::Utils::Crypto::CRC64 crc64Hash{}; + const auto expectedChecksum = crc64Hash.Calculate(bodyString); + EXPECT_TRUE(expectedChecksum.IsSuccess()); + const Aws::Utils::Base64::Base64 base64{}; + const auto expectedChecksumBase64 = base64.Encode(expectedChecksum.GetResult()); + + const auto retriedRequest = _mockHttpClient->GetMostRecentHttpRequest(); + const auto seenChecksum = retriedRequest.GetRequestHash().second->GetHash(); + EXPECT_TRUE(seenChecksum.IsSuccess()); + const auto seenChecksumBase64 = base64.Encode(seenChecksum.GetResult()); + EXPECT_EQ(seenChecksumBase64, expectedChecksumBase64); } \ No newline at end of file diff --git a/tests/testing-resources/include/aws/testing/mocks/http/MockHttpClient.h b/tests/testing-resources/include/aws/testing/mocks/http/MockHttpClient.h index 420402bb45eb..b8f4d7a3ca4d 100644 --- a/tests/testing-resources/include/aws/testing/mocks/http/MockHttpClient.h +++ b/tests/testing-resources/include/aws/testing/mocks/http/MockHttpClient.h @@ -23,6 +23,9 @@ class MockHttpClient : public Aws::Http::HttpClient { public: using ResponseCallbackTuple = std::pair, std::function>; + using ResponseAndRequestCallbackTuple = std::tuple, + std::function, + std::function&)>>; std::shared_ptr MakeRequest(const std::shared_ptr& request, Aws::Utils::RateLimits::RateLimiterInterface* readLimiter = nullptr, @@ -46,6 +49,16 @@ class MockHttpClient : public Aws::Http::HttpClient } return responseToUse.first; } + if (!m_responseAndRequestsCallback.empty()) { + auto responseToUse = m_responseAndRequestsCallback.front(); + m_responseAndRequestsCallback.pop(); + if (std::get<0>(responseToUse)) { + std::get<0>(responseToUse)->SetOriginatingRequest(request); + std::get<1>(responseToUse)(std::get<0>(responseToUse)->GetResponseBody()); + std::get<2>(responseToUse)(request); + } + return std::get<0>(responseToUse); + } return Aws::MakeShared(MockHttpAllocationTag, request); } @@ -60,6 +73,11 @@ class MockHttpClient : public Aws::Http::HttpClient //when you are finished. void AddResponseToReturn(const std::shared_ptr& response) { m_responsesToUse.emplace(response, [](Aws::IOStream&) -> void {}); } void AddResponseToReturn(const std::shared_ptr& response, const std::function& callbackFucntion) { m_responsesToUse.emplace(response, callbackFucntion); } + void AddResponseToReturn(const std::shared_ptr& response, + const std::function& callbackFucntion, + const std::function&)>& requestCallback) { + m_responseAndRequestsCallback.emplace(response, callbackFucntion, requestCallback); + } void Reset() { @@ -68,9 +86,12 @@ class MockHttpClient : public Aws::Http::HttpClient std::swap(m_responsesToUse, empty); } + + private: mutable Aws::Vector m_requestsMade; mutable Aws::Queue m_responsesToUse; + mutable Aws::Queue m_responseAndRequestsCallback; }; class MockHttpClientFactory : public Aws::Http::HttpClientFactory From 3908edf8bfcc101a9276a47c7f41ded8cdd44bac Mon Sep 17 00:00:00 2001 From: sbaluja Date: Fri, 30 Jan 2026 15:37:31 -0500 Subject: [PATCH 2/3] Only save checksums when stream is at EOF --- .../source/client/AWSClient.cpp | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/aws-cpp-sdk-core/source/client/AWSClient.cpp b/src/aws-cpp-sdk-core/source/client/AWSClient.cpp index 3a0f2ca52aea..1548b0b9ece2 100644 --- a/src/aws-cpp-sdk-core/source/client/AWSClient.cpp +++ b/src/aws-cpp-sdk-core/source/client/AWSClient.cpp @@ -376,6 +376,17 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "Request failed, now waiting " << sleepMillis << " ms before attempting again."); if(request.GetBody()) { + if (request.GetBody()->tellg() == EOF) { + // Save checksum information from the original request if we haven't already and stream is finalized + RetryContext context = request.GetRetryContext(); + if (context.m_requestHash == nullptr) { + auto originalRequestHash = httpRequest->GetRequestHash(); + if (originalRequestHash.second != nullptr) { + context.m_requestHash = Aws::MakeShared>>(AWS_CLIENT_LOG_TAG, originalRequestHash); + request.SetRetryContext(context); + } + } + } request.GetBody()->clear(); request.GetBody()->seekg(0); } @@ -397,16 +408,6 @@ HttpResponseOutcome AWSClient::AttemptExhaustively(const Aws::Http::URI& uri, newUri.SetAuthority(newEndpoint); } - // Save checksum information from the original request if we haven't already - safe to assume that the checksum has been finalized, since we have sent and received a response - RetryContext context = request.GetRetryContext(); - if (context.m_requestHash == nullptr) { - auto originalRequestHash = httpRequest->GetRequestHash(); - if (originalRequestHash.second != nullptr) { - context.m_requestHash = Aws::MakeShared>>(AWS_CLIENT_LOG_TAG, originalRequestHash); - request.SetRetryContext(context); - } - } - httpRequest = CreateHttpRequest(newUri, method, request.GetResponseStreamFactory()); httpRequest->SetHeaderValue(Http::SDK_INVOCATION_ID_HEADER, invocationId); From 134c140eb8ae864e94c4e331eb3c4c3f1790612f Mon Sep 17 00:00:00 2001 From: sbaluja Date: Fri, 30 Jan 2026 15:38:22 -0500 Subject: [PATCH 3/3] Fix PartiallyConsumedStreamChecksumReuse test --- tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp index 87528e58f82e..b45d4c4a378d 100644 --- a/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp +++ b/tests/aws-cpp-sdk-s3-unit-tests/S3UnitTests.cpp @@ -637,7 +637,10 @@ TEST_F(S3UnitTest, PartiallyConsumedStreamChecksumReuse) { auto successResponse = Aws::MakeShared(ALLOCATION_TAG, errorResponseStream); successResponse->SetResponseCode(HttpResponseCode::OK); _mockHttpClient->AddResponseToReturn( - successResponse, [](IOStream&) -> void {}, [](const std::shared_ptr&) -> void {}); + successResponse, [](IOStream& stream) -> void {EXPECT_EQ(stream.tellg(), 0);}, [](const std::shared_ptr& request) -> void { + Aws::Array tempBuffer; + request->GetContentBody()->read(tempBuffer.data(), 9216); + }); // The top level test has a no retry policy so we have to create one that retries const AWSCredentials credentials{"mock", "credentials"}; @@ -650,8 +653,6 @@ TEST_F(S3UnitTest, PartiallyConsumedStreamChecksumReuse) { const auto response = clientWithRetries.PutObject(request); AWS_EXPECT_SUCCESS(response); - EXPECT_EQ(_mockHttpClient->GetAllRequestsMade().size(), 2UL); - Aws::Utils::Crypto::CRC64 crc64Hash{}; const auto expectedChecksum = crc64Hash.Calculate(bodyString); EXPECT_TRUE(expectedChecksum.IsSuccess());