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

…fixes #5005

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

@@ -76,6 +76,30 @@ async function hash (key, data) {
  return subtle.sign(algorithm, await generateHmacKey(key), ec.encode(data))
 }
 
+/**
+ * @see
+ * https://github.com/smithy-lang/smithy-typescript/blob/main/packages/smithy-client/src/extended-encode-uri-component.ts
+ */
+function extendedEncodeURIComponent(str) {
+ return encodeURIComponent(str).replace(/[!'()*]/g, (c) =>
+ `%${c.charCodeAt(0).toString(16).toUpperCase()}`
+ )
+}
+
+class Query {
+ #params = []
+
+ append(key, value) {
+ this.#params.push([key, value])
+ }
+
+ toString() {
+ return this.#params
+ .map(([key, value]) => `${extendedEncodeURIComponent(key)}=${extendedEncodeURIComponent(value)}`)
+ .join('&')
+ }
+}
+
 /**
  * @see https://docs.aws.amazon.com/IAM/latest/UserGuide/create-signed-request.html
  * @param {Record<string,string>} param0
@@ -90,32 +114,32 @@ export default async function createSignedURL ({
 }) {
  const Service = 's3'
  const host = `${bucketName}.${Service}.${Region}.amazonaws.com`
- const CanonicalUri = `/${encodeURI(Key)}`
+ const CanonicalUri = `/${Key.split('/').map(extendedEncodeURIComponent).join('/')}`
  const payload = 'UNSIGNED-PAYLOAD'
 
  const requestDateTime = new Date().toISOString().replace(/[-:]|\.\d+/g, '') // YYYYMMDDTHHMMSSZ
  const date = requestDateTime.slice(0, 8) // YYYYMMDD
  const scope = `${date}/${Region}/${Service}/aws4_request`
 
- const url = new URL(`https://${host}${CanonicalUri}`)
+ const query = new Query();
  // N.B.: URL search params needs to be added in the ASCII order
- url.searchParams.set('X-Amz-Algorithm', 'AWS4-HMAC-SHA256')
- url.searchParams.set('X-Amz-Content-Sha256', payload)
- url.searchParams.set('X-Amz-Credential', `${accountKey}/${scope}`)
- url.searchParams.set('X-Amz-Date', requestDateTime)
- url.searchParams.set('X-Amz-Expires', expires)
+ query.append('X-Amz-Algorithm', 'AWS4-HMAC-SHA256')
+ query.append('X-Amz-Content-Sha256', payload)
+ query.append('X-Amz-Credential', `${accountKey}/${scope}`)
+ query.append('X-Amz-Date', requestDateTime)
+ query.append('X-Amz-Expires', expires)
  // We are signing on the client, so we expect there's going to be a session token:
- url.searchParams.set('X-Amz-Security-Token', sessionToken)
- url.searchParams.set('X-Amz-SignedHeaders', 'host')
+ query.append('X-Amz-Security-Token', sessionToken)
+ query.append('X-Amz-SignedHeaders', 'host')
  // Those two are present only for Multipart Uploads:
- if (partNumber) url.searchParams.set('partNumber', partNumber)
- if (uploadId) url.searchParams.set('uploadId', uploadId)
- url.searchParams.set('x-id', partNumber && uploadId ? 'UploadPart' : 'PutObject')
+ if (partNumber) query.append('partNumber', partNumber)
+ if (uploadId) query.append('uploadId', uploadId)
+ query.append('x-id', partNumber && uploadId ? 'UploadPart' : 'PutObject')
 
  // Step 1: Create a canonical request
  const canonical = createCanonicalRequest({
  CanonicalUri,
- CanonicalQueryString: url.search.slice(1),
+ CanonicalQueryString: query.toString(),
  SignedHeaders: {
  host,
  },
@@ -141,7 +165,7 @@ export default async function createSignedURL ({
  const signature = arrayBufferToHexString(await hash(kSigning, stringToSign))
 
  // Step 5: Add the signature to the request
- url.searchParams.set('X-Amz-Signature', signature)
+ query.append('X-Amz-Signature', signature)
 
- return url
+ return new URL(`https://${host}${CanonicalUri}?${query.toString()}`)
 }

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,40 @@ 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 uploadId = 'Upload!\'()*Id'
+ const Key = '!\'()*/!\'()*.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))
  })
 })