Skip to content

Commit 10aeebc

Browse files
sbisciglsbiscigl
committed
Revert "add option to ignore content length in S3Crt"
This reverts commit 22e831e.
1 parent 22e831e commit 10aeebc

File tree

10 files changed

+35
-132
lines changed

10 files changed

+35
-132
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7191,10 +7191,6 @@ 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-
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;
71987194
private:
71997195
friend class Aws::Client::ClientWithAsyncTemplateMethods<S3CrtClient>;
72007196
void init(const S3Crt::ClientConfiguration& clientConfiguration,

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,6 @@ 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-
SKIP_CONTENT_LENGTH,
172-
} contentLenghtConfiguration {CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM};
173163
/* End of S3 CRT specifics */
174164
private:
175165
void LoadS3CrtSpecificConfig(const Aws::String& profileName);

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,10 @@ S3CrtClient::S3CrtClient(const S3Crt::ClientConfiguration& clientConfiguration,
239239
signPayloads,
240240
false),
241241
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
242-
m_clientConfiguration(clientConfiguration),
242+
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
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);
248246
init(clientConfiguration, m_credProvider);
249247
}
250248

@@ -258,12 +256,10 @@ S3CrtClient::S3CrtClient(const AWSCredentials& credentials, const S3Crt::ClientC
258256
signPayloads,
259257
false),
260258
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
261-
m_clientConfiguration(clientConfiguration),
259+
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
262260
m_credProvider(Aws::MakeShared<SimpleAWSCredentialsProvider>(ALLOCATION_TAG, credentials)),
263261
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
264262
{
265-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
266-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
267263
init(clientConfiguration, m_credProvider);
268264
}
269265

@@ -278,12 +274,10 @@ S3CrtClient::S3CrtClient(const std::shared_ptr<AWSCredentialsProvider>& credenti
278274
signPayloads,
279275
false),
280276
Aws::MakeShared<S3CrtErrorMarshaller>(ALLOCATION_TAG)),
281-
m_clientConfiguration(clientConfiguration, signPayloads),
277+
m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
282278
m_credProvider(credentialsProvider),
283279
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
284280
{
285-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
286-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
287281
init(clientConfiguration, m_credProvider);
288282
}
289283

@@ -5586,12 +5580,3 @@ bool S3CrtClient::MultipartUploadSupported() const
55865580
{
55875581
return true;
55885582
}
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-
{
5594-
if (m_clientConfiguration.contentLenghtConfiguration == S3CrtClientConfiguration::CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM) {
5595-
BASECLASS::AddContentLengthToRequest(httpRequest, body, isChunked);
5596-
}
5597-
}

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,6 @@ 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-
296288
/**
297289
* Gets the underlying ErrorMarshaller for subclasses to use.
298290
*/
@@ -311,6 +303,7 @@ namespace Aws
311303
std::shared_ptr<Auth::AWSCredentialsProvider> GetCredentialsProvider() const {
312304
return m_signerProvider->GetCredentialsProvider();
313305
}
306+
protected:
314307

