Skip to content

[Bug-Fix] Fix S3 multipart copy requiring s3:GetObjectVersion on versioned buckets #3867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kai-ion wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Bug-Fix] Fix S3 multipart copy requiring s3:GetObjectVersion on versioned buckets #3867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,11 +875,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
const auto& head = headOutcome.GetResult();
const Aws::String sourceETag = head.GetETag();

// Pin the source version so UploadPartCopy reads a stable object.
if (sourceVersionId.empty() && !head.GetVersionId().empty()) {
sourceVersionId = head.GetVersionId();
httpRequest->SetHeaderValue("x-amz-copy-source", Aws::Http::URI::URLEncodePath(copySource) + "?versionId=" + sourceVersionId);
}
if (!httpRequest->HasHeader("x-amz-copy-source-if-match") && !sourceETag.empty()) {
httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
}
Expand Down
76 changes: 75 additions & 1 deletion tests/aws-cpp-sdk-s3-crt-integration-tests/BucketAndObjectOperationTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@
#include <aws/s3-crt/model/PutObjectTaggingRequest.h>
#include <aws/s3-crt/model/MetadataDirective.h>
#include <aws/s3-crt/model/TaggingDirective.h>
#include <aws/s3-crt/model/PutBucketVersioningRequest.h>
#include <aws/s3-crt/model/VersioningConfiguration.h>
#include <aws/s3-crt/model/BucketVersioningStatus.h>
#include <aws/s3-crt/model/ListObjectVersionsRequest.h>
#include <aws/s3-crt/ClientConfiguration.h>
#include <aws/testing/ProxyConfig.h>
#include <aws/testing/platform/PlatformTesting.h>
Expand Down Expand Up @@ -89,6 +93,7 @@ namespace
static std::string BASE_EVENT_STREAM_ERRORS_IN_EVENT_TEST_BUCKET_NAME = "errorsinevent";
static std::string BASE_CHECKSUMS_BUCKET_NAME = "checksums-crt";
static std::string BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME = "crt-credentials-test";
static std::string BASE_VERSIONED_COPY_BUCKET_NAME = "versionedcopytest";
static const char* ALLOCATION_TAG = "BucketAndObjectOperationTest";
static const char* TEST_OBJ_KEY = "TestObjectKey";
static const char* TEST_NOT_MODIFIED_OBJ_KEY = "TestNotModifiedObjectKey";
Expand Down Expand Up @@ -129,7 +134,8 @@ namespace
std::ref(BASE_EVENT_STREAM_LARGE_FILE_TEST_BUCKET_NAME),
std::ref(BASE_EVENT_STREAM_ERRORS_IN_EVENT_TEST_BUCKET_NAME),
std::ref(BASE_CHECKSUMS_BUCKET_NAME),
std::ref(BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME)
std::ref(BASE_CRT_CREDENTIALS_TEST_BUCKET_NAME),
std::ref(BASE_VERSIONED_COPY_BUCKET_NAME)
};

for (auto& testBucketName : TEST_BUCKETS)
Expand Down Expand Up @@ -1077,6 +1083,74 @@ namespace
}
}

