Skip to content

Conversation

@sbiscigl
Copy link
Contributor

Description of changes:

  • updates the CRT to version v0.31.1
  • Fixes aws-chunked encoding in the CRT http cient, it did not work before.

A simple put object/get object with the CRT HTTP client and vanillia s3 client does not work before this patch as aws-chunked encoding was broken

i.e.

#include <aws/core/Aws.h>
#include <aws/s3/S3Client.h>
#include <aws/s3/model/GetObjectRequest.h>
#include <aws/s3/model/PutObjectRequest.h>

using namespace Aws;
using namespace Aws::Utils;
using namespace Aws::S3;
using namespace Aws::S3::Model;

namespace {
const char* LOG_TAG = "test-application";
}

auto main() -> int {
  SDKOptions options{};
  options.loggingOptions.logLevel = Logging::LogLevel::Debug;
  InitAPI(options);
  {
    S3Client client{};
    auto request = PutObjectRequest();
    request.SetBucket("sbiscigl");
    request.SetKey("test-crt-http-client");
    auto body = Aws::MakeShared<StringStream>(LOG_TAG, "some test string body");
    request.SetBody(body);
    const auto response = client.PutObject(request);
    assert(response.IsSuccess());
    auto get_object_request = GetObjectRequest();
    get_object_request.SetBucket(BUCKET_NAME);
    get_object_request.SetKey(KEY_NAME);
    const auto get_object_result = client.GetObject(get_object_request);
    assert(get_object_result.IsSuccess());
    std::cout << get_object_result.GetResult().GetBody().rdbuf() << std::endl;
  }
  ShutdownAPI(options);
  return 0;
}

This fixes the CRT HTTP client to use the same patch that we used for the original curl issuse(#3186).

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sbiscigl sbiscigl force-pushed the update-crt-fix-chunked branch from a9cd51b to 679160d Compare March 18, 2025 19:55
@sbiscigl sbiscigl marked this pull request as ready for review March 18, 2025 19:56
size_t amountToRead = buffer.capacity - buffer.len;
const auto bytesRead = BufferedRead(reinterpret_cast<char *>(buffer.buffer), amountToRead);
buffer.len += bytesRead;;
return bytesRead > 0;
Copy link
Contributor

@graebm graebm Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/awslabs/aws-crt-cpp/blob/743ab3891c4fb015a74713b7b631c720ce0ffe8e/include/aws/crt/io/Stream.h#L108-L113

                 * @return true if nothing went wrong.
                 * Return true even if you read 0 bytes because the end-of-file has been reached.
                 * Return true even if you read 0 bytes because data is not currently available.
                 *
                 * Return false if an actual failure condition occurs,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@sbiscigl sbiscigl force-pushed the update-crt-fix-chunked branch from 679160d to 136ff14 Compare March 18, 2025 20:46
@sbiscigl sbiscigl force-pushed the update-crt-fix-chunked branch from 136ff14 to 87121b4 Compare March 18, 2025 21:05
@sbiscigl sbiscigl merged commit f8951cd into main Mar 19, 2025
3 of 4 checks passed
@sbiscigl sbiscigl deleted the update-crt-fix-chunked branch March 19, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants