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

S3 Non-multipart location #4618

Closed
2 tasks done
kotx opened this issue Aug 12, 2023 · 6 comments
Closed
2 tasks done

S3 Non-multipart location #4618

kotx opened this issue Aug 12, 2023 · 6 comments
Assignees
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 duplicate This issue/PR already exists Feature

Comments

@kotx
Copy link

kotx commented Aug 12, 2023

Initial checklist

  • I understand this is a feature request and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Problem

When hooking completeMultipartUpload, you can return a location string to display a link in Dashboard.
https://uppy.io/docs/aws-s3-multipart/#completemultipartuploadfile--uploadid-key-parts-

However, this is not present in getUploadParameters: https://uppy.io/docs/aws-s3-multipart/#getuploadparametersfile-options
(Side note: getUploadParameters doesn't actually pass option, that might be an oversight in the documentation?)

Solution

Allow getUploadParameters to return a location property.

Alternatives

Maybe have a getLocation(key) function in the S3 plugin?

@kotx kotx added the Feature label Aug 12, 2023
@Murderlon
Copy link
Member

Does this fix your issue: #4614?

@kotx
Copy link
Author

kotx commented Aug 14, 2023

Does this fix your issue: #4614?

I don't think so. Cloudflare R2 in particular does not return a correct Location header, it returns the region auto for some reason instead.
Also, sometimes the bucket access url is not the same as the internal bucket url. For example when the bucket is accessed behind a firewall.

@Murderlon
Copy link
Member

See this issue why we don't support Cloudfare R2 (until they provide a fix on their end): #4505

Closing as duplicate.

@Murderlon Murderlon closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2023
@Murderlon Murderlon added AWS S3 Plugin that handles uploads to Amazon AWS S3 duplicate This issue/PR already exists labels Aug 21, 2023
@kotx
Copy link
Author

kotx commented Aug 22, 2023

See this issue why we don't support Cloudfare R2 (until they provide a fix on their end): #4505

Closing as duplicate.
@Murderlon

Actually both issues (#4514, #4505) are for multipart and not Cloudflare R2-specific, my bad. Non-multipart upload (which is PutObject) does not return a Location header, and shouldn't, from what I can tell from the spec.

In short, the issue is not R2-specific and Uppy should support a location parameter for non-multipart uploads as it does for multipart uploads. Because it is otherwise impossible to return a URL for s3 non-multipart uploads.

@Murderlon
Copy link
Member

Doesn't this solve your issue: #4614

There is also a test in it for non multipart uploads.

@kotx
Copy link
Author

kotx commented Aug 24, 2023

The Location field doesn't do what I want (it returns a bucket URL that requires authorization, not a public url or custom domain). I like the current API with multipart which allows the server to return a location field that the client can just return to Uppy, which allows for arbitrary URLs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AWS S3 Plugin that handles uploads to Amazon AWS S3 duplicate This issue/PR already exists Feature
Projects
None yet
Development

No branches or pull requests

3 participants