diff --git a/src/aws-cpp-sdk-core/source/http/HttpRequest.cpp b/src/aws-cpp-sdk-core/source/http/HttpRequest.cpp index 55ca99c64661..23ddce0aa242 100644 --- a/src/aws-cpp-sdk-core/source/http/HttpRequest.cpp +++ b/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); diff --git a/tests/aws-cpp-sdk-core-tests/http/HttpRequestTest.cpp b/tests/aws-cpp-sdk-core-tests/http/HttpRequestTest.cpp new file mode 100644 index 000000000000..f2ea9db654ea --- /dev/null +++ b/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 +#include +#include +#include + +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(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)); +}