Skip to content

Commit 4b7681d

Browse files
sbisciglsbiscigl
committed
add option to ignore content length in S3Crt
1 parent 36382c7 commit 4b7681d

File tree

6 files changed

+89
-30
lines changed

6 files changed

+89
-30
lines changed

‎generated/src/aws-cpp-sdk-s3-crt/include/aws/s3-crt/S3CrtClient.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7191,7 +7191,12 @@ namespace Aws
71917191
Aws::Client::XmlOutcome GenerateXmlOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
71927192
Aws::Client::StreamOutcome GenerateStreamOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
71937193

7194-
private:
7194+
protected:
7195+
void AddContentLengthToRequest(const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
7196+
const std::shared_ptr<Aws::IOStream>& body,
7197+
bool isChunked) const override;
7198+
7199+
private:
71957200
friend class Aws::Client::ClientWithAsyncTemplateMethods<S3CrtClient>;
71967201
void init(const S3Crt::ClientConfiguration& clientConfiguration,
71977202
const std::shared_ptr<Aws::Auth::AWSCredentialsProvider> credentialsProvider);

‎generated/src/aws-cpp-sdk-s3-crt/include/aws/s3-crt/S3CrtClientConfiguration.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,16 @@ namespace Aws
160160
* This setting maps to CRT's network_interface_names_array config.
161161
*/
162162
Aws::Vector<Aws::String> networkInterfaceNames;
163+
164+
/**
165+
* Configuration on how to handle missing content length. By default the SDK attempts to seek the stream
166+
* to find the total length of the stream before streaming. CRT will fallback to multipart upload if there
167+
* is no content length, creating multipart uploads of partSize before knowing the stream has ended.
168+
*/
169+
enum class CONTENT_LENGTH_CONFIGURATION {
170+
SEEK_STREAM,
171+
MULTIPART_UPLOAD,
172+
} contentLenghtConfiguration {CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM};
163173
/* End of S3 CRT specifics */
164174
private:
165175
void LoadS3CrtSpecificConfig(const Aws::String& profileName);

‎generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,10 +239,12 @@ S3CrtClient::S3CrtClient(const S3Crt::ClientConfiguration& clientConfiguration,
239239
signPayloads,
240240
false),
241241
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
242-
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
242+
m_clientConfiguration(clientConfiguration),
243243
m_credProvider(Aws::MakeShared<DefaultAWSCredentialsProviderChain>(ALLOCATION_TAG, credentialsProvider)),
244244
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
245245
{
246+
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
247+
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
246248
init(clientConfiguration, m_credProvider);
247249
}
248250

@@ -256,10 +258,12 @@ S3CrtClient::S3CrtClient(const AWSCredentials& credentials, const S3Crt::ClientC
256258
signPayloads,
257259
false),
258260
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
259-
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
261+
m_clientConfiguration(clientConfiguration),
260262
m_credProvider(Aws::MakeShared<SimpleAWSCredentialsProvider>(ALLOCATION_TAG, credentials)),
261263
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
262264
{
265+
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
266+
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);AWS_UNREFERENCED_PARAM(useVirtualAddressing);
263267
init(clientConfiguration, m_credProvider);
264268
}
265269

@@ -274,10 +278,12 @@ S3CrtClient::S3CrtClient(const std::shared_ptr<AWSCredentialsProvider>& credenti
274278
signPayloads,
275279
false),
276280
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
277-
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
281+
m_clientConfiguration(clientConfiguration),
278282
m_credProvider(credentialsProvider),
279283
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
280284
{
285+
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
286+
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
281287
init(clientConfiguration, m_credProvider);
282288
}
283289

@@ -5580,3 +5586,11 @@ bool S3CrtClient::MultipartUploadSupported() const
55805586
{
55815587
return true;
55825588
}
5589+
5590+
void S3CrtClient::AddContentLengthToRequest(const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
5591+
const std::shared_ptr<Aws::IOStream>& body,
5592+
bool isChunked) const {
5593+
if (m_clientConfiguration.contentLenghtConfiguration == S3CrtClientConfiguration::CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM) {
5594+
BASECLASS::AddContentLengthToRequest(httpRequest, body, isChunked);
5595+
}
5596+
}

