diff --git a/packages/@uppy/aws-s3-multipart/src/createSignedURL.js b/packages/@uppy/aws-s3-multipart/src/createSignedURL.js index 593b327b6e..bef07e5b9c 100644 --- a/packages/@uppy/aws-s3-multipart/src/createSignedURL.js +++ b/packages/@uppy/aws-s3-multipart/src/createSignedURL.js @@ -76,6 +76,10 @@ async function hash (key, data) { 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} param0 @@ -90,7 +94,13 @@ export default async function createSignedURL ({ }) { 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. + * Note that the `/` character is purposefully not included in list below. + * + * @see https://tc39.es/ecma262/#sec-encodeuri-uri + */ + const CanonicalUri = `/${encodeURI(Key).replace(/[;?:@&=+$,#!'()*]/g, percentEncode)}` const payload = 'UNSIGNED-PAYLOAD' const requestDateTime = new Date().toISOString().replace(/[-:]|\.\d+/g, '') // YYYYMMDDTHHMMSSZ diff --git a/packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js b/packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js index cfcb12742c..bb873829eb 100644 --- a/packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js +++ b/packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js @@ -47,7 +47,7 @@ describe('createSignedURL', () => { Bucket: bucketName, Fields: {}, Key: 'some/key', - }, { expiresIn: 900 }))).searchParams.get('X-Amz-Signature'), + }), { expiresIn: 900 })).searchParams.get('X-Amz-Signature'), ) }) it('should be able to sign multipart upload', async () => { @@ -71,7 +71,43 @@ 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 extractUploadId = /([?&])uploadId=([^&]+?)(&|$)/ + const extractSignature = /([?&])X-Amz-Signature=([^&]+?)(&|$)/ + assert.strictEqual(implResult.search.match(extractUploadId)[2], sdkResult.search.match(extractUploadId)[2]) + assert.strictEqual(implResult.search.match(extractSignature)[2], sdkResult.search.match(extractSignature)[2]) + }) })