Skip to content

Commit cb96480

Browse files
pulimsrpulimsr
authored
Verifying ContentRange on response from GetObject (#3604)
1 parent cc79f0d commit cb96480

File tree

5 files changed

+195
-3
lines changed

5 files changed

+195
-3
lines changed

‎cmake/sdksCommon.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ list(APPEND SDK_TEST_PROJECT_LIST "s3control:tests/aws-cpp-sdk-s3control-integra
112112
list(APPEND SDK_TEST_PROJECT_LIST "sns:tests/aws-cpp-sdk-sns-integration-tests")
113113
list(APPEND SDK_TEST_PROJECT_LIST "sqs:tests/aws-cpp-sdk-sqs-integration-tests")
114114
list(APPEND SDK_TEST_PROJECT_LIST "sqs:tests/aws-cpp-sdk-sqs-unit-tests")
115-
list(APPEND SDK_TEST_PROJECT_LIST "transfer:tests/aws-cpp-sdk-transfer-tests")
115+
list(APPEND SDK_TEST_PROJECT_LIST "transfer:tests/aws-cpp-sdk-transfer-tests,tests/aws-cpp-sdk-transfer-unit-tests")
116116
list(APPEND SDK_TEST_PROJECT_LIST "text-to-speech:tests/aws-cpp-sdk-text-to-speech-tests,tests/aws-cpp-sdk-polly-sample")
117117
list(APPEND SDK_TEST_PROJECT_LIST "timestream-query:tests/aws-cpp-sdk-timestream-query-unit-tests")
118118
list(APPEND SDK_TEST_PROJECT_LIST "transcribestreaming:tests/aws-cpp-sdk-transcribestreaming-integ-tests")

‎src/aws-cpp-sdk-transfer/source/transfer/TransferManager.cpp‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,35 @@ namespace Aws
825825
return rangeStream.str();
826826
}
827827

828+
static bool VerifyContentRange(const Aws::String& requestedRange, const Aws::String& responseContentRange)
829+
{
830+
if (requestedRange.empty() || responseContentRange.empty())
831+
{
832+
return false;
833+
}
834+
835+
const auto requestPrefix = "bytes=";
836+
if (requestedRange.substr(0, strlen(requestPrefix)) != requestPrefix)
837+
{
838+
return false;
839+
}
840+
Aws::String requestRange = requestedRange.substr(strlen(requestPrefix));
841+
842+
const auto responsePrefix = "bytes ";
843+
if (responseContentRange.substr(0, strlen(responsePrefix)) != responsePrefix)
844+
{
845+
return false;
846+
}
847+
Aws::String responseRange = responseContentRange.substr(strlen(responsePrefix));
848+
size_t slashPos = responseRange.find('/');
849+
if (slashPos != Aws::String::npos)
850+
{
851+
responseRange = responseRange.substr(0, slashPos);
852+
}
853+
854+
return requestRange == responseRange;
855+
}
856+
828857
void TransferManager::DoSinglePartDownload(const std::shared_ptr<TransferHandle>& handle)
829858
{
830859
auto queuedParts = handle->GetQueuedParts();
@@ -1091,7 +1120,6 @@ namespace Aws
10911120
const std::shared_ptr<const Aws::Client::AsyncCallerContext>& context)
10921121
{
10931122
AWS_UNREFERENCED_PARAM(client);
1094-
AWS_UNREFERENCED_PARAM(request);
10951123

10961124
std::shared_ptr<TransferHandleAsyncContext> transferContext =
10971125
std::const_pointer_cast<TransferHandleAsyncContext>(std::static_pointer_cast<const TransferHandleAsyncContext>(context));
@@ -1108,8 +1136,32 @@ namespace Aws
11081136
handle->SetError(outcome.GetError());
11091137
TriggerErrorCallback(handle, outcome.GetError());
11101138
}
1111-
else
1139+
else if (request.RangeHasBeenSet())
11121140
{
1141+
const auto& requestedRange = request.GetRange();
1142+
const auto& responseContentRange = outcome.GetResult().GetContentRange();
1143+
1144+
if (responseContentRange.empty() || !VerifyContentRange(requestedRange, responseContentRange)) {
1145+
Aws::Client::AWSError<Aws::S3::S3Errors> error(Aws::S3::S3Errors::INTERNAL_FAILURE,
1146+
"ContentRangeMismatch",
1147+
"ContentRange in response does not match requested range",
1148+
false);
1149+
AWS_LOGSTREAM_ERROR(CLASS_TAG, "Transfer handle [" << handle->GetId()
1150+
<< "] ContentRange mismatch. Requested: [" << requestedRange
1151+
<< "] Received: [" << responseContentRange << "]");
1152+
handle->ChangePartToFailed(partState);
1153+
handle->SetError(error);
1154+
TriggerErrorCallback(handle, error);
1155+
handle->Cancel();
1156+
1157+
if(partState->GetDownloadBuffer())
1158+
{
1159+
m_bufferManager.Release(partState->GetDownloadBuffer());
1160+
partState->SetDownloadBuffer(nullptr);
1161+
}
1162+
return;
1163+
}
1164+
11131165
if(handle->ShouldContinue())
11141166
{
11151167
Aws::IOStream* bufferStream = partState->GetDownloadPartStream();

‎tests/aws-cpp-sdk-transfer-tests/TransferTests.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2328,6 +2328,40 @@ TEST_P(TransferTests, TransferManager_TestRelativePrefix)
23282328
}
23292329
}
23302330

2331+
TEST_P(TransferTests, TransferManager_ContentRangeVerificationTest)
2332+
{
2333+
const Aws::String RandomFileName = Aws::Utils::UUID::RandomUUID();
2334+
Aws::String testFileName = MakeFilePath(RandomFileName.c_str());
2335+
ScopedTestFile testFile(testFileName, MEDIUM_TEST_SIZE, testString);
2336+
2337+
TransferManagerConfiguration transferManagerConfig(m_executor.get());
2338+
transferManagerConfig.s3Client = m_s3Clients[GetParam()];
2339+
auto transferManager = TransferManager::Create(transferManagerConfig);
2340+
2341+
std::shared_ptr<TransferHandle> uploadPtr = transferManager->UploadFile(testFileName, GetTestBucketName(), RandomFileName, "text/plain", Aws::Map<Aws::String, Aws::String>());
2342+
uploadPtr->WaitUntilFinished();
2343+
ASSERT_EQ(TransferStatus::COMPLETED, uploadPtr->GetStatus());
2344+
ASSERT_TRUE(WaitForObjectToPropagate(GetTestBucketName(), RandomFileName.c_str()));
2345+
2346+
auto downloadFileName = MakeDownloadFileName(RandomFileName);
2347+
auto createStreamFn = [=](){
2348+
#ifdef _MSC_VER
2349+
return Aws::New<Aws::FStream>(ALLOCATION_TAG, Aws::Utils::StringUtils::ToWString(downloadFileName.c_str()).c_str(), std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc);
2350+
#else
2351+
return Aws::New<Aws::FStream>(ALLOCATION_TAG, downloadFileName.c_str(), std::ios_base::out | std::ios_base::in | std::ios_base::binary | std::ios_base::trunc);
2352+
#endif
2353+
};
2354+
2355+
uint64_t offset = 1024;
2356+
uint64_t partSize = 2048;
2357+
std::shared_ptr<TransferHandle> downloadPtr = transferManager->DownloadFile(GetTestBucketName(), RandomFileName, offset, partSize, createStreamFn);
2358+
2359+
downloadPtr->WaitUntilFinished();
2360+
ASSERT_EQ(TransferStatus::COMPLETED, downloadPtr->GetStatus());
2361+
ASSERT_EQ(partSize, downloadPtr->GetBytesTotalSize());
2362+
ASSERT_EQ(partSize, downloadPtr->GetBytesTransferred());
2363+
}
2364+
23312365
INSTANTIATE_TEST_SUITE_P(Https, TransferTests, testing::Values(TestType::Https));
23322366
INSTANTIATE_TEST_SUITE_P(Http, TransferTests, testing::Values(TestType::Http));
23332367

‎tests/aws-cpp-sdk-transfer-unit-tests/CMakeLists.txt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
add_project(aws-cpp-sdk-transfer-unit-tests
2+
"Unit Tests for the Transfer Manager"
3+
aws-cpp-sdk-transfer
4+
aws-cpp-sdk-s3
5+
testing-resources
6+
aws_test_main
7+
aws-cpp-sdk-core)
8+
9+
add_definitions(-DRESOURCES_DIR="${CMAKE_CURRENT_SOURCE_DIR}/resources")
10+
11+
if(MSVC AND BUILD_SHARED_LIBS)
12+
add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1)
13+
endif()
14+
15+
enable_testing()
16+
17+
if(PLATFORM_ANDROID AND BUILD_SHARED_LIBS)
18+
add_library(${PROJECT_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/TransferUnitTests.cpp)
19+
else()
20+
add_executable(${PROJECT_NAME} ${CMAKE_CURRENT_SOURCE_DIR}/TransferUnitTests.cpp)
21+
endif()
22+
23+
set_compiler_flags(${PROJECT_NAME})
24+
set_compiler_warnings(${PROJECT_NAME})
25+
26+
target_link_libraries(${PROJECT_NAME} ${PROJECT_LIBS})
27+
28+
if(MSVC AND BUILD_SHARED_LIBS)
29+
set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "/DELAYLOAD:aws-cpp-sdk-transfer.dll /DELAYLOAD:aws-cpp-sdk-core.dll")
30+
target_link_libraries(${PROJECT_NAME} delayimp.lib)
31+
endif()