‎src/aws-cpp-sdk-core/include/aws/core/client/AWSClient.h‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ namespace Aws
7676
* Abstract AWS Client. Contains most of the functionality necessary to build an http request, get it signed, and send it across the wire.
7777
*/
7878
class AWS_CORE_API AWSClient
79-
{
79+
{
8080
public:
8181
/**
8282
* configuration will be used for http client settings, retry strategy, throttles, and signing information.
@@ -285,6 +285,14 @@ namespace Aws
285285
virtual void BuildHttpRequest(const Aws::AmazonWebServiceRequest& request,
286286
const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest) const;
287287

288+
/**
289+
* Adds content-length to the request if the request has a body by attempting to seek the end of
290+
* the body.
291+
*/
292+
virtual void AddContentLengthToRequest(const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
293+
const std::shared_ptr<Aws::IOStream>& body,
294+
bool isChunked) const;
295+
288296
/**
289297
* Gets the underlying ErrorMarshaller for subclasses to use.
290298
*/
@@ -303,7 +311,6 @@ namespace Aws
303311
std::shared_ptr<Auth::AWSCredentialsProvider> GetCredentialsProvider() const {
304312
return m_signerProvider->GetCredentialsProvider();
305313
}
306-
protected:
307314

308315
/**
309316
* Creates an HttpRequest instance with the given URI and sets the proper headers from the

‎src/aws-cpp-sdk-core/source/client/AWSClient.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -820,30 +820,7 @@ void AWSClient::AddContentBodyToRequest(const std::shared_ptr<Aws::Http::HttpReq
820820
httpRequest->DeleteHeader(Http::CONTENT_LENGTH_HEADER);
821821
}
822822
}
823-
824-
//Add transfer-encoding:chunked to header
825-
if (body && isChunked && !httpRequest->HasHeader(Http::CONTENT_LENGTH_HEADER))
826-
{
827-
httpRequest->SetTransferEncoding(CHUNKED_VALUE);
828-
}
829-
//in the scenario where we are adding a content body as a stream, the request object likely already
830-
//has a content-length header set and we don't want to seek the stream just to find this information.
831-
else if (body && !httpRequest->HasHeader(Http::CONTENT_LENGTH_HEADER))
832-
{
833-
if (!m_httpClient->SupportsChunkedTransferEncoding())
834-
{
835-
AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "This http client doesn't support transfer-encoding:chunked. " <<
836-
"The request may fail if it's not a seekable stream.");
837-
}
838-
AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Found body, but content-length has not been set, attempting to compute content-length");
839-
body->seekg(0, body->end);
840-
auto streamSize = body->tellg();
841-
body->seekg(0, body->beg);
842-
Aws::StringStream ss;
843-
ss << streamSize;
844-
httpRequest->SetContentLength(ss.str());
845-
}
846-
823+
AddContentLengthToRequest(httpRequest, body, isChunked);
847824
if (needsContentMd5 && body && !httpRequest->HasHeader(Http::CONTENT_MD5_HEADER))
848825
{
849826
AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Found body, and content-md5 needs to be set" <<
@@ -862,6 +839,29 @@ void AWSClient::AddContentBodyToRequest(const std::shared_ptr<Aws::Http::HttpReq
862839
}
863840
}
864841

842+
void AWSClient::AddContentLengthToRequest(
843+
const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
844+
const std::shared_ptr<Aws::IOStream>& body,
845+
bool isChunked) const
846+
{
847+
if (body && isChunked && !httpRequest->HasHeader(Http::CONTENT_LENGTH_HEADER)) {
848+
httpRequest->SetTransferEncoding(CHUNKED_VALUE);
849+
} else if (body && !httpRequest->HasHeader(Http::CONTENT_LENGTH_HEADER))
850+
{
851+
if (!m_httpClient->SupportsChunkedTransferEncoding()) {
852+
AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "This http client doesn't support transfer-encoding:chunked. " <<
853+
"The request may fail if it's not a seekable stream.");
854+
}
855+
AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Found body, but content-length has not been set, attempting to compute content-length");
856+
body->seekg(0, body->end);
857+
auto streamSize = body->tellg();
858+
body->seekg(0, body->beg);
859+
Aws::StringStream ss;
860+
ss << streamSize;
861+
httpRequest->SetContentLength(ss.str());
862+
}
863+
}
864+
865865
Aws::String Aws::Client::GetAuthorizationHeader(const Aws::Http::HttpRequest& httpRequest)
866866
{
867867
// Extract the hex-encoded signature from the authorization header rather than recalculating it.

‎tests/aws-cpp-sdk-s3-crt-integration-tests/BucketAndObjectOperationTest.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,6 +1530,29 @@ namespace
15301530
}
15311531
}
15321532

1533+
TEST_F(BucketAndObjectOperationTest, ContentLengthMissingShouldWork) {
1534+
const Aws::String fullBucketName = CalculateBucketName(BASE_PUT_OBJECTS_BUCKET_NAME.c_str());
1535+
SCOPED_TRACE(Aws::String("FullBucketName ") + fullBucketName);
1536+
CreateBucketRequest createBucketRequest;
1537+
createBucketRequest.SetBucket(fullBucketName);
1538+
createBucketRequest.SetACL(BucketCannedACL::private_);
1539+
1540+
CreateBucketOutcome createBucketOutcome = Client->CreateBucket(createBucketRequest);
1541+
AWS_ASSERT_SUCCESS(createBucketOutcome);
1542+
const CreateBucketResult& createBucketResult = createBucketOutcome.GetResult();
1543+
ASSERT_TRUE(!createBucketResult.GetLocation().empty());
1544+
ASSERT_TRUE(WaitForBucketToPropagate(fullBucketName));
1545+
TagTestBucket(fullBucketName, Client);
1546+
1547+
S3CrtClientConfiguration configuration{};
1548+
configuration.contentLenghtConfiguration = S3CrtClientConfiguration::CONTENT_LENGTH_CONFIGURATION::MULTIPART_UPLOAD;
1549+
const S3CrtClient client{configuration};
1550+
auto request = PutObjectRequest{}.WithBucket(fullBucketName).WithKey("sam");
1551+
request.SetBody(Aws::MakeShared<StringStream>(ALLOCATION_TAG, "bridges"));
1552+
const auto response = client.PutObject(request);
1553+
AWS_EXPECT_SUCCESS(response);
1554+
}
1555+
15331556
class TestMonitoring: public Aws::Monitoring::MonitoringInterface
15341557
{
15351558
mutable std::shared_ptr<Aws::Vector<Aws::String>> m_sequence;

0 commit comments

Comments
 (0)