Skip to content

Commit

Permalink
@uppy/aws-s3-multipart: fix escaping issue with client signed request (
Browse files Browse the repository at this point in the history
  • Loading branch information
hiromi2424 committed Mar 19, 2024
1 parent 730459d commit c306d6c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 3 deletions.
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])
})
})

0 comments on commit c306d6c

Please sign in to comment.