315308
/**
316309
* 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 & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,30 @@ void AWSClient::AddContentBodyToRequest(const std::shared_ptr<Aws::Http::HttpReq
820820
httpRequest->DeleteHeader(Http::CONTENT_LENGTH_HEADER);
821821
}
822822
}
823-
AddContentLengthToRequest(httpRequest, body, isChunked);
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+
824847
if (needsContentMd5 && body && !httpRequest->HasHeader(Http::CONTENT_MD5_HEADER))
825848
{
826849
AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Found body, and content-md5 needs to be set" <<
@@ -839,28 +862,6 @@ void AWSClient::AddContentBodyToRequest(const std::shared_ptr<Aws::Http::HttpReq
839862
}
840863
}
841864

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-
if (!m_httpClient->SupportsChunkedTransferEncoding()) {
851-
AWS_LOGSTREAM_WARN(AWS_CLIENT_LOG_TAG, "This http client doesn't support transfer-encoding:chunked. " <<
852-
"The request may fail if it's not a seekable stream.");
853-
}
854-
AWS_LOGSTREAM_TRACE(AWS_CLIENT_LOG_TAG, "Found body, but content-length has not been set, attempting to compute content-length");
855-
body->seekg(0, body->end);
856-
auto streamSize = body->tellg();
857-
body->seekg(0, body->beg);
858-
Aws::StringStream ss;
859-
ss << streamSize;
860-
httpRequest->SetContentLength(ss.str());
861-
}
862-
}
863-
864865
Aws::String Aws::Client::GetAuthorizationHeader(const Aws::Http::HttpRequest& httpRequest)
865866
{
866867
// 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: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,29 +1530,6 @@ 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::SKIP_CONTENT_LENGTH;
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-
15561533
class TestMonitoring: public Aws::Monitoring::MonitoringInterface
15571534
{
15581535
mutable std::shared_ptr<Aws::Vector<Aws::String>> m_sequence;

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/S3ClientHeader.vm‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,6 @@ namespace ${rootNamespace}
212212
Aws::Client::XmlOutcome GenerateXmlOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
213213
Aws::Client::StreamOutcome GenerateStreamOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
214214

215-
protected:
216-
void AddContentLengthToRequest(const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
217-
const std::shared_ptr<Aws::IOStream>& body,
218-
bool isChunked) const override;
219215
#end
220216
private:
221217
friend class Aws::Client::ClientWithAsyncTemplateMethods<${className}>;

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/S3ClientSource.vm‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -356,15 +356,4 @@ bool ${className}::MultipartUploadSupported() const
356356
{
357357
return true;
358358
}
359-
#if($serviceNamespace == "S3Crt")
360-
361-
void ${className}::AddContentLengthToRequest(const std::shared_ptr<Aws::Http::HttpRequest>& httpRequest,
362-
const std::shared_ptr<Aws::IOStream>& body,
363-
bool isChunked) const
364-
{
365-
if (m_clientConfiguration.contentLenghtConfiguration == ${className}Configuration::CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM) {
366-
BASECLASS::AddContentLengthToRequest(httpRequest, body, isChunked);
367-
}
368-
}
369-
#end
370359
#end

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtClientConfigHeader.vm‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,4 @@
9393
* This setting maps to CRT's network_interface_names_array config.
9494
*/
9595
Aws::Vector<Aws::String> networkInterfaceNames;
96-
97-
/**
98-
* Configuration on how to handle missing content length. By default the SDK attempts to seek the stream
99-
* to find the total length of the stream before streaming. CRT will fallback to multipart upload if there
100-
* is no content length, creating multipart uploads of partSize before knowing the stream has ended.
101-
*/
102-
enum class CONTENT_LENGTH_CONFIGURATION {
103-
SEEK_STREAM,
104-
MULTIPART_UPLOAD,
105-
} contentLenghtConfiguration {CONTENT_LENGTH_CONFIGURATION::SEEK_STREAM};
10696
/* End of S3 CRT specifics */

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtServiceClientSourceInit.vm‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,12 @@ ${className}::${className}(const ${clientConfigurationNamespace}::ClientConfigur
132132
false),
133133
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
134134
#if($serviceModel.endpointRules)
135-
m_clientConfiguration(clientConfiguration),
135+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
136136
${defaultCredentialsProviderChainMember},
137137
#else${defaultCredentialsProviderChainMember}${virtualAddressingInit}${USEast1RegionalEndpointInitString},
138138
#end
139139
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
140140
{
141-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
142-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
143141
init(clientConfiguration${credentialsParam});
144142
}
145143