‎tests/aws-cpp-sdk-transfer-unit-tests/TransferUnitTests.cpp‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#include <gtest/gtest.h>
2+
#include <aws/core/Aws.h>
3+
#include <aws/core/utils/threading/PooledThreadExecutor.h>
4+
#include <aws/s3/S3Client.h>
5+
#include <aws/s3/model/GetObjectRequest.h>
6+
#include <aws/s3/model/GetObjectResult.h>
7+
#include <aws/transfer/TransferManager.h>
8+
#include <aws/testing/AwsTestHelpers.h>
9+
#include <aws/testing/MemoryTesting.h>
10+
#include <sstream>
11+
12+
using namespace Aws;
13+
using namespace Aws::S3;
14+
using namespace Aws::S3::Model;
15+
using namespace Aws::Transfer;
16+
using namespace Aws::Utils::Threading;
17+
18+
const char* ALLOCATION_TAG = "TransferUnitTest";
19+
20+
class MockS3Client : public S3Client {
21+
public:
22+
MockS3Client() : S3Client(){};
23+
24+
GetObjectOutcome GetObject(const GetObjectRequest& request) const override {
25+
GetObjectResult result;
26+
27+
if (request.RangeHasBeenSet()) {
28+
// Always return mismatched range to trigger validation failure
29+
result.SetContentRange("bytes 1024-2047/2048");
30+
}
31+
32+
auto stream = Aws::New<std::stringstream>(ALLOCATION_TAG);
33+
*stream << "mock data";
34+
result.ReplaceBody(stream);
35+
return GetObjectOutcome(std::move(result));
36+
}
37+
};
38+
39+
class TransferUnitTest : public testing::Test {
40+
protected:
41+
void SetUp() override {
42+
executor = Aws::MakeShared<PooledThreadExecutor>(ALLOCATION_TAG, 1);
43+
mockS3Client = Aws::MakeShared<MockS3Client>(ALLOCATION_TAG);
44+
}
45+
46+
static void SetUpTestSuite() {
47+
InitAPI(_options);
48+
}
49+
50+
static void TearDownTestSuite() {
51+
ShutdownAPI(_options);
52+
}
53+
54+
std::shared_ptr<PooledThreadExecutor> executor;
55+
std::shared_ptr<MockS3Client> mockS3Client;
56+
static SDKOptions _options;
57+
};
58+
59+
SDKOptions TransferUnitTest::_options;
60+
61+
TEST_F(TransferUnitTest, ContentValidationShouldFail) {
62+
TransferManagerConfiguration config(executor.get());
63+
config.s3Client = mockS3Client;
64+
auto transferManager = TransferManager::Create(config);
65+
66+
auto createStreamFn = []() {
67+
return Aws::New<std::stringstream>(ALLOCATION_TAG);
68+
};
69+
70+
// Request bytes 0-1023 but mock returns 1024-2047, should fail validation
71+
auto handle = transferManager->DownloadFile("test-bucket", "test-key", 0, 1024, createStreamFn);
72+
handle->WaitUntilFinished();
73+
74+
EXPECT_EQ(TransferStatus::FAILED, handle->GetStatus());
75+
}

0 commit comments

Comments
 (0)