Fixing uplaod problem with rename method

[milosh012]

Hello,

there is a problem when /tmp dir is not on the same hard disk partition where I want to upload file. This is because php method rename. Quick solution is to use move_uploaded_file method instead of rename

Best, Milos P.S. Your component is cool.

[Gargron]

@milosh012 Can you describe your circumstances a bit more? PHP version, OS? I accept that move_uploaded_file would be a quick fix, but there is a reason I didn't use it – rename should be functionally equivalent _only without checking is_uploaded_file_ (which we do anyway). That abstraction and equivalency gives us a bit more space on Filesystem implementations.

[milosh012]

On my local setup (OSX), I have 2 partition. First one (main one) is non-case-sensitive and on that partition is located /tmp dir. Second partition is case-sensitve partition and on that partition are all my projects. PHP version is 5.4.15. On every upload I got this 500 HTTP error code, with error msg:

rename(/private/var/tmp/phpAzFqU0,/Volumes/data/projects/.../uploads/xxx.jpg): Operation not permitted

I have checked over the google and found some answers saying that this is because of different partitions.

I think that my request is fine and do not harm your component (only make it even better)...

[Gargron]

Have you checked if the target directory is writable by whatever user is executing the script? That could also cause "Operation not permitted"

[thers]

@milosh012 If you look at move_uploaded_file realization you will notice that this function is actually a shortcut for is_file_uploaded which realization you can find here and rename, so you should just check your write permissions as was mentioned by @Gargron :)

[thers]

Well, actually the only difference is that move_uploaded_file cleans up internal hash table :)

[Gargron]

Obviously the utility of the unit-test-ability is overshadowed by the utility of maximum compatibility, but I want to make sure this really is the reason of the error before changing rename to move_uploaded_file.

[thers]

@Gargron But how can decoupled version of move_uploaded_file break compatibility? All we need is just check php/php-src and even if you don't know C lang well you will notice that it just can not break compatibility.

[thers]

Also, @milosh012 you can change upload_tmp_dir to the dir on the same partition where your projects stored. Anyway problem is not in is_uploaded_file+rename or move_uploaded_file.

[milosh012]

@Gargron Of course that I checked that my dir is writable. I just replaced line where was rename() and put move_uploaded_file() and everything work perfect!

@RiderSx I think that changing upload_tmp_dir is not a solution - that is just forcing the rename method to work.

[Gargron]

@RiderSx It can break compatibility as per the docs, which state that from version 4.3.3 rename works with *nix partitions and from version 5.3.1 also with Windows drives, so at some point it must not have worked – and apparently it also doesn't work for @milosh012.

Now, I'm not sure. Will using copy and unlink mitigate the problem? (I am trying to avoid the coupled function that is move_uploaded_file, but we'll use it if all else fails).

[thers]

@Gargron yeah i'm already read it, also read conversation about mentioned bug. Your package, obviously, can not be used with PHP 4.3.3 :) so very strange that this bug appeared now. @milosh012 can check if copy and unlink works for his environment.

Actually this bug appeared because of destination filesystem restrictions for chown and chmod, as mentioned in changelog for 4.3.3, so yeah, i guess copy and unlink should work properly. Hope this solution will work :)

[thers]

But im still confused, in move_uploaded_file we can see VCWD_CHMOD(new_path, 0666 & ~oldmask); which means that if rename not work with destination FS then move_uploaded_file should not work too, it all means that problem not in chmod syscall, uh :(

[milosh012]

@Gargron @RiderSx I have tried copy && unlink and it works... Do you want to make another pull request with that approach?

[Gargron]

@milosh012 Yeah, you can do that :)