TEST_F(BucketAndObjectOperationTest, TestMultipartCopyOnVersionedBucketDoesNotRequireGetObjectVersion)
{
Aws::String fullBucketName = CalculateBucketName(BASE_VERSIONED_COPY_BUCKET_NAME.c_str());
SCOPED_TRACE(Aws::String("FullBucketName ") + fullBucketName);
CreateBucketRequest createBucketRequest;
createBucketRequest.SetBucket(fullBucketName);
createBucketRequest.SetACL(BucketCannedACL::private_);
CreateBucketOutcome createBucketOutcome = Client->CreateBucket(createBucketRequest);
AWS_ASSERT_SUCCESS(createBucketOutcome);
ASSERT_TRUE(WaitForBucketToPropagate(fullBucketName));
TagTestBucket(fullBucketName, Client);

PutBucketVersioningRequest versioningRequest;
versioningRequest.SetBucket(fullBucketName);
VersioningConfiguration versioningConfig;
versioningConfig.SetStatus(BucketVersioningStatus::Enabled);
versioningRequest.SetVersioningConfiguration(versioningConfig);
auto versioningOutcome = Client->PutBucketVersioning(versioningRequest);
AWS_ASSERT_SUCCESS(versioningOutcome);

const char* sourceKey = "multipart-copy-versioned-source";
const char* destKey = "multipart-copy-versioned-dest";

// Upload an object larger than the multipart copy threshold (>64MB for CRT)
const size_t objectSize = 70 * 1024 * 1024;
auto objectStream = Aws::MakeShared<Aws::StringStream>(ALLOCATION_TAG);
std::string largePayload(objectSize, 'x');
*objectStream << largePayload;
objectStream->flush();

PutObjectRequest putObjectRequest;
putObjectRequest.SetBucket(fullBucketName);
putObjectRequest.SetKey(sourceKey);
putObjectRequest.SetBody(objectStream);
putObjectRequest.SetContentLength(static_cast<long>(objectSize));
PutObjectOutcome putObjectOutcome = Client->PutObject(putObjectRequest);
AWS_ASSERT_SUCCESS(putObjectOutcome);
ASSERT_TRUE(WaitForObjectToPropagate(fullBucketName, sourceKey));

// Copy without specifying versionId. Should succeed without s3:GetObjectVersion.
CopyObjectRequest copyRequest;
copyRequest.WithBucket(fullBucketName)
.WithKey(destKey)
.WithCopySource(fullBucketName + "/" + sourceKey);
auto copyOutcome = Client->CopyObject(copyRequest);
AWS_ASSERT_SUCCESS(copyOutcome);

// Clean up: delete all object versions so the bucket can be deleted
ListObjectVersionsRequest listVersionsRequest;
listVersionsRequest.SetBucket(fullBucketName);
auto listVersionsOutcome = Client->ListObjectVersions(listVersionsRequest);
if (listVersionsOutcome.IsSuccess())
{
for (const auto& version : listVersionsOutcome.GetResult().GetVersions())
{
DeleteObjectRequest deleteRequest;
deleteRequest.SetBucket(fullBucketName);
deleteRequest.SetKey(version.GetKey());
deleteRequest.SetVersionId(version.GetVersionId());
Client->DeleteObject(deleteRequest);
}
}

DeleteBucketRequest deleteBucketRequest;
deleteBucketRequest.SetBucket(fullBucketName);
Client->DeleteBucket(deleteBucketRequest);
}

TEST_F(BucketAndObjectOperationTest, TestObjectOperationWithEventStream)
{
GTEST_SKIP() << "Select objects is not supported on new AWS accounts";
Expand Down
6 changes: 0 additions & 6 deletions ...s/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtSpecificOperations.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
const auto& head = headOutcome.GetResult();
const Aws::String sourceETag = head.GetETag();

// Pin the source version so UploadPartCopy reads a stable object.
if (sourceVersionId.empty() && !head.GetVersionId().empty())
{
sourceVersionId = head.GetVersionId();
httpRequest->SetHeaderValue("x-amz-copy-source", Aws::Http::URI::URLEncodePath(copySource) + "?versionId=" + sourceVersionId);
}
if (!httpRequest->HasHeader("x-amz-copy-source-if-match") && !sourceETag.empty())
{
httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
Expand Down
6 changes: 0 additions & 6 deletions ...amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/SmithyS3CrtSpecificOperations.vm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@ S3CrtClient::CopyObjectPropertiesOutcome S3CrtClient::PopulateCopyObjectProperti
const auto& head = headOutcome.GetResult();
const Aws::String sourceETag = head.GetETag();

// Pin the source version so UploadPartCopy reads a stable object.
if (sourceVersionId.empty() && !head.GetVersionId().empty())
{
sourceVersionId = head.GetVersionId();
httpRequest->SetHeaderValue("x-amz-copy-source", Aws::Http::URI::URLEncodePath(copySource) + "?versionId=" + sourceVersionId);
}
if (!httpRequest->HasHeader("x-amz-copy-source-if-match") && !sourceETag.empty())
{
httpRequest->SetHeaderValue("x-amz-copy-source-if-match", sourceETag);
Expand Down
Loading