Image permissions 0000

[djbrock]

I'm not sure if this is an OC issue or a host issue, but since upgrading from 0.7.8 to 0.7.9RC5 all the images I upload via OC have permissions set at 000 and I have to change them through my host (hostmonster). Is anyone else having this trouble?

[hm2k]

Is this similar to this:

http://forum.opencart.com/index.php/top ... ml#msg8236
http://forum.opencart.com/index.php/topic,1891.0.html

If so a solution was posted here:

http://forum.opencart.com/index.php/topic,2367.0.html
http://code.google.com/p/open-cart/source/detail?r=236

See if that fixes it for you.

[Qphoria]

But that fix is in RC5 so it should already fix it for you.

[djbrock]

Thanks for the response from both of you.  I replaced this is upload.php

function save($key, $file)
        {
                if (file_exists($file)) @unlink($file);
                $status=@move_uploaded_file($_FILES[$key]['tmp_name'], $file);
                if ($status) @chmod($file, 'a+r');
                return $status;
        }     

with this from http://forum.opencart.com/index.php/topic,1891.0.html

function save($key, $file)
{
if (file_exists($file))
{
@unlink($file);
}
$status = @move_uploaded_file($_FILES[$key]['tmp_name'], $file);
if ($status)
chmod($file, 0644);
return $status;
}

and it fixes the problem. 

[Qphoria]

If you didn't already have that, then it doesn't seem like you properly upgraded 0.7.8 to 0.7.9RC5

[djbrock]

Unless some other contrib I've used reverted it.  I actually completely deleted the directory and installed fresh from 0.7.9RC5 just last week because of such problems. 

Before that I'd used WINscp to mirror the files from the \upload directory to the \store directory and then ran the upgrade.php. 

[hm2k]

If you could, could you change the code in the original from:

Code: Select all

if ($status) @chmod($file, 'a+r');

to

Code: Select all

if ($status) chmod($file, 'a+r');

And try again, this should give you an error, tell me what that error is.

Meanwhile, can you also test that changing it to:

Code: Select all

if ($status) @chmod($file, '0644');

Solves the problem, without any further changes being needed...

I need you to test this as I'm not seeing this issue in my environments.

[djbrock]

I made the change to

if ($status) chmod($file, 'a+r');

but instead of an error, I got a "success" message with no icon showing.

I tried the second option

if ($status) @chmod($file, '0644');

again, a success message but no icon showing.

[hm2k]

Are you saying that it didn't solve your problem, and there was no error?

[djbrock]

It did not solve the problem.  In fact, it reproduced the problem.  A proper upload of the image with the "success" notification, but no visible image. 

[hm2k]

I really don't understand that, the only difference between the two functions you posted is the chmod...

Try this:

Code: Select all

if ($status) @chmod($file, 0644);

[hm2k]

I'm also now seeing this issue. I'm going to do some more tests later today.

[bruce]

Hi hm2k,

One interesting thing I have noticed after various implementations is the user that apache runs under. Sometimes it is the same as the ftp user which makes everything easy. Other times it is not and I cannot even view or download log files.

There may be some interaction here between the folder permissions and owner (ftp user) and the apache user.

Or I may just be dreaming 

[hm2k]

Yes, it could well be a chown issue rather than chmod, or something along those lines.

However, my production server for the live site i'm testing runs suphp, so the apache/php user and ftp user should all be the same.

I will investigate it fully though a bit later on.

[fido-x]

I've managed to recreate this problem on my own test server. I found that setting permissions to either "a+r" or "0644" is causing problems. "a+r" seems to set the permissions to "000" making the file unreadable by anyone, whereas setting permissions to "0644" is also not working properly (I'm ending up with something like "r-------T").

How I resolved the problem was by changing the following line in the save function of "library/filesystem/upload.php":-

Code: Select all

$status=@move_uploaded_file($_FILES[$key]['tmp_name'], $file);

to

Code: Select all

$status=@copy($_FILES[$key]['tmp_name'], $file);

Using "copy()" instead of "move_uploaded_file()" sets the permissions correctly. The temporary file created during the upload process should be deleted automatically when the script finishes executing, ie. after the file has been copied.

Fido-X.

[Qphoria]

good work fido

does this work regardless of a+r and 0644?

[fido-x]

does this work regardless of a+r and 0644?

Yes. You can even delete the line that reads-

Code: Select all

if ($status) @chmod($file, 'a+r');

since "copy()" sets the permissions correctly.

PS
I have mentioned this elsewhere on the forum (http://forum.opencart.com/index.php/topic,1891.msg12007.html#msg12007), but hm2k decided to stick with "move_uploaded_file()" (not criticism - just observation, after all, using "move_uploaded_file()" should have worked).

[hm2k]

The option before was to use move_uploaded_file() && chmod() or copy() && unlink().

I decided to use move_uploaded_file() because its more related to what we're actually trying to do.

However, if we can't use chmod() in all situations, then we can't give the file read permissions, meaning that copy() && unlink() is our only option.

I will update the code accordingly.

[hm2k]

Does anyone have any issues with this?

Code: Select all

   function save($key, $file) {
      if (file_exists($file)) @unlink($file);
      $status=@copy($_FILES[$key]['tmp_name'], $file);
      if ($status) @unlink($_FILES[$key]['tmp_name']);
      return $status;
   }


[bruce]

Only one... very small one 

Firstly, from the php manual...

The file will be deleted from the temporary directory at the end of the request if it has not been moved away or renamed.

so the unlink is redundant, but conservative and there is certainly no harm in that.

If we are playing conservatively, I would suggest that you also leave in the original chmod with the numeric parameter so...

Code: Select all

   function save($key, $file)
   {
      if (file_exists($file))
      {
         @unlink($file);
      }
      $status = @copy($_FILES[$key]['tmp_name'], $file);
      if ($status)
      {
         @chmod($file, 0644);
         @unlink($_FILES[$key]['tmp_name']);
      }
      return $status;
   }


cheers

Bruce