refactor out chunk framing

kai-ion · kai-ion · commit a6b040e0b1fc · 2025-12-02T14:53:50.000-05:00
squash 083ea7a # refactor out chunk framing
diff --git a/src/aws-cpp-sdk-core/CMakeLists.txt b/src/aws-cpp-sdk-core/CMakeLists.txt
@@ -51,7 +51,7 @@ file(GLOB AWS_INTERNAL_HEADERS "include/aws/core/internal/*.h")
 file(GLOB NET_HEADERS "include/aws/core/net/*.h")
 file(GLOB HTTP_HEADERS "include/aws/core/http/*.h")
 file(GLOB HTTP_STANDARD_HEADERS "include/aws/core/http/standard/*.h")
-    file(GLOB HTTP_INTERCEPTOR_HEADERS "include/aws/core/http/interceptor/*.h")
+file(GLOB HTTP_INTERCEPTOR_HEADERS "include/aws/core/http/interceptor/*.h")
 file(GLOB CONFIG_HEADERS "include/aws/core/config/*.h")
 file(GLOB CONFIG_DEFAULTS_HEADERS "include/aws/core/config/defaults/*.h")
 file(GLOB ENDPOINT_HEADERS "include/aws/core/endpoint/*.h")
@@ -470,7 +470,6 @@ if(MSVC)
     source_group("Source Files\\net\\windows" FILES ${NET_SOURCE})
     source_group("Source Files\\http" FILES ${HTTP_SOURCE})
     source_group("Source Files\\http\\standard" FILES ${HTTP_STANDARD_SOURCE})
-
     source_group("Source Files\\http\\interceptor" FILES ${HTTP_INTERCEPTOR_SOURCE})
     source_group("Source Files\\config" FILES ${CONFIG_SOURCE})
     source_group("Source Files\\config\\defaults" FILES ${CONFIG_DEFAULTS_SOURCE})
diff --git a/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h b/src/aws-cpp-sdk-core/include/aws/core/client/ClientConfiguration.h
@@ -78,6 +78,16 @@ namespace Aws
           WHEN_REQUIRED,
         };
 
+        /**
+         * Control whether ChunkingInterceptor applies aws-chunked encoding.
+         * ENABLE: Apply aws-chunked encoding for requests with checksums (default)
+         * DISABLE: Skip ChunkingInterceptor, rely on signer for chunking
+         */
+        enum class UseAwsChunkedEncoding {
+          ENABLE,
+          DISABLE,
+        };
+
         struct RequestCompressionConfig {
           UseRequestCompression useRequestCompression=UseRequestCompression::ENABLE;
           size_t requestMinCompressionSizeBytes = 10240;
@@ -493,6 +503,12 @@ namespace Aws
            * https://docs.aws.amazon.com/sdkref/latest/guide/feature-account-endpoints.html
            */
           Aws::String accountIdEndpointMode = "preferred";
+
+          /**
+           * Control whether ChunkingInterceptor applies aws-chunked encoding.
+           * Default ENABLE for backward compatibility.
+           */
+          UseAwsChunkedEncoding useAwsChunkedEncoding = UseAwsChunkedEncoding::ENABLE;
           /**
           * Configuration structure for credential providers in the AWS SDK.
           * This structure allows passing configuration options to credential providers
diff --git a/src/aws-cpp-sdk-core/include/aws/core/http/interceptor/ChunkingInterceptor.h b/src/aws-cpp-sdk-core/include/aws/core/http/interceptor/ChunkingInterceptor.h
@@ -19,14 +19,16 @@ namespace Aws
  */
 class ChunkingInterceptor : public smithy::interceptor::Interceptor {
 public:
-    ChunkingInterceptor() = default;
+    explicit ChunkingInterceptor(Aws::Client::UseAwsChunkedEncoding useAwsChunkedEncoding = Aws::Client::UseAwsChunkedEncoding::ENABLE)
+        : m_useAwsChunkedEncoding(useAwsChunkedEncoding) {}
     ~ChunkingInterceptor() override = default;
 
     ModifyRequestOutcome ModifyBeforeSigning(smithy::interceptor::InterceptorContext& context) override;
     ModifyResponseOutcome ModifyBeforeDeserialization(smithy::interceptor::InterceptorContext& context) override;
 
 private:
     bool ShouldApplyAwsChunking(const std::shared_ptr<Aws::Http::HttpRequest>& request) const;
+    Aws::Client::UseAwsChunkedEncoding m_useAwsChunkedEncoding;
 };
 
     } // namespace Http
