Backward incompatible changes introduced in TransferManager

[aokhovat]

Describe the bug

It appears there was a backward incompatible change introduced in PR 2598 affecting TransferManager::UploadFile. Prior to the change, the Aws::S3::S3Client objects created with Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never going over HTTPS worked OK. After the change, there is this error in AWS log file:

[WARN] 2023-10-10 23:47:11.679 AWSErrorMarshaller [140021297194752] Encountered Unknown AWSError 'MissingContentLength': 
[ERROR] 2023-10-10 23:47:11.679 AWSXmlClient [140021297194752] HTTP response code: 411
Resolved remote host IP address: 10.74.96.219
Request ID: tx000004254aebd81e6c5e9-006525e27f-196de6b5-dev-zone-pw
Exception name: MissingContentLength
Error message: Unable to parse ExceptionName: MissingContentLength Message:  

and it appears we need to use Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent option instead to make the call work after the PR merge.

PR was merged into GitHub with tag 1.11.131. I have confirmed the pre/post PR change with versions 1.11.110 vs 1.11.160 with a simple test app similar to the sample code in SDK documentation. (I could not compile versions closer to the 1.11.131 tag, as I kept getting compiler errors due to: undefined CURLINFO_SPEED_DOWNLOAD_T. )

The issue seems to manifest itself in AWSClient::BuildHttpRequest where "crc32" is added to the header. As a consequence, in AWSAuthV4Signer::SignRequest a new path is taken where there is a hash on the request.

Expected Behavior

I expected the changes to be backward compatible, i.e. my current option: Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never to keep working

Current Behavior

I need to change code to use Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent

Reproduction Steps

The SDK Sample code is what I used. Prior versions to 1.11.131 should work OK with Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never and versions higher seem to fail and a change to Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent is required

Possible Solution

No response

Additional Information/Context

We have had several different applications confirm that change to Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent works OK and Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never fails

AWS CPP SDK version used

Tags 1.11.110 and 1.11.160

Compiler and Version used

gcc (GCC) 10.2.1 20210130 (Red Hat 10.2.1-11)

Operating System and version

Red Hat Enterprise Linux Server release 7.9 (Maipo)

[sbiscigl]

Hey @aokhovat thanks for reaching out, I tried to replicate your issue using the following code and could not

#include <aws/core/Aws.h>
#include <aws/core/utils/threading/Executor.h>
#include <aws/transfer/TransferManager.h>
#include <aws/s3/S3Client.h>
#include <iostream>
#include <thread>

using namespace std;
using namespace Aws;
using namespace Aws::Utils;
using namespace Aws::S3;
using namespace Aws::Transfer;

int main() {
  Aws::SDKOptions options;
  options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Debug;

  Aws::InitAPI(options);
  {
    S3ClientConfiguration config({}, Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never);
    auto s3Client = Aws::MakeShared<Aws::S3::S3Client>("S3Client", config);
    auto executor = Aws::MakeShared<Aws::Utils::Threading::PooledThreadExecutor>("executor",
        std::thread::hardware_concurrency() - 1);
    Aws::Transfer::TransferManagerConfiguration managerConfiguration(executor.get());
    managerConfiguration.s3Client = s3Client;
    managerConfiguration.transferInitiatedCallback = [](const TransferManager *,
        const std::shared_ptr<const TransferHandle> &) -> void {};
    managerConfiguration.errorCallback = [](const TransferManager *,
        const std::shared_ptr<const TransferHandle> &,
        const Aws::Client::AWSError<Aws::S3::S3Errors> &error) -> void {
        std::cout << "Transfer failed!\n";
        std::cout << "Error: " << error.GetMessage() << "\n";
    };
    auto transferManager = Aws::Transfer::TransferManager::Create(managerConfiguration);

    transferManager->UploadFile("/path/to/your/file/file.txt",
        "yourbucketname",
        "yourobjectkey",
        "text/plain",
        {});

    transferManager->WaitUntilAllFinished();
  }
  Aws::ShutdownAPI(options);
  return 0;
}

You are right in that, that change did change behavior. We are now chunk encoding checksums per the s3 spec instead of using MD5. This however should not effect you and be backwards compatible as S3 supports this. We actually run tests to verify that transfer does work, additionally the never policy setting is used by default.

I am however curious are you using AWS S3 as a backend or something with a compatible API like minio?

[aokhovat]

@sbiscigl Thanks for looking into this issue. We have both S3 and S3 compatible API's. We need to do some more investigation on our side. Will make sure to update here. Thanks again.

[sbiscigl]

With AWS S3 I suspect you will not see failures, with S3 compatible APIs my guess is that the service doesnt support crc32/trailing checksums. If you want the old behavior you can set the configuration to use not set again. i.e.

Aws::Transfer::TransferManagerConfiguration managerConfiguration(...);
managerConfiguration.checksumAlgorithm = S3::Model::ChecksumAlgorithm::NOT_SET;

As for the S3 compat API, whatever you are using will have to support the trailing headers as outlined by S3

[aokhovat]

@sbiscigl Thanks again for all your help. It was tracked down to a S3 compatible API issue. I will close the ticket now.

[github-actions[bot]]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.