Skip to content

Commit 6f6b084

Browse files
kai-ionkai-ion
committed
refactor: centralize aws-chunked encoding in ChunkingInterceptor
Move aws-chunked encoding logic from individual HTTP clients to a centralized ChunkingInterceptor for better separation of concerns. - Add ChunkingInterceptor to handle aws-chunked encoding at request level - Remove custom chunking logic from CRT, Curl, and Windows HTTP clients - Simplify HTTP clients to focus on transport-only responsibilities - Maintain full backwards compatibility with existing APIs unit test for chunking stream added logic to detect custom http client and smart default
1 parent e03a5aa commit 6f6b084

File tree

18 files changed

+467
-95
lines changed

18 files changed

+467
-95
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ namespace Aws
7878
WHEN_REQUIRED,
7979
};
8080

81+
/**
82+
* Control HTTP client chunking implementation mode.
83+
* DEFAULT: Use SDK's ChunkingInterceptor for aws-chunked encoding
84+
* CLIENT_IMPLEMENTATION: Rely on HTTP client's native chunking (default for custom clients)
85+
*/
86+
enum class HttpClientChunkedMode {
87+
DEFAULT,
88+
CLIENT_IMPLEMENTATION,
89+
};
90+
8191
struct RequestCompressionConfig {
8292
UseRequestCompression useRequestCompression=UseRequestCompression::ENABLE;
8393
size_t requestMinCompressionSizeBytes = 10240;
@@ -493,6 +503,12 @@ namespace Aws
493503
* https://docs.aws.amazon.com/sdkref/latest/guide/feature-account-endpoints.html
494504
*/
495505
Aws::String accountIdEndpointMode = "preferred";
506+
507+
/**
508+
* Control HTTP client chunking implementation mode.
509+
* Default is set automatically: CLIENT_IMPLEMENTATION for custom clients, DEFAULT for AWS clients.
510+
*/
511+
HttpClientChunkedMode httpClientChunkedMode = HttpClientChunkedMode::CLIENT_IMPLEMENTATION;
496512
/**
497513
* Configuration structure for credential providers in the AWS SDK.
498514
* This structure allows passing configuration options to credential providers

‎src/aws-cpp-sdk-core/include/aws/core/http/HttpClient.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ namespace Aws
4848
*/
4949
virtual bool SupportsChunkedTransferEncoding() const { return true; }
5050

51+
/**
52+
* Returns true if this is a default AWS SDK HTTP client implementation.
53+
*/
54+
virtual bool IsDefaultAwsHttpClient() const { return false; }
55+
5156
/**
5257
* Stops all requests in progress and prevents any others from initiating.
5358
*/

‎src/aws-cpp-sdk-core/include/aws/core/http/crt/CRTHttpClient.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ namespace Aws
5252
Aws::Utils::RateLimits::RateLimiterInterface* readLimiter,
5353
Aws::Utils::RateLimits::RateLimiterInterface* writeLimiter) const override;
5454

55+
bool IsDefaultAwsHttpClient() const override { return true; }
56+
5557
private:
5658
// Yeah, I know, but someone made MakeRequest() const and didn't think about the fact that
5759
// making an HTTP request most certainly mutates state. It was me. I'm the person that did that, and

‎src/aws-cpp-sdk-core/include/aws/core/http/curl/CurlHttpClient.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ class AWS_CORE_API CurlHttpClient: public HttpClient
3737
Aws::Utils::RateLimits::RateLimiterInterface* readLimiter = nullptr,
3838
Aws::Utils::RateLimits::RateLimiterInterface* writeLimiter = nullptr) const override;
3939

40+
bool IsDefaultAwsHttpClient() const override { return true; }
41+
4042
static void InitGlobalState();
4143
static void CleanupGlobalState();
4244

‎src/aws-cpp-sdk-core/include/aws/core/http/windows/IXmlHttpRequest2HttpClient.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ namespace Aws
5454
*/
5555
virtual bool SupportsChunkedTransferEncoding() const override { return false; }
5656

