Fix S3CrtClient to use origin-form request-target

src/aws-cpp-sdk-core/source/http/HttpRequest.cpp

@@ -70,15 +70,6 @@ namespace Aws
             // CRT sigv4 don't any encoding if double encoding is off, need to encode the path before passing to CRT.
             const URI& uri = m_uri;
             Aws::StringStream ss;
-            Aws::StringStream port;
-            if (uri.GetScheme() == Scheme::HTTP && uri.GetPort() != HTTP_DEFAULT_PORT)
-            {
-                port << ":" << uri.GetPort();
-            }
-            else if (uri.GetScheme() == Scheme::HTTPS && uri.GetPort() != HTTPS_DEFAULT_PORT)
-            {
-                port << ":" << uri.GetPort();
-            }
 
             // Note: GetURLEncodedPathRFC3986 does legacy mode by default, which
             // leaves some reserved delimeter characters unencoded (things like
@@ -88,9 +79,12 @@ namespace Aws
             // practice however CRT will never support legacy mode and will
             // always need modern encoding, so be explicit here about which mode
             // we use.
-            ss << SchemeMapper::ToString(uri.GetScheme()) << SEPARATOR << uri.GetAuthority() << port.str()
-                << ((uri.GetPath() == "/") ? "" : uri.GetURLEncodedPath())
-                << uri.GetQueryString();
+            //
+            // Origin-form request-target (path + query) per RFC 9112 §3.2.1.
+            // GetURLEncodedPath() always begins with '/', including the root
+            // path. aws-c-http rewrites this to absolute-form only when the
+            // request goes through a forward proxy.
+            ss << uri.GetURLEncodedPath() << uri.GetQueryString();
 
             request->SetPath(Aws::Crt::ByteCursorFromCString(ss.str().c_str()));
             const char *method = HttpMethodMapper::GetNameForHttpMethod(m_method);

tests/aws-cpp-sdk-core-tests/http/HttpRequestTest.cpp

@@ -0,0 +1,79 @@
+/**
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0.
+ */
+
+#include <aws/core/http/URI.h>
+#include <aws/core/http/standard/StandardHttpRequest.h>
+#include <aws/crt/http/HttpRequestResponse.h>
+#include <aws/testing/AwsCppSdkGTestSuite.h>
+
+using namespace Aws::Http;
+using namespace Aws::Http::Standard;
+
+namespace {
+// Resets the process-global path-separator flag when the test leaves, including on assertion failure.
+class PreservePathSeparatorsGuard {
+ public:
+  explicit PreservePathSeparatorsGuard(bool preserve) { SetPreservePathSeparators(preserve); }
+
+  ~PreservePathSeparatorsGuard() { SetPreservePathSeparators(false); }
+
+  PreservePathSeparatorsGuard(const PreservePathSeparatorsGuard&) = delete;
+  PreservePathSeparatorsGuard& operator=(const PreservePathSeparatorsGuard&) = delete;
+};
+
+Aws::String CrtRequestPath(const char* uri, HttpMethod method) {
+  StandardHttpRequest request(URI(uri), method);
+  auto crtRequest = request.ToCrtHttpRequest();
+  if (!crtRequest) {
+    ADD_FAILURE() << "ToCrtHttpRequest returned null for " << uri;
+    return {};
+  }
+
+  auto path = crtRequest->GetPath();
+  if (!path.has_value()) {
+    ADD_FAILURE() << "CRT request has no path for " << uri;
+    return {};
+  }
+
+  return Aws::String(reinterpret_cast<const char*>(path->ptr), path->len);
+}
+}  // namespace
+
+class HttpRequestConversionTest : public Aws::Testing::AwsCppSdkGTestSuite {};
+
+TEST_F(HttpRequestConversionTest, OriginFormNonDefaultPort) {
+  EXPECT_EQ("/bucket/key?uploadId=abc&partNumber=1",
+            CrtRequestPath("http://example.com:9000/bucket/key?uploadId=abc&partNumber=1", HttpMethod::HTTP_PUT));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormHttpsPath) {
+  EXPECT_EQ("/my-bucket/my-key", CrtRequestPath("https://s3.amazonaws.com/my-bucket/my-key", HttpMethod::HTTP_GET));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormQueryString) {
+  EXPECT_EQ("/path/to/file?query=value&foo=bar",
+            CrtRequestPath("https://storage.example.com:443/path/to/file?query=value&foo=bar", HttpMethod::HTTP_POST));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormRootPath) { EXPECT_EQ("/", CrtRequestPath("http://localhost:8080/", HttpMethod::HTTP_HEAD)); }
+
+// Old absolute-form code dropped a root path and emitted "https://host?list-type=2".
+TEST_F(HttpRequestConversionTest, OriginFormRootPathWithQuery) {
+  EXPECT_EQ("/?list-type=2", CrtRequestPath("https://host/?list-type=2", HttpMethod::HTTP_GET));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormTrailingSlash) {
+  EXPECT_EQ("/bucket/prefix/", CrtRequestPath("https://host/bucket/prefix/", HttpMethod::HTTP_GET));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormEncodesReservedCharacters) {
+  EXPECT_EQ("/bucket/a%3Ab%20c%2Bd", CrtRequestPath("https://host/bucket/a:b c+d", HttpMethod::HTTP_PUT));
+}
+
+TEST_F(HttpRequestConversionTest, OriginFormPreservesLeadingSlashInKey) {
+  PreservePathSeparatorsGuard guard(true);
+  (void)guard;
+  EXPECT_EQ("//key", CrtRequestPath("https://host//key", HttpMethod::HTTP_GET));
+}