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

Files are appended together when uploading two different files with same filename or same file with same filename uploaded from two browsers #52

Closed
airani opened this issue Jun 3, 2018 · 18 comments
Assignees
Labels
Projects

Comments

@airani
Copy link

airani commented Jun 3, 2018

If upload two diffrent files with same filename or same file with same filename upload from two browsers files append together

@ankitpokhrel
Copy link
Owner

If upload two different files with same filename

Uploads are differentiated using unique Upload-Key and it has nothing to do with filename. Can you check if your upload metadata is being saved to cache?

same file with same filename upload from two browsers files append together

Again, I suspect there is some cache issue in your setup. If you upload same file again with same Upload-Key it will get append together. The state of previous upload is fetched from cache to resume the upload. You need to use different Upload-Key to upload same file again. To set Upload-Key you can use $client->setKey($uploadKey) method.

@airani
Copy link
Author

airani commented Jun 7, 2018

I used File cache and works fine and problem not solved!

Again, I explain problem:

I Upload a.mp4 (100mb) file from chrome browser and it's successfully uploaded in /storage path. Again, I upload a.mp4 (100mb) file from firefox browser and it's successfully uploaded in /storage path. But /storage/a.mp4 size is 200mb now!

@ankitpokhrel
Copy link
Owner

@airani can you provide cache contents for those cases. i.e first, upload a file and copy its cache key and contents. Then, upload same file again from another browser and copy cache key and contents.

@ankitpokhrel ankitpokhrel added this to To Do in Kanban Jun 15, 2018
@ankitpokhrel ankitpokhrel moved this from To Do to In progress in Kanban Jun 15, 2018
@ankitpokhrel
Copy link
Owner

Closing because of no response

Kanban automation moved this from In progress to Done Jul 13, 2018
@Yangwendaxia
Copy link

@ankitpokhrel this issue should not be closed.
As @airani described, if we use two different browsers to upload the same file, all of them will send a POST HTTP request, it will lead to writing a file in the same directory, then Patch HTTP request will append chunked files to this file, I think it is the root cause of file appending issue.

@ankitpokhrel
Copy link
Owner

@Yangwendaxia @airani Looks like you guys are trying to upload same file from different browsers in parallel. In that case, of course, one request is going to override another as the package is not designed for the distributed upload. But I agree that it would be great if we could make it work for distributed uploads.

@drastik
Copy link

drastik commented Sep 26, 2018

@ankitpokhrel
This is not only when attempting parallel.

I'm using Uppy as the frontend widget with TUS PHP in the back. No matter what the set of circumstances (same browser, different, etc); if you upload the same filename to same directory, it appends it on to the existing file.

To reproduce:
Upload a file for the first time, then delete the entry that Uppy/tus-js-client makes in your browser's local storage for that file.
Upload it again, and instead of making the usual HEAD call first, it will go straight to POST, then tus-php will concatenate.

@ankitpokhrel
Copy link
Owner

@drastik When you clear browser local storage, uppy will send new request for the upload. Here, the file is being appended because file with same name already exist in the server. If you want to prevent that, you can organize your uploads in the server. For instance, you can prefix your file name using upload key or save all new uploads in a separate folder.

You can change server.php from examples to the following. It will save all new uploads in a separate folder.

<?php

require __DIR__ . '/../vendor/autoload.php';

$server = new \TusPhp\Tus\Server('redis');

if (strtolower($server->getRequest()->method()) === 'post') {
    $baseDir    = __DIR__ . '/../uploads/';
    $uploadPath = $baseDir . $server->getUploadKey();

    if ( ! is_dir($uploadPath)) {
        mkdir($uploadPath);
    }

    $server->setUploadDir($uploadPath);
}

$response = $server->serve();

$response->send();

exit(0); // Exit from current PHP process.

@ankitpokhrel ankitpokhrel changed the title If upload two diffrent files with same filename or same file with same filename upload from two browsers files append together If upload two different files with same filename or same file with same filename upload from two browsers files append together Sep 27, 2018
@drastik
Copy link

drastik commented Sep 27, 2018

Right.

@drastik
Copy link

drastik commented Sep 27, 2018

Oops, early comment.

@ankitpokhrel
Do you think that is a good behavior though? To upload the same file and concatenate into existing, causing a broken file. I patched tus-php's handlePost() to check if we have the upload key in cache, and if we have full filesize already. If we do, it sends request back without trying to create or concatenate it again.

Do you see an issue with this approach?

I understand your suggestion as well, and I may do that.. though I had wanted to avoid renaming user's files or destination folders, since my other framework was handling all of that.

@samundra
Copy link
Collaborator

Wow! You guys are having an interesting discussion going on. When I have sometime I will try to reproduce the error at my end and will try to figure out.