57+
bool IsDefaultAwsHttpClient() const override { return true; }
58+
5759
protected:
5860
/**
5961
* Override any configuration on request handle.

‎src/aws-cpp-sdk-core/include/aws/core/http/windows/WinHttpSyncHttpClient.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ namespace Aws
4242
*/
4343
const char* GetLogTag() const override { return "WinHttpSyncHttpClient"; }
4444

45+
bool IsDefaultAwsHttpClient() const override { return true; }
46+
4547
private:
4648
// WinHttp specific implementations
4749
void* OpenRequest(const std::shared_ptr<HttpRequest>& request, void* connection, const Aws::StringStream& ss) const override;

‎src/aws-cpp-sdk-core/include/aws/core/http/windows/WinINetSyncHttpClient.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ namespace Aws
3939
* Gets log tag for use in logging in the base class.
4040
*/
4141
const char* GetLogTag() const override { return "WinInetSyncHttpClient"; }
42+
43+
bool IsDefaultAwsHttpClient() const override { return true; }
4244
private:
4345

4446
// WinHttp specific implementations

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <smithy/tracing/TelemetryProvider.h>
1010
#include <smithy/interceptor/Interceptor.h>
1111
#include <smithy/client/features/ChecksumInterceptor.h>
12+
#include <smithy/client/features/ChunkingInterceptor.h>
1213
#include <smithy/client/features/UserAgentInterceptor.h>
1314

1415
#include <aws/crt/Variant.h>
@@ -20,6 +21,7 @@
2021
#include <aws/core/utils/Outcome.h>
2122
#include <aws/core/NoResult.h>
2223
#include <aws/core/http/HttpClientFactory.h>
24+
#include <aws/core/http/HttpClient.h>
2325
#include <aws/core/client/AWSErrorMarshaller.h>
2426
#include <aws/core/AmazonWebServiceResult.h>
2527
#include <utility>
@@ -98,9 +100,19 @@ namespace client
98100
m_serviceName(std::move(serviceName)),
99101
m_serviceUserAgentName(std::move(serviceUserAgentName)),
100102
m_httpClient(std::move(httpClient)),
101-
m_errorMarshaller(std::move(errorMarshaller)),
102-
m_interceptors{Aws::MakeShared<ChecksumInterceptor>("AwsSmithyClientBase", *m_clientConfig)}
103+
m_errorMarshaller(std::move(errorMarshaller))
103104
{
105+
// Create modified config for chunking interceptor
106+
Aws::Client::ClientConfiguration chunkingConfig(*m_clientConfig);
107+
if (!m_httpClient->IsDefaultAwsHttpClient()) {
108+
chunkingConfig.httpClientChunkedMode = Aws::Client::HttpClientChunkedMode::CLIENT_IMPLEMENTATION;
109+
}
110+
111+
m_interceptors = {
112+
Aws::MakeShared<ChecksumInterceptor>("AwsSmithyClientBase", *m_clientConfig),
113+
Aws::MakeShared<features::ChunkingInterceptor>("AwsSmithyClientBase", chunkingConfig)
114+
};
115+
104116
baseInit();
105117
}
106118

