Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

aws: Fix for s3 double encoding #35048

Merged
merged 1 commit into from
Jul 6, 2024
Merged

aws: Fix for s3 double encoding #35048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions source/extensions/common/aws/utility.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}";

Expand Down Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions test/extensions/common/aws/utility_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down