@@ -154,14 +152,12 @@ ${className}::${className}(const AWSCredentials& credentials, const ${clientConf
154152
false),
155153
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
156154
#if($serviceModel.endpointRules)
157-
m_clientConfiguration(clientConfiguration),
155+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
158156
${simpleCredentialsProviderMember},
159157
#else${simpleCredentialsProviderMember}${virtualAddressingInit}${USEast1RegionalEndpointInitString},
160158
#end
161159
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
162160
{
163-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
164-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
165161
init(clientConfiguration${credentialsParam});
166162
}
167163

@@ -177,15 +173,13 @@ ${className}::${className}(const std::shared_ptr<AWSCredentialsProvider>& creden
177173
false),
178174
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
179175
#if($serviceModel.endpointRules)
180-
m_clientConfiguration(clientConfiguration${signPayloadsParam}),
176+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
181177
${credentialsProviderMember},
182178
#else
183179
${credentialsProviderMember}${virtualAddressingInit}${USEast1RegionalEndpointInitString},
184180
#end
185181
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
186182
{
187-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
188-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
189183
init(clientConfiguration${credentialsParam});
190184
}
191185
#else
@@ -195,15 +189,13 @@ ${className}::${className}(const ${clientConfigurationNamespace}::ClientConfigur
195189
SERVICE_NAME, Aws::Region::ComputeSignerRegion(clientConfiguration.region)${signPayloadsParam}${doubleEncodeValue}),
196190
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
197191
#if($serviceModel.endpointRules)
198-
m_clientConfiguration(clientConfiguration${signPayloadsParam}),
192+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
199193
${defaultCredentialsProviderChainMember},
200194
#else
201195
${defaultCredentialsProviderChainMember}${virtualAddressingInit}${USEast1RegionalEndpointInitString},
202196
#end
203197
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
204198
{
205-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
206-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
207199
init(clientConfiguration${credentialsParam});
208200
}
209201

@@ -213,15 +205,13 @@ ${className}::${className}(const AWSCredentials& credentials, const ${clientConf
213205
SERVICE_NAME, Aws::Region::ComputeSignerRegion(clientConfiguration.region)${signPayloadsParam}${doubleEncodeValue}),
214206
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
215207
#if($serviceModel.endpointRules)
216-
m_clientConfiguration(clientConfiguration),
208+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
217209
${simpleCredentialsProviderMember},
218210
#else
219211
${simpleCredentialsProviderMember}${virtualAddressingInit}${USEast1RegionalEndpointInitString},
220212
#end
221213
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
222214
{
223-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
224-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
225215
init(clientConfiguration${credentialsParam});
226216
}
227217

@@ -232,15 +222,13 @@ ${className}::${className}(const std::shared_ptr<AWSCredentialsProvider>& creden
232222
SERVICE_NAME, Aws::Region::ComputeSignerRegion(clientConfiguration.region)${signPayloadsParam}${doubleEncodeValue}),
233223
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
234224
#if($serviceModel.endpointRules)
235-
m_clientConfiguration(clientConfiguration),
225+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
236226
${virtualAddressingInit},
237227
#else
238228
${virtualAddressingInit}${USEast1RegionalEndpointInitString}${credentialsProviderMember},
239229
#end
240230
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
241231
{
242-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
243-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
244232
init(clientConfiguration${credentialsParam});
245233
}
246234
#end
@@ -251,15 +239,13 @@ ${className}::${className}(const std::shared_ptr<Aws::Auth::AWSAuthSignerProvide
251239
BASECLASS(clientConfiguration, signerProvider,
252240
Aws::MakeShared<${metadata.classNamePrefix}ErrorMarshaller>(ALLOCATION_TAG)),
253241
#if($serviceModel.endpointRules)
254-
m_clientConfiguration(clientConfiguration),
242+
m_clientConfiguration(clientConfiguration${signPayloadsParam}${virtualAddressingInit}${USEast1RegionalEndpointInitString}),
255243
${virtualAddressingInit},
256244
#else
257245
${virtualAddressingInit},
258246
#end
259247
m_identityProvider(Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this))
260248
{
261-
AWS_UNREFERENCED_PARAM(useVirtualAddressing);
262-
AWS_UNREFERENCED_PARAM(USEast1RegionalEndPointOption);
263249
init(m_clientConfiguration);
264250
}
265251

0 commit comments

Comments
 (0)