Add an option to fix non-compliant RFC3986 encoding.

jeking3 · sbiscigl · commit cdecd6c51f5d · 2022-06-02T16:09:47.000-04:00
diff --git a/aws-cpp-sdk-core-tests/http/URITest.cpp b/aws-cpp-sdk-core-tests/http/URITest.cpp
@@ -281,7 +281,20 @@ TEST(URITest, TestParseWithColon)
     EXPECT_EQ(80, complexUri.GetPort());
     EXPECT_STREQ("/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:1234/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:Key", complexUri.GetPath().c_str());
     EXPECT_STREQ(strComplexUri, complexUri.GetURIString().c_str());
+}
 
+TEST(URITest, TestParseWithColonCompliant)
+{
+    Aws::Http::SetCompliantRfc3986Encoding(true);
+    const char* strComplexUri = "http://s3.us-east-1.amazonaws.com/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:1234/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:Key";
+    URI complexUri(strComplexUri);
+    const char* compliantStrComplexUri = "http://s3.us-east-1.amazonaws.com/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject%3A1234/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject%3AKey";
+    EXPECT_EQ(Scheme::HTTP, complexUri.GetScheme());
+    EXPECT_STREQ("s3.us-east-1.amazonaws.com", complexUri.GetAuthority().c_str());
+    EXPECT_EQ(80, complexUri.GetPort());
+    EXPECT_STREQ("/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:1234/awsnativesdkputobjectstestbucket20150702T200059Z/TestObject:Key", complexUri.GetPath().c_str());
+    EXPECT_STREQ(compliantStrComplexUri, complexUri.GetURIString().c_str());
+    Aws::Http::SetCompliantRfc3986Encoding(false);
 }
 
 TEST(URITest, TestGetURLEncodedPath)
@@ -328,3 +341,31 @@ TEST(URITest, TestGetRFC3986URLEncodedPath)
     uri = "https://test.com/segment+other/b;jsession=1";
     EXPECT_STREQ("/segment%2Bother/b%3Bjsession=1", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
 }
