@uppy/aws-s3-multipart: Fix escaping issue with client signed request

[hiromi2424]

This fixes #5005 .

AWS SDK implementation

AWS SDK will escape all characters including "!'()*" such as not escaped by encodeURI/encodeURIComponent .
This is aimed according to RFC 3986.
https://datatracker.ietf.org/doc/html/rfc3986

Those are escaped by AWS SDK's implementation, but it is complicated, so I investigated what code escapes that:

Escaping path

S3 presigner using command (for example, UploadPartCommand)
=> Ser(Request serializer) Middleware of UploadPartCommand
=> se_UploadPartCommand
=> resolvedPath
=> extended-uri-component
https://github.com/smithy-lang/smithy-typescript/blob/main/packages/smithy-client/src/extended-encode-uri-component.ts

Escaping query

@aws/signature-v4 (now deprecated, moved to @smithy/signature-v4)
SignatureV4
=> getCanonicalQuery
https://github.com/smithy-lang/smithy-typescript/blob/main/packages/signature-v4/src/getCanonicalQuery.ts
=> escapeUri
https://github.com/smithy-lang/smithy-typescript/blob/main/packages/util-uri-escape/src/escape-uri.ts

Uppy's current implementation

Escaping path

Using encodeURI to path directly
=> All of "!'()*" chars are not escaped.

Escaping query

Using URL#searchParams to escape query
=> Almost of special chars are escaped, but not only "*" is NOT.

[github-actions]

Diff output files

diff --git a/packages/@uppy/aws-s3-multipart/lib/createSignedURL.js b/packages/@uppy/aws-s3-multipart/lib/createSignedURL.js
index 7ce2b7c..c6d54e0 100644
--- a/packages/@uppy/aws-s3-multipart/lib/createSignedURL.js
+++ b/packages/@uppy/aws-s3-multipart/lib/createSignedURL.js
@@ -48,6 +48,9 @@ async function hash(key, data) {
   } = globalThis.crypto;
   return subtle.sign(algorithm, await generateHmacKey(key), ec.encode(data));
 }
+function percentEncode(c) {
+  return `%${c.charCodeAt(0).toString(16).toUpperCase()}`;
+}
 export default async function createSignedURL(_ref2) {
   let {
     accountKey,
@@ -62,7 +65,7 @@ export default async function createSignedURL(_ref2) {
   } = _ref2;
   const Service = "s3";
   const host = `${bucketName}.${Service}.${Region}.amazonaws.com`;
-  const CanonicalUri = `/${encodeURI(Key)}`;
+  const CanonicalUri = `/${encodeURI(Key).replace(/[;?:@&=+$,#!'()*]/g, percentEncode)}`;
   const payload = "UNSIGNED-PAYLOAD";
   const requestDateTime = new Date().toISOString().replace(/[-:]|\.\d+/g, "");
   const date = requestDateTime.slice(0, 8);

[hiromi2424]

Needs e2e test?
It is heavy to me setup locally...
And I don't know this repo's AWS account and bucket for CI was properly setup CORS and permissions for sts getFederationToken, or not.
If additional e2e test is needed, please help to write and setup that 😅

[Murderlon]

Thanks a lot for looking into this and writing a test. I'm just wondering why this is reverse engineered, can't we use the AWS SDK for this to make sure we stay aligned?

[aduh95]

can't we use the AWS SDK for this to make sure we stay aligned?

The AWS SDK is enormous, the goal was to limit the grow of the bundle size.

[aduh95]

Thanks for the work 🙌