diff --git a/source/extensions/common/aws/utility.cc b/source/extensions/common/aws/utility.cc index 99988e40366b..5e2082a16cc7 100644 --- a/source/extensions/common/aws/utility.cc +++ b/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 diff --git a/test/extensions/common/aws/utility_test.cc b/test/extensions/common/aws/utility_test.cc index 936ba14a130d..badeac166cc6 100644 --- a/test/extensions/common/aws/utility_test.cc +++ b/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);