uppy icon indicating copy to clipboard operation
uppy copied to clipboard

AWs s3 multipart upload plugin is possibly accessing wrong attribute IDs in the response

Open narioinc opened this issue 2 years ago • 3 comments

Hi Team,

First, a big thanks to everyone for creating such a wonderful uploader tooling and making it easy to intergate upload workflows into apps

I just found a quick issue with the AWS s3 multipart upload plugin. @uppy/aws-s3-multipart The plugin currently fails when accessing AWS s3 via the createMultipartUpload() action as it is trying to access "uploadId" and "key" Whereas it seems that the AWS s3 JS docs currently point to "UploadId" and "Key" in their docs. Might have been due to a breaking API change on AWS side, but thought ill send the uppy team a heads up

AWS docs: https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#createMultipartUpload-property

Currently when using the uppy framework for uplaoding to AWS S3, shows this error image

The change to the capital letters in the API may have lead to this issue. Please let me know if I can help with a PR for this correction and if there are any developer agreements that need to take place for enabling PR requests.

Thanks !!

narioinc avatar Feb 22 '22 03:02 narioinc

Hey, good catch, thanks for exploring that. If you are willing to send a PR, that'd be awesome! No dev agreements to sign :)

aduh95 avatar Feb 24 '22 22:02 aduh95

Hi @aduh95

thanks, I have the fix ready, but since Uppy has a lot of devs using the framework in prod, I wanted to have a test fixture with a sample app to test out the changes before sending a PR. ill also check if we have any unit tests as well. I''ll fork and send a PR out by tomorrow, 26-02-2022.

narioinc avatar Feb 25 '22 03:02 narioinc

Hi @narioinc, is there any update on this, did you manage to fix it? We’d appreciate your PR.

arturi avatar Mar 14 '22 13:03 arturi

Hi, looking at this deeper I don't see the issue. The way you describe the issue seems to indicate that an error occurs when we do createMultipartUpload and send uploadId and key. But we don't send that, those values are the response from the creation:

https://github.com/transloadit/uppy/blob/31a9e0c2fdf39b41ac98a36d2fe4ef7365c98394/packages/%40uppy/companion/src/server/controllers/s3.js#L96-L114

We merely send the uncapitalized keys to Uppy, which does expect it that way.

I can confirm this works in an E2E test which uses this:

Screenshot 2022-08-11 at 12 35 49

Murderlon avatar Aug 11 '22 10:08 Murderlon