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

Uploaded binary files differ from originals #2

Closed
NewPlayer2 opened this issue Oct 28, 2015 · 6 comments
Closed

Uploaded binary files differ from originals #2

NewPlayer2 opened this issue Oct 28, 2015 · 6 comments
Labels

Comments

@NewPlayer2
Copy link

Installed with composer and used the upload.php code from under "How to use".

Large finished files under the uploads directory DO NOT match the originals. This makes the whole script useless.

@dilab
Copy link
Owner

dilab commented Oct 30, 2015

What kind of binary files did you try to upload?

And was the file size matching?

More information would be helpful.

@georaldc
Copy link
Contributor

A few issues I see here:

  1. When combining chunked files, the order is messed up due to not being naturally sorted. Running natsort against the $chunkFiles array inside the createFileFromChunks should fix this.
  2. Sometimes, the folder holding your temp chunked files gets deleted first before the process of combining files gets completed. You then end up with errors such as being unable to call file_get_contents('/path/to/chunked/file') due to the file getting deleted.
  3. Haven't tested this fully yet but it looks like fopen in append mode doesn't work well with certain types of files like large binary files. Cake's File class' append method uses this and well, it just breaks the resulting file. Modifying the createFileFromChunks() method to do something similar to how the other PHP implementation sample (https://github.com/23/resumable.js/blob/master/samples/Backend%20on%20PHP.md) works fixes this issue. Quick example below
public function createFileFromChunks($chunkFiles, $destFile)
{ 
    natsort($chunkFiles);
    if (($fp = fopen($destFile, 'w')) !== false) {
        foreach ($chunkFiles as $chunkFile) {
            if (fwrite($fp, file_get_contents($chunkFile))) {
                unlink($chunkFile);
            }
        }
        fclose($fp);
    }
    return true;
}

I would imagine combining these 3 should fix the issues encountered by @NewPlayer2

EDIT:
I may be wrong with _#_2. I think the reason for that error is because on certain uploads, the isFileUploadComplete method returns true multiple times at the end of some chunks. Might be something wrong with your math but I think you can just use the resumableTotalChunks value to determine the exact number of chunks resumable has created. The error in _#_3 might also be related to this.

@dilab
Copy link
Owner

dilab commented Dec 19, 2015

I will try to replicate the issues as soon as I have some free time.

Looks like I will have to test couple of file formats:

  1. video
  2. binary

@dilab
Copy link
Owner

dilab commented Dec 20, 2015

added condition to delete tmp folder only when file is combined from chunks.

@dilab dilab added the bug label Dec 20, 2015
@dilab
Copy link
Owner

dilab commented Dec 20, 2015

Issues should be fixed with 0.1.2.

@dilab dilab closed this as completed Dec 20, 2015
@daitam
Copy link

daitam commented Apr 21, 2017

The problem still persists with partly uploaded ZIP files. If you try to reupload after page refresh, then uploaded file is corrupted and file size doesn't match original file.

Solved this by using @georaldc solution:

public function createFileFromChunks($chunkFiles, $destFile)
{ 
    $this->log('Beginning of create files from chunks');

    natsort($chunkFiles);
    
    if (($fp = fopen($destFile, 'w')) !== false) {
        foreach ($chunkFiles as $chunkFile) {
            fwrite($fp, file_get_contents($chunkFile));

            $this->log('Append ', ['chunk file' => $chunkFile]);
        }
        fclose($fp);
    }

    $this->log('End of create files from chunks');
    return true;
}

Also dropped out unlink(...), because we have $tmpFolder->delete(); which deletes whole folder and everything inside it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants