aws: Fix for s3 double encoding (#35048)

Commit Message: aws: Fix for s3 double encoding 
Additional Description:

Bugfix for issue #35023 .
Includes additional unit tests and support for s3-outposts which also
has the same workaround in the SDK for double encoding.

Risk Level: Low
Testing: Unit
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue] 35023
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

Signed-off-by: Nigel Brittain <[email protected]>

source/extensions/common/aws/utility.cc

@@ -29,6 +29,7 @@ constexpr absl::string_view QUERY_SEPERATOR = "&";
 constexpr absl::string_view QUERY_SPLITTER = "?";
 constexpr absl::string_view RESERVED_CHARS = "-._~";
 constexpr absl::string_view S3_SERVICE_NAME = "s3";
+constexpr absl::string_view S3_OUTPOSTS_SERVICE_NAME = "s3-outposts";
 constexpr absl::string_view URI_ENCODE = "%{:02X}";
 constexpr absl::string_view URI_DOUBLE_ENCODE = "%25{:02X}";
 
@@ -125,8 +126,9 @@ std::string Utility::createCanonicalRequest(
  */
 std::string Utility::canonicalizePathString(absl::string_view path_string,
                                             absl::string_view service_name) {
-  // If service is S3, do not normalize but only encode the path
-  if (absl::EqualsIgnoreCase(service_name, S3_SERVICE_NAME)) {
+  // If service is S3 or outposts, do not normalize but only encode the path
+  if (absl::EqualsIgnoreCase(service_name, S3_SERVICE_NAME) ||
+      absl::EqualsIgnoreCase(service_name, S3_OUTPOSTS_SERVICE_NAME)) {
     return encodePathSegment(path_string, service_name);
   }
   // If service is not S3, normalize and encode the path
@@ -153,8 +155,8 @@ bool isReservedChar(const char c) {
 }
 
 void encodeS3Path(std::string& encoded, const char& c) {
-  // Do not encode '/' for S3
-  if (c == PATH_SPLITTER[0]) {
+  // Do not encode '/' for S3 and do not double encode
+  if ((c == PATH_SPLITTER[0]) || (c == '%')) {
     encoded.push_back(c);
   } else {
     absl::StrAppend(&encoded, fmt::format(URI_ENCODE, c));
@@ -163,11 +165,13 @@ void encodeS3Path(std::string& encoded, const char& c) {
 
 std::string Utility::encodePathSegment(absl::string_view decoded, absl::string_view service_name) {
   std::string encoded;
+
   for (char c : decoded) {
     if (isReservedChar(c)) {
       // Escape unreserved chars from RFC 3986
       encoded.push_back(c);
-    } else if (absl::EqualsIgnoreCase(service_name, S3_SERVICE_NAME)) {
+    } else if (absl::EqualsIgnoreCase(service_name, S3_SERVICE_NAME) ||
+               absl::EqualsIgnoreCase(service_name, S3_OUTPOSTS_SERVICE_NAME)) {
       encodeS3Path(encoded, c);
     } else {
       // TODO: @aws, There is some inconsistency between AWS services if this should be double

test/extensions/common/aws/utility_test.cc

@@ -320,6 +320,38 @@ TEST(UtilityTest, EncodeS3PathSegment) {
   EXPECT_EQ("/test/%5E%21%40%3D/-_~.", encoded_path);
 }
 
+// We assume that by the time our path has reached encodePathSegment, it has already been uriEncoded
+// These tests validate that we do not doubly encode these
+TEST(UtilityTest, CheckDoubleEncodingS3) {
+  const absl::string_view path = "/test%20file";
+  const auto encoded_path = Utility::encodePathSegment(path, "s3");
+  EXPECT_EQ("/test%20file", encoded_path);
+}
+
+TEST(UtilityTest, CheckDoubleEncodingS3withSpace) {
+  const absl::string_view path = "/test file";
+  const auto encoded_path = Utility::encodePathSegment(path, "s3");
+  EXPECT_EQ("/test%20file", encoded_path);
+}
+
+TEST(UtilityTest, CheckDoubleEncodingS3Outposts) {
+  const absl::string_view path = "/test%20file";
+  const auto encoded_path = Utility::encodePathSegment(path, "s3-outposts");
+  EXPECT_EQ("/test%20file", encoded_path);
+}
+
+TEST(UtilityTest, CheckDoubleEncodingS3Folder) {
+  const absl::string_view path = "/test%20folder/test%20file";
+  const auto encoded_path = Utility::encodePathSegment(path, "s3");
+  EXPECT_EQ("/test%20folder/test%20file", encoded_path);
+}
+
+TEST(UtilityTest, CheckDoubleEncodingS3FolderPercentFile) {
+  const absl::string_view path = "/test%20folder/%25";
+  const auto encoded_path = Utility::encodePathSegment(path, "s3");
+  EXPECT_EQ("/test%20folder/%25", encoded_path);
+}
+
 TEST(UtilityTest, CanonicalizeQueryString) {
   const absl::string_view query = "a=1&b=2";
   const auto canonical_query = Utility::canonicalizeQueryString(query);