transloadit / uppy

The next open source file uploader for web browsers :dog:
https://uppy.io
MIT License
29.18k stars 2.01k forks source link

upload-success not setting uploadURL correctly #5388

Open pekeler opened 3 months ago

pekeler commented 3 months ago

Initial checklist

Link to runnable example

No response

Steps to reproduce

Using self hosted Companion and uppy-svelte, we used to use upload-success to get the path of the uploaded file to S3 from result.body.key which worked great using version 3. Now, after upgrading to v4.1 (and the companion to v5) and seeing that upload-success has changed such that we're supposed to be getting the path from result.uploadURL, we find that the uploadURL is always just set to the bucket's base URL without anything for the uploaded file itself.

Inspecting the networking traffic between browser and companion, we see that the response from companion has the filepath under fields.key. So it seems there's some mismatch between the latest companion and the latest client lib?

https://uppy.io/docs/uppy/#upload-success

Expected behavior

result.uploadURL is the complete file's path on S3 (i.e. a bucket-url/UUID-filename)

Actual behavior

result.uploadURL is just the bucket's URL

Murderlon commented 3 months ago

Hi, I just tested locally and I do get the complete file path. Are you on latest versions of everything?

pekeler commented 3 months ago

Yes, npm outdated shows that everything is up-to-date.

(This is using DigitalOcean Spaces as S3 storage - though this has worked fine for us with previous versions of Uppy).

Murderlon commented 3 months ago

It works for me for S3 and we don't officially support or test against DigitalOcean Spaces. If you can reproduce on S3 that would be great. Otherwise you may have to contribute yourself to fix it.

timothygachengo commented 2 months ago

I have the same issue with AWS S3 where it returns only the base bucket URL.

augustosamame commented 2 months ago

+1

I also use AWS S3 and I can see the uppy uploaded file in S3 cache folder. However, Im also only not getting the base bucket URL in uploadURL key.

not using Companion but Shrine plugin

"@uppy/aws-s3": "^4.0.3", "@uppy/core": "^4.1.2", "@uppy/dashboard": "^4.0.3", "@uppy/locales": "^4.0.3",

gem "shrine", "~> 3.6.0"

this is a complete example response (notice the weird "": "" key in the reponse body key which might be a clue):

{ "source": "Dashboard", "id": "sample/image/jpeg-2v-2v-1e-image/jpeg-91516-1671405542014", "name": "sample_image.jpeg", "extension": "jpeg", "meta": { "relativePath": null, "name": "sample_image.jpeg", "type": "image/jpeg" }, "type": "image/jpeg", "data": { "lastModified": 1724512075822, "lastModifiedDate": "2024-08-24T15:07:55.822Z", "name": "sample_image.jpeg" }, "progress": { "uploadStarted": 1724512075828, "uploadComplete": true, "percentage": 100, "bytesUploaded": 64790, "bytesTotal": 64790 }, "size": 64790, "isGhost": false, "isRemote": false, "preview": "blob:http://localhost:3000/727c9392-9254-4115-84e9-65d37a09ee56", "response": { "body": { "location": "https://devtech-edukaierp-dev.s3.amazonaws.com/", "": "" }, "status": 200, "uploadURL": "https://devtech-edukaierp-dev.s3.amazonaws.com/" }, "uploadURL": "https://devtech-edukaierp-dev.s3.amazonaws.com/", "isPaused": false }

texpert commented 2 months ago

When using @uppy/aws-s3 and doing non multi-part uploads, the nonMultipartUpload method is used from HTTPCommunicationQueue.

Seems that every other uploading method in HTTPCommunicationQueue is doing

const { uploadId, key } = await this.getUploadId(file, signal)

, and then adding the key to the returned result.

No such luck with the nonMultipartUpload method.

Sushant-Borsania commented 2 months ago

The multipart upload (shouldUseMultipart = true) returns the full URL but the nonMultipartUpload doesn't. It's just a bucket URL. :(

needcaffeine commented 2 months ago

+1 that this is broken for:

Murderlon commented 1 month ago

I'm trying locally with shouldUseMultipart: false but I always get the full URL back? Not sure how to reproduce.

janklimo commented 3 weeks ago

Seeing the same behavior in v4. @Murderlon here are the file and data objects I get in the upload-success event, perhaps it helps:

Screenshot 2024-10-17 at 12 54 07
2lives commented 2 weeks ago

I encounter the same regardless of whether or not I set shouldUseMultipart to false: image

Only became an issue once upgrading from the following package versions: image

to updated uppy package versions: image

upload success file:

image

complete:

image

hope this helps lead to a resolution.

mifi commented 6 days ago

Managed to reproduce, working on a fix

mifi commented 1 day ago

the problem seems to appear when the s3 bucket does not have the correct CORS policy configured as described in the docs - in particular "ExposeHeaders": ["ETag", "Location"]. in previous versions upppy this was kind of OK because Uppy would log a warning to the browser console so that we know what's wrong:

https://github.com/transloadit/uppy/blob/6d413f5eac23eb457d2d3a33dc0df9687b051695/packages/%40uppy/aws-s3/src/index.ts#L767

but apparently that null/undefined check broke in a refactor (another reason to have strict types #5336). I've now created a PR #5505 that restores the behavior where it will warn when CORS is not configured properly.