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

packages/@uppy/aws-s3-multipart/src/createSignedURL.js

@@ -76,6 +76,10 @@
   return subtle.sign(algorithm, await generateHmacKey(key), ec.encode(data))
 }
 
+function percentEncode(c) {
+  return `%${c.charCodeAt(0).toString(16).toUpperCase()}`
+}
+
 /**
  * @see https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
  * @param {Record<string,string>} param0
@@ -90,7 +94,13 @@
 }) {
   const Service = 's3'
   const host = `${bucketName}.${Service}.${Region}.amazonaws.com`
-  const CanonicalUri = `/${encodeURI(Key)}`
+  /**
+   * List of char out of `encodeURI()` is taken from ECMAScript spec
+   *
+   * @see https://tc39.es/ecma262/#sec-encodeuri-uri
+   */
+  const CanonicalUri = `/${encodeURI(Key).replace(/[;?:@&=+$,#!'()*]/g, percentEncode)}`
+  console.log(CanonicalUri)
   const payload = 'UNSIGNED-PAYLOAD'
 
   const requestDateTime = new Date().toISOString().replace(/[-:]|\.\d+/g, '') // YYYYMMDDTHHMMSSZ
@@ -143,5 +153,5 @@
   // Step 5: Add the signature to the request
   url.searchParams.set('X-Amz-Signature', signature)
 
-  return url
+  return url;
 }

packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js

@@ -51,6 +51,8 @@ describe('createSignedURL', () => {
     )
   })
   it('should be able to sign multipart upload', async () => {
+    const url = new URL('https://example.com/?%21%27%28%29%2A')
+    console.log(url.search)
     const client = new S3Client(s3ClientOptions)
     const partNumber = 99
     const uploadId = 'dummyUploadId'
@@ -71,7 +73,42 @@ describe('createSignedURL', () => {
         UploadId: uploadId,
         PartNumber: partNumber,
         Key: 'some/key',
-      }, { expiresIn: 900 }))).searchParams.get('X-Amz-Signature'),
+      }), { expiresIn: 900 })).searchParams.get('X-Amz-Signature'),
+    )
+  })
+  it('should escape path and query as restricted to RFC 3986', async () => {
+    const client = new S3Client(s3ClientOptions)
+    const partNumber = 99
+    const specialChars = ';?:@&=+$,#!\'()'
+    const uploadId = `Upload${specialChars}Id`
+    // '.*' chars of path should be encoded
+    const Key = `${specialChars}.*/${specialChars}.*.ext`
+    const implResult =
+      await createSignedURL({
+        accountKey: s3ClientOptions.credentials.accessKeyId,
+        accountSecret: s3ClientOptions.credentials.secretAccessKey,
+        sessionToken: s3ClientOptions.credentials.sessionToken,
+        uploadId,
+        partNumber,
+        bucketName,
+        Key,
+        Region: s3ClientOptions.region,
+        expires: 900,
+      })
+    const sdkResult =
+      new URL(
+        await getSignedUrl(client, new UploadPartCommand({
+          Bucket: bucketName,
+          UploadId: uploadId,
+          PartNumber: partNumber,
+          Key,
+        }), { expiresIn: 900 }
+      )
     )
+    assert.strictEqual(implResult.pathname, sdkResult.pathname)
+
+    const extractUploadIdQueryValue = (result) =>
+      result.search.match(/uploadId=(.+?)(&|$)/)[1]
+    assert.strictEqual(extractUploadIdQueryValue(implResult), extractUploadIdQueryValue(sdkResult))
   })
 })