Upload fails with InvalidChunkSizeError for large objects

[rohansuri]

Describe the bug

Hello,

I'm trying a simple S3 PUT of an object of size 1MB. Through some reading of the code, I found that the sdk reads the request body at least thrice:

• md5 checksum
• sign the payload
• actually sending the payload

So I decided to:

• turn off signing
• choose CRC32C as the checksum algorithm, which as per this thread is streaming

For small objects (1KB), it works fine. For large object, of size 1MB, I see the following error:

Unable to parse ExceptionName: InvalidChunkSizeError Message: Only the last chunk is allowed to have a size less than 8192 bytes

Expected Behavior

The request should succeed even with custom configurations set that override the default checksum algorithm as well as payload signing policy.

Current Behavior

The PUT request fails with HTTP error code 403 stating InvalidChunkSizeError.

Reproduction Steps

TEST_F(S3Test, crc){
    Aws::SDKOptions options;
    Aws::InitAPI(options);

    Aws::S3::S3ClientConfiguration config;
    config.payloadSigningPolicy =
            Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never;
    auto client = std::make_unique<Aws::S3::S3Client>(config);

    Aws::S3::Model::PutObjectRequest request;
    request.SetBucket("s3-gtest");
    request.SetKey("crctest");
    // Use a checksum that is streaming and so doesn't require reading the
    // request body twice.
    request.SetChecksumAlgorithm(Aws::S3::Model::ChecksumAlgorithm::CRC32C);

    std::string s(1024*1024, 'a');
    std::stringstream ss;
    ss << s;
    auto stream = Aws::MakeShared<Aws::IOStream>(nullptr, ss.rdbuf());
    request.SetBody(stream);

    auto outcome = client->PutObject(request);
    if(!outcome.IsSuccess()){
        std::cout << "outcome err = " << outcome.GetError().GetMessage() << std::endl;
    }
    client.reset();
    Aws::ShutdownAPI(options);
}

stdout:

outcome err = Unable to parse ExceptionName: InvalidChunkSizeError Message: Only the last chunk is allowed to have a size less than 8192 bytes

Possible Solution

No response

Additional Information/Context

No response

AWS CPP SDK version used

1.11.411

Compiler and Version used

Apple clang version 15.0.0 (clang-1500.3.9.4)

Operating System and version

MacOS 14.4.1

[rohansuri]

@DmitriyMusatkin any thoughts?

[jmklix]

We are looking into this

[rohansuri]

Thanks for the update @jmklix

[sbiscigl]

@rohansuri I was able to replicate this consistenly, truly appreciate the complete reproduction. This issue is that in our curl client where we apply aws-chunking curl never guarantees that 8KB will be present in a chunk. This is a fundamental design flaw that we are working on ironing out right now. We are currently working on a checksumming code refactor to make this more straight forward and easier to follow. We are treating this with a high priority

[rohansuri]

Thank you @sbiscigl for looking into this!

Once the issue is fixed and the client is configured to:

• turn off signing
• use CRC32C for the checksum,

is it ensured that the request body for PUT requests will only be streamed once?

Also is there a way for now to workaround this issue (maybe switching the internal HTTP client implementation that is being used)?

[sgoth]

I'm also seeing this error randomly with PUTs via TransferManager::UploadFile on 1.11.352.
Not sure if TransferManager selects such a checksum algo internally - i don't set one explicitly.

Retrying makes it work sooner or later...

[rohansuri]

@sbiscigl any updates on this?

[sgoth]

Tried to see what checksumming is in use for me.

As i use contentMD5 = true, multipart uploads set it to NOT_SET - which is overwritten to MD5 internally as i learned here: #2968

Relevant code in TransferManager:

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

Lines 390 to 393 in 7c44fbe

	createMultipartRequest.SetChecksumAlgorithm(m_transferConfig.computeContentMD5
	? S3::Model::ChecksumAlgorithm::NOT_SET
	: m_transferConfig.checksumAlgorithm);
	createMultipartRequest.SetCustomizedAccessLogTag(m_transferConfig.customizedAccessLogTag);

For SinglePart it seems completely off (probably a failed merge conflict resolution):

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

Lines 550 to 553 in 7c44fbe

	putObjectRequest.SetChecksumAlgorithm(m_transferConfig.computeContentMD5
	? S3::Model::ChecksumAlgorithm::NOT_SET
	: m_transferConfig.checksumAlgorithm);
	putObjectRequest.SetChecksumAlgorithm(m_transferConfig.checksumAlgorithm);

First it is set to NOT_SET then again to TransferManagerConfig default CRC32

Is it currently possible to get reliable PUTs with any checksum/contentMD5 setting?

Edit:
As i was testing with file of around 2MB in size this resulted in the SinglePart path.
Explicitly setting the TransferManagerConfig.checksumAlgorithm to NOT_SET (seemingly) makes it work correctly.
100/100 uploads worked when before 1/3 did not - always the same file.

So not only CRC32C but also CRC32 seems problematic.

[sbiscigl]

@sbiscigl any updates on this?

We're refactoring a lot of how we are doing checksum to address continuing issues with checksumming that we see
i.e.
pull/3134
pull/3140

we're revisiting and re-working a lot of the code around checksums and S3.

Explicitly setting the TransferManagerConfig.checksumAlgorithm to NOT_SET (seemingly) makes it work correctly.

checksum algorithm NOT_SET will default to MD5 in the SDK. setting the transfer manager config to use contentMD5 will result it to use NOT_SET which in turn will use MD5.

So not only CRC32C but also CRC32 seems problematic.

Its not a specific checksum its aws-chunked. We are working at fixing it, and we're doing a hoslitic refactor to make it more user friendly for everyone as we understand its quite frustrating right now.

[rohansuri]

@sbiscigl thank you for the update.

[rohansuri]

As part of this, will we also look into permitting the user to completely turn off the checksum?

[sbiscigl]

As part of this, will we also look into permitting the user to completely turn off the checksum?

yes

[sbiscigl]

also merged your request @sgoth, good catch "probably a failed merge conflict resolution)" is correct, sorry about that, thank you for bringing it to our attention and helping us fix it.

[rohansuri]

Hello, just checking if we have any updates here :)

[jmklix]

This should be fixed with this PR: #3186

[rohansuri]

thank you @jmklix @sbiscigl!

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

[rohansuri]

@jmklix any expected date when this will be released?

[sgoth]

@rohansuri it already is - see 061f67b to see all the tags/releases that include the commit. Every release >= 1.11.445 has the change.

[rohansuri]

Thank you @sgoth ! This issue has the pending-release tag so I wasn't sure if it got released.

[rohansuri]

I just validated the fix today (same unit test that used to fail) and it is all good. Thank you so much for fixing this!