Regarding the actual query, I think there's no way for server to know which file belong to which client other than doing the manual tagging on the file itself. So, like already provided here, the easiest solution would be to identify the client at application level and handle the uploads accordingly. Otherwise, have to send extra metadata with file itself to identify the individual client and respective uploaded contents which I think is extra work for Tus-PHP client and server end.

@ankitpokhrel
Copy link
Owner

Reopening this issue because of ongoing discussions

@ankitpokhrel ankitpokhrel reopened this Sep 28, 2018
@drastik
Copy link

drastik commented Sep 28, 2018

Can we match the file from client to cache by checksum?

@ankitpokhrel
Copy link
Owner

I patched tus-php's handlePost() to check if we have the upload key in cache, and if we have full filesize already. If we do, it sends request back without trying to create or concatenate it again.

@drastik Won't you have same issue again after you clear your server cache (same as you cleared localStorage)?

Can we match the file from client to cache by checksum?

We can do it only if all clients sends checksum on their request. But it is not the case even in uppy. That is why we have this condition to ignore checksum if we don't get any checksum in request header.

tus-php/src/Tus/Server.php

Lines 617 to 623 in 9a34d98

protected function getClientChecksum()
{
$checksumHeader = $this->getRequest()->header('Upload-Checksum');
if (empty($checksumHeader)) {
return '';
}

@drastik
Copy link

drastik commented Sep 29, 2018

I like your suggestion for folders based on UUID @ankitpokhrel .

I think we can close this issue.

Thank you very much for this project by the way. I have a patch I will commit for properly finding filename from Upload-metadata in the case where your client sends extra values, I will create a separate issue for the PR.

@AurelienMendes
Copy link

AurelienMendes commented Nov 27, 2018

Hi everybody !
Since I faced the same problem, I would like to propose/discuss an optional solution.

The context : we have a folder "TEST" with inside a file a1.jpg and a "SUBTEST" subfolder with inside a different file but with the same name, "a1.jpg"
Thus we have
TEST/a1.jpg
TEST/SUBTEST/a1.jpg

Now in Server class, in the handlePost method
$filePath = $this->getUploadDir() . DIRECTORY_SEPARATOR . $fileName;
if(Config::get('naming') === 'rename'){
/*
* Change filePath
*/
$ext = explode('.', $fileName);
$ext = end($ext);
$filePath = $this->getUploadDir() . DIRECTORY_SEPARATOR . $this->getUploadKey() . '.' . $ext;
}

we can change the filename so that on the server it's now unique (based on uuid).
In Config/default.php we let the user choose :
naming => 'rename' //but could be 'initial to keep it unchanged'

This way, there is no concatenation as discussed aboved.
Does it sound good for you ?

Shusss,
Aurélien

@HollyIT
Copy link

HollyIT commented Dec 15, 2018

I've been dealing with the same problem. What I've come up with, which seems to be working great with testing (resumes with no problem and no filename collisions) is to extend the Server class and copy the handlePost() method into my new class, then simply prepend a uniqid() to the fileName in the path:

  $filePath  = $this->uploadDir . DIRECTORY_SEPARATOR . uniqid() . '_' . $fileName;

This got me thinking that maybe a better, permanent solution would be to offer the possibility of a $filePath callback. I tried this out in my extended class and it's working the same as above:

    public function setFilePathCallback($callback)
    {
        if (is_callable($callback)) {
            $this->filePathCallback = \Closure::bind($callback, $this, get_class());
        } else {
            $this->filePathCallback = null;
        }
    }

Now in the handlePost():

        $filePath  = $this->filePathCallback instanceof \Closure ? call_user_func($this->filePathCallback, $fileName) : $this->uploadDir . DIRECTORY_SEPARATOR . $fileName;

Which finally gives you the ability to simply:

            $server = new Server('redis');
            $server->setFilePathCallback(function($fileName) {
                return  $this->uploadDir . DIRECTORY_SEPARATOR . uniqid() . '_' . $fileName;
            });

@ankitpokhrel ankitpokhrel self-assigned this Jan 14, 2019
@ankitpokhrel ankitpokhrel changed the title If upload two different files with same filename or same file with same filename upload from two browsers files append together Files are appended together when uploading two different files with same filename or same file with same filename upload from two browsers Jan 14, 2019
@ankitpokhrel ankitpokhrel changed the title Files are appended together when uploading two different files with same filename or same file with same filename upload from two browsers Files are appended together when uploading two different files with same filename or same file with same filename uploaded from two browsers Jan 14, 2019
@rajpt
Copy link

rajpt commented Aug 31, 2022

What about if we implement same approach as OS deals with same file names?
Create sample.text first time. (considering it is not exist in the upload dir)

if user is trying to upload again sample.text and that file is already exist in the directory then create new file sample-1.text

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Kanban
  
Done
Development

No branches or pull requests

8 participants