‎src/aws-cpp-sdk-core/include/smithy/client/features/ChunkingInterceptor.h‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
/**
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0.
4+
*/
5+
#pragma once
6+
#include <aws/core/http/HttpRequest.h>
7+
#include <aws/core/utils/Array.h>
8+
#include <aws/core/utils/StringUtils.h>
9+
#include <aws/core/utils/HashingUtils.h>
10+
#include <aws/core/utils/logging/LogMacros.h>
11+
#include <aws/core/utils/memory/stl/AWSStringStream.h>
12+
#include <smithy/interceptor/Interceptor.h>
13+
#include <aws/core/client/ClientConfiguration.h>
14+
#include <aws/core/utils/Outcome.h>
15+
#include <aws/core/client/AWSError.h>
16+
#include <memory>
17+
18+
namespace smithy {
19+
namespace client {
20+
namespace features {
21+
22+
static const size_t AWS_DATA_BUFFER_SIZE = 65536;
23+
static const char* ALLOCATION_TAG = "ChunkingInterceptor";
24+
static const char* CHECKSUM_HEADER_PREFIX = "x-amz-checksum-";
25+
26+
template <size_t DataBufferSize = AWS_DATA_BUFFER_SIZE>
27+
class AwsChunkedStreamBuf : public std::streambuf {
28+
public:
29+
AwsChunkedStreamBuf(Aws::Http::HttpRequest* request,
30+
const std::shared_ptr<Aws::IOStream>& stream,
31+
size_t bufferSize = DataBufferSize)
32+
: m_chunkingStream(Aws::MakeShared<Aws::StringStream>(ALLOCATION_TAG)),
33+
m_request(request),
34+
m_stream(stream),
35+
m_data(bufferSize)
36+
{
37+
assert(m_stream != nullptr);
38+
if (m_stream == nullptr) {
39+
AWS_LOGSTREAM_ERROR("AwsChunkedStream", "stream is null");
40+
}
41+
assert(m_request != nullptr);
42+
if (m_request == nullptr) {
43+
AWS_LOGSTREAM_ERROR("AwsChunkedStream", "request is null");
44+
}
45+
46+
setg(nullptr, nullptr, nullptr);
47+
}
48+
49+
protected:
50+
int_type underflow() override {
51+
if (gptr() && gptr() < egptr()) {
52+
return traits_type::to_int_type(*gptr());
53+
}
54+
55+
// only read and write to chunked stream if the underlying stream
56+
// is still in a valid state
57+
if (m_stream->good()) {
58+
// Try to read in a 64K chunk, if we cant we know the stream is over
59+
m_stream->read(m_data.GetUnderlyingData(), m_data.GetLength());
60+
size_t bytesRead = static_cast<size_t>(m_stream->gcount());
61+
writeChunk(bytesRead);
62+
63+
// if we've read everything from the stream, we want to add the trailer
64+
// to the underlying stream
65+
if ((m_stream->peek() == EOF || m_stream->eof()) && !m_stream->bad()) {
66+
writeTrailerToUnderlyingStream();
67+
}
68+
}
69+
70+
// if the underlying stream is empty there is nothing to read
71+
if ((m_chunkingStream->peek() == EOF || m_chunkingStream->eof()) && !m_chunkingStream->bad()) {
72+
return traits_type::eof();
73+
}
74+
75+
// Read from chunking stream to internal buffer
76+
m_chunkingStream->read(m_buffer.GetUnderlyingData(), m_buffer.GetLength());
77+
size_t bytesRead = static_cast<size_t>(m_chunkingStream->gcount());
78+
if (bytesRead == 0) {
79+
return traits_type::eof();
80+
}
81+
82+
setg(m_buffer.GetUnderlyingData(), m_buffer.GetUnderlyingData(), m_buffer.GetUnderlyingData() + bytesRead);
83+
return traits_type::to_int_type(*gptr());
84+
}
85+
86+
private:
87+
void writeTrailerToUnderlyingStream() {
88+
Aws::StringStream chunkedTrailerStream;
89+
chunkedTrailerStream << "0\r\n";
90+
if (m_request->GetRequestHash().second != nullptr) {
91+
chunkedTrailerStream << "x-amz-checksum-" << m_request->GetRequestHash().first << ":"
92+
<< Aws::Utils::HashingUtils::Base64Encode(m_request->GetRequestHash().second->GetHash().GetResult()) << "\r\n";
93+
}
94+
chunkedTrailerStream << "\r\n";
95+
const auto chunkedTrailer = chunkedTrailerStream.str();
96+
if (m_chunkingStream->eof()) {
97+
m_chunkingStream->clear();
98+
}
99+
*m_chunkingStream << chunkedTrailer;
100+
}
101+
102+
void writeChunk(size_t bytesRead) {
103+
if (m_request->GetRequestHash().second != nullptr) {
104+
m_request->GetRequestHash().second->Update(reinterpret_cast<unsigned char*>(m_data.GetUnderlyingData()), bytesRead);
105+
}
106+
107+
if (bytesRead > 0 && m_chunkingStream && !m_chunkingStream->bad()) {
108+
if (m_chunkingStream->eof()) {
109+
m_chunkingStream->clear();
110+
}
111+
*m_chunkingStream << Aws::Utils::StringUtils::ToHexString(bytesRead) << "\r\n";
112+
m_chunkingStream->write(m_data.GetUnderlyingData(), bytesRead);
113+
*m_chunkingStream << "\r\n";
114+
}
115+
}
116+
117+
std::shared_ptr<Aws::IOStream> m_chunkingStream;
118+
Aws::Http::HttpRequest* m_request{nullptr};
119+
std::shared_ptr<Aws::IOStream> m_stream;
120+
Aws::Utils::Array<char> m_data;
121+
Aws::Utils::Array<char> m_buffer{DataBufferSize};
122+
};
123+
124+
class AwsChunkedIOStream : public Aws::IOStream {
125+
public:
126+
AwsChunkedIOStream(Aws::Http::HttpRequest* request,
127+
const std::shared_ptr<Aws::IOStream>& originalBody,
128+
size_t bufferSize = AWS_DATA_BUFFER_SIZE)
129+
: Aws::IOStream(&m_buf),
130+
m_buf(request, originalBody, bufferSize) {}
131+
132+
private:
133+
AwsChunkedStreamBuf<> m_buf;
134+
};
135+
136+
/**
137+
* Interceptor that handles chunked encoding for streaming requests with checksums.
138+
* Wraps request body with chunked stream and sets appropriate headers.
139+
*/
140+
class ChunkingInterceptor : public smithy::interceptor::Interceptor {
141+
public:
142+
explicit ChunkingInterceptor(const Aws::Client::ClientConfiguration& config)
143+
: m_httpClientChunkedMode(config.httpClientChunkedMode) {}
144+
~ChunkingInterceptor() override = default;
145+
146+
ModifyRequestOutcome ModifyBeforeSigning(smithy::interceptor::InterceptorContext& context) override {
147+
auto request = context.GetTransmitRequest();
148+
149+
if (!ShouldApplyChunking(request)) {
150+
return request;
151+
}
152+
153+
auto originalBody = request->GetContentBody();
154+
if (!originalBody) {
155+
return request;
156+
}
157+
158+
// Set up chunked encoding headers for checksum calculation
159+
const auto& hashPair = request->GetRequestHash();
160+
if (hashPair.second != nullptr) {
161+
Aws::String checksumHeaderValue = Aws::String(CHECKSUM_HEADER_PREFIX) + hashPair.first;
162+
request->DeleteHeader(checksumHeaderValue.c_str());
163+
request->SetHeaderValue(Aws::Http::AWS_TRAILER_HEADER, checksumHeaderValue);
164+
request->SetTransferEncoding(Aws::Http::CHUNKED_VALUE);
165+
166+
if (!request->HasContentEncoding()) {
167+
request->SetContentEncoding(Aws::Http::AWS_CHUNKED_VALUE);
168+
} else {
169+
Aws::String currentEncoding = request->GetContentEncoding();
170+
if (currentEncoding.find(Aws::Http::AWS_CHUNKED_VALUE) == Aws::String::npos) {
171+
request->SetContentEncoding(Aws::String{Aws::Http::AWS_CHUNKED_VALUE} + "," + currentEncoding);
172+
}
173+
}
174+
175+
if (request->HasHeader(Aws::Http::CONTENT_LENGTH_HEADER)) {
176+
request->SetHeaderValue(Aws::Http::DECODED_CONTENT_LENGTH_HEADER, request->GetHeaderValue(Aws::Http::CONTENT_LENGTH_HEADER));
177+
request->DeleteHeader(Aws::Http::CONTENT_LENGTH_HEADER);
178+
}
179+
}
180+
181+
auto chunkedBody = Aws::MakeShared<AwsChunkedIOStream>(
182+
ALLOCATION_TAG, request.get(), originalBody);
183+
184+
request->AddContentBody(chunkedBody);
185+
return request;
186+
}
187+
188+
ModifyResponseOutcome ModifyBeforeDeserialization(smithy::interceptor::InterceptorContext& context) override {
189+
return context.GetTransmitResponse();
190+
}
191+
192+
private:
193+
bool ShouldApplyChunking(const std::shared_ptr<Aws::Http::HttpRequest>& request) const {
194+
// Use configuration setting to determine chunking behavior
195+
if (m_httpClientChunkedMode != Aws::Client::HttpClientChunkedMode::DEFAULT) {
196+
return false;
197+
}
198+
199+
if (!request || !request->GetContentBody()) {
200+
return false;
201+
}
202+
203+
// Check if request has checksum requirements
204+
const auto& hashPair = request->GetRequestHash();
205+
return hashPair.second != nullptr;
206+
}
207+
208+
Aws::Client::HttpClientChunkedMode m_httpClientChunkedMode;
209+
};
210+
211+
} // namespace features
212+
} // namespace client
213+
} // namespace smithy

‎src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -218,26 +218,10 @@ bool AWSAuthV4Signer::SignRequestWithCreds(Aws::Http::HttpRequest& request, cons
218218
request.SetAwsSessionToken(credentials.GetSessionToken());
219219
}
220220

221-
// If the request checksum, set the signer to use a unsigned
222-
// trailing payload. otherwise use it in the header
223-
if (request.GetRequestHash().second != nullptr && !request.GetRequestHash().first.empty() && request.GetContentBody() != nullptr) {
224-
AWS_LOGSTREAM_DEBUG(v4LogTag, "Note: Http payloads are not being signed. signPayloads="
225-
<< signBody << " http scheme=" << Http::SchemeMapper::ToString(request.GetUri().GetScheme()));
226-
if (request.GetRequestHash().second != nullptr) {
221+
// If the request has checksum and chunking was applied by interceptor, use streaming payload
222+
if (request.GetRequestHash().second != nullptr && !request.GetRequestHash().first.empty() &&
223+
request.GetContentBody() != nullptr && request.HasHeader(Http::AWS_TRAILER_HEADER)) {
227224
payloadHash = STREAMING_UNSIGNED_PAYLOAD_TRAILER;
228-
Aws::String checksumHeaderValue = Aws::String("x-amz-checksum-") + request.GetRequestHash().first;
229-
request.DeleteHeader(checksumHeaderValue.c_str());
230-
request.SetHeaderValue(Http::AWS_TRAILER_HEADER, checksumHeaderValue);
231-
request.SetTransferEncoding(CHUNKED_VALUE);
232-
request.HasContentEncoding()
233-
? request.SetContentEncoding(Aws::String{Http::AWS_CHUNKED_VALUE} + "," + request.GetContentEncoding())
234-
: request.SetContentEncoding(Http::AWS_CHUNKED_VALUE);
235-
236-
if (request.HasHeader(Http::CONTENT_LENGTH_HEADER)) {
237-
request.SetHeaderValue(Http::DECODED_CONTENT_LENGTH_HEADER, request.GetHeaderValue(Http::CONTENT_LENGTH_HEADER));
238-
request.DeleteHeader(Http::CONTENT_LENGTH_HEADER);
239-
}
240-
}
241225
} else {
242226
payloadHash = ComputePayloadHash(request);
243227
if (payloadHash.empty()) {

0 commit comments

Comments
 (0)