diff --git a/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinHttpSyncHttpClient.h b/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinHttpSyncHttpClient.h
@@ -51,7 +51,6 @@ namespace Aws
             bool DoReceiveResponse(void* httpRequest) const override;
             bool DoQueryHeaders(void* httpRequest, std::shared_ptr<Aws::Http::HttpResponse>& response, Aws::StringStream& ss, uint64_t& read) const override;
             bool DoSendRequest(void* httpRequest) const override;
-            bool DoSendRequest(void* httpRequest, const std::shared_ptr<HttpRequest>& request) const;
             bool DoQueryDataAvailable(void* hHttpRequest, uint64_t& available) const override;
             bool DoReadData(void* hHttpRequest, char* body, uint64_t size, uint64_t& read) const override;
             void* GetClientModule() const override;
diff --git a/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinINetSyncHttpClient.h b/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinINetSyncHttpClient.h
@@ -49,7 +49,6 @@ namespace Aws
             bool DoReceiveResponse(void* hHttpRequest) const override;
             bool DoQueryHeaders(void* hHttpRequest, std::shared_ptr<Aws::Http::HttpResponse>& response, Aws::StringStream& ss, uint64_t& read) const override;
             bool DoSendRequest(void* hHttpRequest) const override;
-            bool DoSendRequest(void* hHttpRequest, const std::shared_ptr<HttpRequest>& request) const override;
             bool DoQueryDataAvailable(void* hHttpRequest, uint64_t& available) const override;
             bool DoReadData(void* hHttpRequest, char* body, uint64_t size, uint64_t& read) const override;
             void* GetClientModule() const override;
diff --git a/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinSyncHttpClient.h b/src/aws-cpp-sdk-core/include/aws/core/http/windows/WinSyncHttpClient.h
@@ -11,7 +11,6 @@
 #include <aws/core/auth/AWSAuthSigner.h>
 #include <aws/core/http/HttpClient.h>
 #include <aws/core/http/standard/StandardHttpResponse.h>
