Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Mar 19, 2024
12 changes: 11 additions & 1 deletion packages/@uppy/aws-s3-multipart/src/createSignedURL.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,string>} param0
Expand All @@ -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
Expand Down
40 changes: 38 additions & 2 deletions packages/@uppy/aws-s3-multipart/src/createSignedURL.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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])
})
})
Loading