Post by djbrock » Sat Dec 20, 2008 1:34 am

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?

New member

Posts

Joined
Thu Dec 04, 2008 10:37 pm

User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by Qphoria » Sat Dec 20, 2008 1:55 am

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

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by djbrock » Sat Dec 20, 2008 1:58 am

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. 

New member

Posts

Joined
Thu Dec 04, 2008 10:37 pm

Post by Qphoria » Sat Dec 20, 2008 1:59 am

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

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by djbrock » Sat Dec 20, 2008 2:09 am

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. 

New member

Posts

Joined
Thu Dec 04, 2008 10:37 pm

Post by hm2k » Sat Dec 20, 2008 2:28 am

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.

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by djbrock » Sat Dec 20, 2008 2:47 am

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.

New member

Posts

Joined
Thu Dec 04, 2008 10:37 pm

Post by hm2k » Sat Dec 20, 2008 3:38 am

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

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by djbrock » Sat Dec 20, 2008 4:41 am

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. 

New member

Posts

Joined
Thu Dec 04, 2008 10:37 pm

Post by hm2k » Sat Dec 20, 2008 7:38 pm

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);

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by hm2k » Sun Dec 21, 2008 7:58 pm

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

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by bruce » Sun Dec 21, 2008 8:21 pm

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  :D

Active Member

Posts

Joined
Wed Dec 12, 2007 2:26 pm

Post by hm2k » Sun Dec 21, 2008 10:18 pm

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.

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by fido-x » Tue Dec 23, 2008 10:46 am

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.

Image
Modules for OpenCart 2.3.0.2
Homepage Module [Free - since OpenCart 0.7.7]
Multistore Extensions
Store Manager Multi-Vendor/Multi-Store management tool

If you're not living on the edge ... you're taking up too much space!


User avatar
Expert Member

Posts

Joined
Sat Jun 28, 2008 1:09 am
Location - Tasmania, Australia

Post by Qphoria » Tue Dec 23, 2008 11:21 am

good work fido :)

does this work regardless of a+r and 0644?

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by fido-x » Tue Dec 23, 2008 11:31 am

Qphoria wrote: 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/top ... l#msg12007), but hm2k decided to stick with "move_uploaded_file()" (not criticism - just observation, after all, using "move_uploaded_file()" should have worked).
Last edited by fido-x on Tue Dec 23, 2008 11:36 am, edited 1 time in total.

Image
Modules for OpenCart 2.3.0.2
Homepage Module [Free - since OpenCart 0.7.7]
Multistore Extensions
Store Manager Multi-Vendor/Multi-Store management tool

If you're not living on the edge ... you're taking up too much space!


User avatar
Expert Member

Posts

Joined
Sat Jun 28, 2008 1:09 am
Location - Tasmania, Australia

Post by hm2k » Tue Dec 23, 2008 5:16 pm

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.

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by hm2k » Tue Dec 23, 2008 7:46 pm

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;
	}

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by bruce » Tue Dec 23, 2008 8:15 pm

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

Active Member

Posts

Joined
Wed Dec 12, 2007 2:26 pm
Who is online

Users browsing this forum: No registered users and 5 guests