-#include <aws/core/utils/UnreferencedParam.h>
 
 namespace Aws
 {
@@ -93,10 +92,6 @@ namespace Aws
             virtual bool DoReceiveResponse(void* hHttpRequest) const = 0;
             virtual bool DoQueryHeaders(void* hHttpRequest, std::shared_ptr<Aws::Http::HttpResponse>& response, Aws::StringStream& ss, uint64_t& read) const = 0;
             virtual bool DoSendRequest(void* hHttpRequest) const = 0;
-            virtual bool DoSendRequest(void* hHttpRequest, const std::shared_ptr<HttpRequest>& request) const { 
-                AWS_UNREFERENCED_PARAM(request);
-                return DoSendRequest(hHttpRequest); 
-            }
             virtual bool DoQueryDataAvailable(void* hHttpRequest, uint64_t& available) const = 0;
             virtual bool DoReadData(void* hHttpRequest, char* body, uint64_t size, uint64_t& read) const = 0;
             virtual void* GetClientModule() const = 0;
diff --git a/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp b/src/aws-cpp-sdk-core/source/auth/signer/AWSAuthV4Signer.cpp
@@ -229,14 +229,13 @@ bool AWSAuthV4Signer::SignRequestWithCreds(Aws::Http::HttpRequest& request, cons
         request.DeleteHeader(checksumHeaderValue.c_str());
         request.SetHeaderValue(Http::AWS_TRAILER_HEADER, checksumHeaderValue);
         request.SetTransferEncoding(CHUNKED_VALUE);
-        // Only add aws-chunked if it's not already present to avoid duplication
-        if (request.HasContentEncoding()) {
-            const auto& currentEncoding = request.GetContentEncoding();
+        if (!request.HasContentEncoding()) {
+            request.SetContentEncoding(Http::AWS_CHUNKED_VALUE);
+        } else {
+            Aws::String currentEncoding = request.GetContentEncoding();
             if (currentEncoding.find(Http::AWS_CHUNKED_VALUE) == Aws::String::npos) {
                 request.SetContentEncoding(Aws::String{Http::AWS_CHUNKED_VALUE} + "," + currentEncoding);
             }
-        } else {
-            request.SetContentEncoding(Http::AWS_CHUNKED_VALUE);
         }
 
         if (request.HasHeader(Http::CONTENT_LENGTH_HEADER)) {
diff --git a/src/aws-cpp-sdk-core/source/client/UserAgent.cpp b/src/aws-cpp-sdk-core/source/client/UserAgent.cpp
@@ -182,6 +182,15 @@ Aws::String UserAgent::SerializeWithFeatures(const Aws::Set<UserAgentFeature>& f
     SerializeMetadata(METADATA, m_compilerMetadata);
   }
 
+  // Add HTTP client metadata
+#if AWS_SDK_USE_CRT_HTTP
+  SerializeMetadata(METADATA, "http#crt");
+#elif ENABLE_CURL_CLIENT
+  SerializeMetadata(METADATA, "http#curl");
+#elif ENABLE_WINDOWS_CLIENT
+  SerializeMetadata(METADATA, "http#winhttp");
+#endif
+
   // metrics
   Aws::Vector<Aws::String> encodedMetrics{};
 
diff --git a/src/aws-cpp-sdk-core/source/http/interceptor/ChunkingInterceptor.cpp b/src/aws-cpp-sdk-core/source/http/interceptor/ChunkingInterceptor.cpp
@@ -51,6 +51,11 @@ class AwsChunkedIOStream : public Aws::IOStream {
 ChunkingInterceptor::ModifyRequestOutcome ChunkingInterceptor::ModifyBeforeSigning(
     smithy::interceptor::InterceptorContext& context) {
     auto request = context.GetTransmitRequest();
+
+    if (m_useAwsChunkedEncoding == Aws::Client::UseAwsChunkedEncoding::DISABLE) {
+        return request;
+    }
+    
     if (!ShouldApplyAwsChunking(request)) {
         return request;
     }
@@ -65,8 +70,10 @@ ChunkingInterceptor::ModifyRequestOutcome ChunkingInterceptor::ModifyBeforeSigni
 
     request->AddContentBody(chunkedBody);
     
-    // Set Transfer-Encoding: chunked for aws-chunked requests
-    request->SetTransferEncoding("chunked");
+    // Set Transfer-Encoding: chunked for aws-chunked requests (only if not already set)
+    if (!request->HasTransferEncoding()) {
+        request->SetTransferEncoding("chunked");
+    }
     
     // Move Content-Length to x-amz-decoded-content-length and remove Content-Length
     if (request->HasHeader("Content-Length")) {
diff --git a/src/aws-cpp-sdk-core/source/http/windows/WinHttpSyncHttpClient.cpp b/src/aws-cpp-sdk-core/source/http/windows/WinHttpSyncHttpClient.cpp
@@ -735,25 +735,6 @@ bool WinHttpSyncHttpClient::DoSendRequest(void* hHttpRequest) const
     return success;
 }
 
-bool WinHttpSyncHttpClient::DoSendRequest(void* hHttpRequest, const std::shared_ptr<HttpRequest>& request) const
-{
-    // Check if this is a Transfer-Encoding: chunked request
-    if (request->HasTransferEncoding() && request->GetTransferEncoding().find("chunked") != Aws::String::npos) {
-        // For chunked requests, use WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH
-        bool success = WinHttpSendRequest(hHttpRequest, NULL, 0, NULL, 0, 0xFFFFFFFF, 0) != 0;
-        if (!success)
-        {
-            DWORD lastError = GetLastError();
-            AWS_LOGSTREAM_ERROR(GetLogTag(), "WinHttpSendRequest failed for chunked request. Error: " << lastError << " (0x" << std::hex << lastError << std::dec << ")");
-            AzWinHttpLogLastError("WinHttpSendRequest");
-        }
-        return success;
-    } else {
-        // For regular requests, use the original method
-        return DoSendRequest(hHttpRequest);
-    }
-}
-
 bool WinHttpSyncHttpClient::DoQueryDataAvailable(void* hHttpRequest, uint64_t& available) const
 {
     return (WinHttpQueryDataAvailable(hHttpRequest, (LPDWORD)&available) != 0);
diff --git a/src/aws-cpp-sdk-core/source/http/windows/WinINetSyncHttpClient.cpp b/src/aws-cpp-sdk-core/source/http/windows/WinINetSyncHttpClient.cpp
@@ -161,19 +161,52 @@ void WinINetSyncHttpClient::DoAddHeaders(void* hHttpRequest, Aws::String& header
 
 uint64_t WinINetSyncHttpClient::DoWriteData(void* hHttpRequest, char* streamBuffer, uint64_t bytesRead, bool isChunked) const
 {
-    AWS_UNREFERENCED_PARAM(isChunked);
     DWORD bytesWritten = 0;
-    if (!InternetWriteFile(hHttpRequest, streamBuffer, (DWORD)bytesRead, &bytesWritten))
+    uint64_t totalBytesWritten = 0;
+    const char CRLF[] = "\r\n";
+
+    if (isChunked)
     {
-        return 0;
+        Aws::String chunkSizeHexString = StringUtils::ToHexString(bytesRead) + CRLF;
+
+        if (!InternetWriteFile(hHttpRequest, chunkSizeHexString.c_str(), (DWORD)chunkSizeHexString.size(), &bytesWritten))
+        {
+            return totalBytesWritten;
+        }
+        totalBytesWritten += bytesWritten;
+        if (!InternetWriteFile(hHttpRequest, streamBuffer, (DWORD)bytesRead, &bytesWritten))
+        {
+            return totalBytesWritten;
+        }
+        totalBytesWritten += bytesWritten;
+        if (!InternetWriteFile(hHttpRequest, CRLF, (DWORD)(sizeof(CRLF) - 1), &bytesWritten))
+        {
+            return totalBytesWritten;
+        }
+        totalBytesWritten += bytesWritten;
     }
-    return bytesWritten;
+    else
+    {
+        if (!InternetWriteFile(hHttpRequest, streamBuffer, (DWORD)bytesRead, &bytesWritten))
+        {
+            return totalBytesWritten;
+        }
+        totalBytesWritten += bytesWritten;
+    }
+
+    return totalBytesWritten;
 }
 
 uint64_t WinINetSyncHttpClient::FinalizeWriteData(void* hHttpRequest) const
 {
-    AWS_UNREFERENCED_PARAM(hHttpRequest);
-    return 0;
+    DWORD bytesWritten = 0;
+    const char trailingCRLF[] = "0\r\n\r\n";
+    if (!InternetWriteFile(hHttpRequest, trailingCRLF, (DWORD)(sizeof(trailingCRLF) - 1), &bytesWritten))
+    {
+        return 0;
+    }
+
+    return bytesWritten;
 }
 
 bool WinINetSyncHttpClient::DoReceiveResponse(void* hHttpRequest) const
@@ -216,21 +249,6 @@ bool WinINetSyncHttpClient::DoSendRequest(void* hHttpRequest) const
     return (HttpSendRequestEx(hHttpRequest, NULL, NULL, 0, 0) != 0);
 }
 
-bool WinINetSyncHttpClient::DoSendRequest(void* hHttpRequest, const std::shared_ptr<HttpRequest>& request) const
-{
-    // Check if this is a Transfer-Encoding: chunked request
-    if (request->HasTransferEncoding() && request->GetTransferEncoding().find("chunked") != Aws::String::npos) {
-        // For chunked encoding, use HttpSendRequestEx with unknown total length
-        INTERNET_BUFFERSA buffers = {0};
-        buffers.dwStructSize = sizeof(INTERNET_BUFFERSA);
-        buffers.dwBufferTotal = 0; // Unknown total length for chunked
-        return (HttpSendRequestExA(hHttpRequest, &buffers, NULL, 0, 0) != 0);
-    } else {
-        // For regular requests, use the simple HttpSendRequestEx
-        return (HttpSendRequestExA(hHttpRequest, NULL, NULL, 0, 0) != 0);
-    }
-}
-
 bool WinINetSyncHttpClient::DoQueryDataAvailable(void* hHttpRequest, uint64_t& available) const
 {
     return (InternetQueryDataAvailable(hHttpRequest, (LPDWORD)&available, /*reserved*/ 0, /*reserved*/ 0) != 0);
diff --git a/src/aws-cpp-sdk-core/source/http/windows/WinSyncHttpClient.cpp b/src/aws-cpp-sdk-core/source/http/windows/WinSyncHttpClient.cpp
@@ -98,13 +98,10 @@ void WinSyncHttpClient::AddHeadersToRequest(const std::shared_ptr<HttpRequest>&
 bool WinSyncHttpClient::StreamPayloadToRequest(const std::shared_ptr<HttpRequest>& request, void* hHttpRequest, Aws::Utils::RateLimits::RateLimiterInterface* writeLimiter) const
 {
     bool success = true;
+    bool isChunked = request->HasTransferEncoding() && request->GetTransferEncoding() == Aws::Http::CHUNKED_VALUE;
     auto payloadStream = request->GetContentBody();
     if(payloadStream)
     {
-        // Check if this is AWS chunked encoding
-        bool isChunked = request->HasHeader("content-encoding") && 
-                        request->GetHeaderValue("content-encoding").find("aws-chunked") != Aws::String::npos;
-        
         uint64_t bytesWritten;
         uint64_t bytesToRead = HTTP_REQUEST_WRITE_BUFFER_LENGTH;
         auto startingPos = payloadStream->tellg();
@@ -144,16 +141,23 @@ bool WinSyncHttpClient::StreamPayloadToRequest(const std::shared_ptr<HttpRequest
             success = success && ContinueRequest(*request) && IsRequestProcessingEnabled();
         }
 
+        if (success && isChunked)
+        {
+            bytesWritten = FinalizeWriteData(hHttpRequest);
+            if (!bytesWritten)
+            {
+                success = false;
+            }
+            else if (writeLimiter)
+            {
+                writeLimiter->ApplyAndPayForCost(bytesWritten);
+            }
+        }
+
         payloadStream->clear();
         payloadStream->seekg(startingPos, payloadStream->beg);
     }
 
-    // For chunked requests, finalize the transfer
-    if (success && request->HasTransferEncoding() && request->GetTransferEncoding().find("chunked") != Aws::String::npos)
-    {
-        FinalizeWriteData(hHttpRequest);
-    }
-
     if(success)
     {
         success = DoReceiveResponse(hHttpRequest);
@@ -360,7 +364,7 @@ std::shared_ptr<HttpResponse> WinSyncHttpClient::MakeRequest(const std::shared_p
 
         AddHeadersToRequest(request, hHttpRequest);
         OverrideOptionsOnRequestHandle(hHttpRequest);
-        if (DoSendRequest(hHttpRequest, request) && StreamPayloadToRequest(request, hHttpRequest, writeLimiter))
+        if (DoSendRequest(hHttpRequest) && StreamPayloadToRequest(request, hHttpRequest, writeLimiter))
         {
             success = BuildSuccessResponse(request, response, hHttpRequest, readLimiter);
         }