+
+TEST(URITest, TestGetRFC3986URLEncodedPathCompliant)
+{
+    Aws::Http::SetCompliantRfc3986Encoding(true);
+
+    URI uri = "https://test.com/path/1234/";
+    EXPECT_STREQ("/path/1234/", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/path/$omething";
+    EXPECT_STREQ("/path/%24omething", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/path/$omethingel$e";
+    EXPECT_STREQ("/path/%24omethingel%24e", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/path/~something.an0ther";
+    EXPECT_STREQ("/path/~something.an0ther", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/path/~something?an0ther";
+    EXPECT_STREQ("/path/~something", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/ሴ";
+    EXPECT_STREQ("/%E1%88%B4", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    uri = "https://test.com/segment+other/b;jsession=1";
+    EXPECT_STREQ("/segment%2Bother/b%3Bjsession%3D1", URI::URLEncodePathRFC3986(uri.GetPath()).c_str());
+
+    Aws::Http::SetCompliantRfc3986Encoding(false);
+}
diff --git a/aws-cpp-sdk-core/include/aws/core/Aws.h b/aws-cpp-sdk-core/include/aws/core/Aws.h
@@ -80,7 +80,7 @@ namespace Aws
      */
     struct HttpOptions
     {
-        HttpOptions() : initAndCleanupCurl(true), installSigPipeHandler(false)
+        HttpOptions() : initAndCleanupCurl(true), installSigPipeHandler(false), compliantRfc3986Encoding(false)
         { }
 
         /**
@@ -100,6 +100,10 @@ namespace Aws
          * NOTE: CURLOPT_NOSIGNAL is already being set.
          */
         bool installSigPipeHandler;
+        /**
+         * Disable legacy URL encoding that leaves `$&,:@=` unescaped for legacy purposes.
+         */
+        bool compliantRfc3986Encoding;
     };
 
     /**
diff --git a/aws-cpp-sdk-core/include/aws/core/http/URI.h b/aws-cpp-sdk-core/include/aws/core/http/URI.h
@@ -21,6 +21,9 @@ namespace Aws
         static const uint16_t HTTP_DEFAULT_PORT = 80;
         static const uint16_t HTTPS_DEFAULT_PORT = 443;
 
+        extern bool s_compliantRfc3986Encoding;
+        AWS_CORE_API void SetCompliantRfc3986Encoding(bool compliant);
+
         //per https://tools.ietf.org/html/rfc3986#section-3.4 there is nothing preventing servers from allowing
         //multiple values for the same key. So use a multimap instead of a map.
         typedef Aws::MultiMap<Aws::String, Aws::String> QueryStringParameterCollection;
diff --git a/aws-cpp-sdk-core/source/Aws.cpp b/aws-cpp-sdk-core/source/Aws.cpp
@@ -136,6 +136,7 @@ namespace Aws
 
         Aws::Http::SetInitCleanupCurlFlag(options.httpOptions.initAndCleanupCurl);
         Aws::Http::SetInstallSigPipeHandlerFlag(options.httpOptions.installSigPipeHandler);
+        Aws::Http::SetCompliantRfc3986Encoding(options.httpOptions.compliantRfc3986Encoding);
         Aws::Http::InitHttp();
         Aws::InitializeEnumOverflowContainer();
         cJSON_AS4CPP_Hooks hooks;
diff --git a/aws-cpp-sdk-core/source/http/URI.cpp b/aws-cpp-sdk-core/source/http/URI.cpp
@@ -24,6 +24,48 @@ namespace Http
 
 const char* SEPARATOR = "://";
 
+bool s_compliantRfc3986Encoding = false;
+void SetCompliantRfc3986Encoding(bool compliant) { s_compliantRfc3986Encoding = compliant; }
+
+Aws::String urlEncodeSegment(const Aws::String& segment)
+{
+    // consolidates legacy escaping logic into one local method
+    if (s_compliantRfc3986Encoding)
+    {
+        return StringUtils::URLEncode(segment.c_str());
+    }
+    else
+    {
+        Aws::StringStream ss;
+        ss << std::hex << std::uppercase;
+        for(unsigned char c : segment) // alnum results in UB if the value of c is not unsigned char & is not EOF
+        {
+            // RFC 3986 §2.3 unreserved characters
+            if (StringUtils::IsAlnum(c))
+            {
+                ss << c;
+                continue;
+            }
+            switch(c)
+            {
+                // §2.3 unreserved characters
+                // The path section of the URL allows unreserved characters to appear unescaped
+                case '-': case '_': case '.': case '~':
+                // RFC 3986 §2.2 Reserved characters
+                // NOTE: this implementation does not accurately implement the RFC on purpose to accommodate for
+                // discrepancies in the implementations of URL encoding between AWS services for legacy reasons.
+                case '$': case '&': case ',':
+                case ':': case '=': case '@':
+                    ss << c;
+                    break;
+                default:
+                    ss << '%' << std::setfill('0') << std::setw(2) << (int)c << std::setw(0);
+            }
+        }
+        return ss.str();
+    }
+}
+
 } // namespace Http
 } // namespace Aws
 
@@ -101,7 +143,7 @@ void URI::SetScheme(Scheme value)
 
 Aws::String URI::URLEncodePathRFC3986(const Aws::String& path)
 {
-    if(path.empty())
+    if (path.empty())
     {
         return path;
     }
@@ -113,34 +155,10 @@ Aws::String URI::URLEncodePathRFC3986(const Aws::String& path)
     // escape characters appearing in a URL path according to RFC 3986
     for (const auto& segment : pathParts)
     {
-        ss << '/';
-        for(unsigned char c : segment) // alnum results in UB if the value of c is not unsigned char & is not EOF
-        {
-            // §2.3 unreserved characters
-            if (StringUtils::IsAlnum(c))
-            {
-                ss << c;
-                continue;
-            }
-            switch(c)
-            {
-                // §2.3 unreserved characters
-                case '-': case '_': case '.': case '~':
-                // The path section of the URL allow reserved characters to appear unescaped
-                // RFC 3986 §2.2 Reserved characters
-                // NOTE: this implementation does not accurately implement the RFC on purpose to accommodate for
-                // discrepancies in the implementations of URL encoding between AWS services for legacy reasons.
-                case '$': case '&': case ',':
-                case ':': case '=': case '@':
-                    ss << c;
-                    break;
-                default:
-                    ss << '%' << std::setfill('0') << std::setw(2) << (int)((unsigned char)c) << std::setw(0);
-            }
-        }
+        ss << '/' << urlEncodeSegment(segment);
     }
 
-    //if the last character was also a slash, then add that back here.
+    // if the last character was also a slash, then add that back here.
     if (path.back() == '/')
     {
         ss << '/';
@@ -216,33 +234,10 @@ Aws::String URI::GetURLEncodedPathRFC3986() const
     ss << std::hex << std::uppercase;
 
     // escape characters appearing in a URL path according to RFC 3986
+    // (mostly; there is some non-standards legacy support that can be disabled)
     for (const auto& segment : m_pathSegments)
     {
-        ss << '/';
-        for(unsigned char c : segment) // alnum results in UB if the value of c is not unsigned char & is not EOF
-        {
-            // §2.3 unreserved characters
-            if (StringUtils::IsAlnum(c))
-            {
-                ss << c;
-                continue;
-            }
-            switch(c)
-            {
-                // §2.3 unreserved characters
-                case '-': case '_': case '.': case '~':
-                // The path section of the URL allow reserved characters to appear unescaped
-                // RFC 3986 §2.2 Reserved characters
-                // NOTE: this implementation does not accurately implement the RFC on purpose to accommodate for
-                // discrepancies in the implementations of URL encoding between AWS services for legacy reasons.
-                case '$': case '&': case ',':
-                case ':': case '=': case '@':
-                    ss << c;
-                    break;
-                default:
-                    ss << '%' << std::setfill('0') << std::setw(2) << (int)((unsigned char)c) << std::setw(0);
-            }
-        }
+        ss << '/' << urlEncodeSegment(segment);
     }
 
     if (m_pathSegments.empty() || m_pathHasTrailingSlash)
diff --git a/aws-cpp-sdk-s3-integration-tests/RunTests.cpp b/aws-cpp-sdk-s3-integration-tests/RunTests.cpp
@@ -9,13 +9,34 @@
 #include <aws/testing/TestingEnvironment.h>
 #include <aws/testing/MemoryTesting.h>
 
+void ParseArgs(int argc, char** argv, Aws::SDKOptions& options)
+{
+    // std::string rather than Aws::String since this happens before the memory manager is initialized
+    const std::string resourcePrefixOption = "--rfc3986_compliant=";
+    // list other options here
+    for(int i = 1; i < argc; i++)
+    {
+        std::string arg = argv[i];
+        if(arg.find(resourcePrefixOption) == 0)
+        {
+            arg = arg.substr(resourcePrefixOption.length()); // get whatever value after the '='
+            if (arg == "true" || arg == "1")
+            {
+                std::cout << "Set RFC3986 compliance mode ON." << std::endl;
+                options.httpOptions.compliantRfc3986Encoding = true;
+            }
+        }
+    }
+}
+
 int main(int argc, char** argv)
 {
     Aws::SDKOptions options;
     options.loggingOptions.logLevel = Aws::Utils::Logging::LogLevel::Trace;
     AWS_BEGIN_MEMORY_TEST_EX(options, 1024, 128);
     Aws::Testing::InitPlatformTest(options);
     Aws::Testing::ParseArgs(argc, argv);
+    ParseArgs(argc, argv, options);
 
     Aws::InitAPI(options);
     ::testing::InitGoogleTest(&argc, argv);
