romange / helio

A modern framework for backend development based on io_uring Linux interface
Apache License 2.0
447 stars 50 forks source link

fix(s3): fix url encoding s3 path #135

Closed andydunstall closed 1 year ago

andydunstall commented 1 year ago

Fixes https://github.com/dragonflydb/controlplane/issues/225, where if the bucket or key includes a character that AWS expects to be URL encoded, the signature will be invalid

AWS defines their own URL path encoding in https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html:

URI encode every byte. UriEncode() must enforce the following rules:
- URI encode every byte except the unreserved characters: 'A'-'Z', 'a'-'z', '0'-'9', '-', '.', '_', and '~'.
- The space character is a reserved character and must be encoded as "%20" (and not as "+").
- Each URI encoded byte is formed by a '%' and the two-digit hexadecimal value of the byte.
- Letters in the hexadecimal value must be uppercase, for example "%1A".
- Encode the forward slash character, '/', everywhere except in the object key name. For example, if the object key name is photos/Jan/sample.jpg, the forward slash in the key name is not encoded.

The standard UriEncode functions provided by your development platform may not work because of differences in implementation and related ambiguity in the underlying RFCs. We recommend that you write your own custom UriEncode function to ensure that your encoding will work.

This implementation is ported from the Go SDK https://github.com/aws/aws-sdk-go/blob/main/private/protocol/rest/build.go#L261

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.14% :warning:

Comparison is base (bb725aa) 76.34% compared to head (4df6b96) 76.21%. Report is 3 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #135 +/- ## ========================================== - Coverage 76.34% 76.21% -0.14% ========================================== Files 97 97 Lines 7057 7051 -6 ========================================== - Hits 5388 5374 -14 - Misses 1669 1677 +8 ``` [see 10 files with indirect coverage changes](https://app.codecov.io/gh/romange/helio/pull/135/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Roman+Gershman)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

andydunstall commented 1 year ago

Tested with:

romange commented 1 year ago

Got ya. Then let's use absl::StrAppendFormat()

On Thu, Sep 7, 2023, 12:51 PM Andy Dunstall @.***> wrote:

@.**** commented on this pull request.

In util/cloud/aws.cc https://github.com/romange/helio/pull/135#discussion_r1318368095:

  • c == '-' ||
  • c == '.' ||
  • c == '_' ||
  • c == '~'
  • ); +}
  • +// Escapes the given path as documented by +// https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html. +std::string AwsEscapePath(const std::string_view& path, bool encode_sep) {

  • std::string escaped;
  • for (char c : path) {
  • if (!AwsIsEscaped(c) || (c == '/' && !encode_sep)) {
  • escaped.push_back(c);
  • } else {
  • escaped += absl::StrFormat("%%%02X", c);

Trying that absl::Hex returns lowercase though we need uppercase to match the signature

— Reply to this email directly, view it on GitHub https://github.com/romange/helio/pull/135#discussion_r1318368095, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4BFCGG3MCF6PSW2I77IFDXZGKIJANCNFSM6AAAAAA4ORLIBE . You are receiving this because your review was requested.Message ID: @.***>

andydunstall commented 1 year ago

Then let's use absl::StrAppendFormat()

Ok sure updated