-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Tus resumes unrelated uploads, causing conflicts or lost metadata #5019
Comments
Hi, did you try to simply pass your own
To me it sounds a lot better to handle duplicate uploads on the server and prevent all your users from having to upload everything twice. That's double their bandwidth and more chances of it going wrong or out of sync. With tus this should be trivial with hooks and events, which our official servers have. |
Yes. It is unconditionally overwritten by the Uppy Tus uploader plugin, so setting that option for the plugin has no effect. See uppy/packages/@uppy/tus/src/index.ts Line 249 in 68af8a3
Currently the back-end controls uploads via |
This is not just about this particular use-case by the way, it may affect other applications that depend on metadata in Tus uploads as well. Tus supports metadata natively and accepts and persists it along with the upload binary data. Metadata is part of the upload artifact. Files with different metadata should not be considered equal and silently skipped, or there should at least be a way to control which metadata is part of an upload identity (e.g. by overriding Tus fingerprints or the generation of |
I experimented a bit and think that it would not be enough to be able to control the Tus fingerprint, because other components (most notably GoldenRetriever) also assume that I solved this for myself by replacing a single line: diff --git a/packages/@uppy/core/src/Uppy.ts b/packages/@uppy/core/src/Uppy.ts
index f813b2242..502ba08fb 100644
--- a/packages/@uppy/core/src/Uppy.ts
+++ b/packages/@uppy/core/src/Uppy.ts
@@ -911,7 +911,7 @@ export class Uppy<M extends Meta, B extends Body> {
const fileType = getFileType(file)
const fileName = getFileName(fileType, file)
const fileExtension = getFileNameAndExtension(fileName).extension
- const id = getSafeFileId(file)
+ const id = getSafeFileId(file).replace("uppy-", `uppy-${this.getID()}`)
const meta = file.meta || {}
meta.name = fileName This is obviously not a real solution, just a hotfix. A better way would be to have a configurable hook that either adds to, or completely replaces the ID generated by |
@Acconut what do you think of this? |
In an ideal world, duplicate uploads of the same file would be avoided, but in practice this is not always easily possible, so I think this should be addressed. The easiest (and sensible) solution seems to be adding the uppy ID into the file ID. tus-js-client puts the fingerprints into the global localStorage, where collisions can appear leading to the described behavior. If we can embed information about the corresponding uppy instance into the file ID and thus fingerprint, this should work. |
Initial checklist
Link to runnable example
No response
Steps to reproduce
1.Create two Uppy instances (A and B) with different
id
andmeta
settings, but the same Tus endpoint. Those instances should be on the same origin domain, but can be on different pages.2. Upload a file to A.
3. Upload the same file to B.
Or simply upload the same file twice, but with different metadata the second time.
Expected behavior
The second upload should trigger a new Tus upload with corresponding metadata. For example, by taking the
id
of the Uppy instance and the upload metadata into account when calculating the Tus fingerprint. Alternatively, there should be a hook to control either theFile.id
or the Tus fingerprint for new files.Actual behavior
The second upload will have the same
File.id
(and thus Tus fingerprint) as the first upload. As a result, tus-js-client will immediately report the file as successfully uploaded and Uppy will emit anupload-success
event. In the back-end however only one tus file exists and that file has the metadata of the first upload. This breaks applications that need to support duplicate uploads, for example to allow the same file being uploaded to different folders or with different metadata.The
onBeforeFileAdded
hook cannot be used to change theFile.id
as other code that depends on it (e.g. Golden RetrieverisGhost
detection) runs before that hook. The Uppy ID is not part of theFile.id
, so two different Uppy instances will create the same file IDs and conflict with each other.The Uppy Tus uploader overrides the
fingerprint
option passed to tus-js-client, and the hard-coded implementation only takesFile.id
andTus.options.endpoint
into account. There is currently no way to change the fingerprint used to detect previous uploads and allow duplicate uploads with the Tus uploader.The text was updated successfully, but these errors were encountered: