0.9.5 Public Beta 3 support

194 Posts

Pandy06269 Reply #211, 20 years ago

I’ve sorted the file manager issues - found the code that chmod’s new directories and uploaded files to 0777 and changed it to 0770 (the default permissions for my webserver.)

The ’access is denied’ issue is on line 254 of manager/actions/files.dynamic.php:

if(substr(strtolower(str_replace('//','/',$startpath."/")), 0, $len)!=strtolower(str_replace('//','/',$filemanager_path."/"))) {

This compares for example, on my system, a $startpath of:
/home/wwwserver/andyshellam.eu/dev/alpha

with a $filemanager_path of:
/home/wwwserver/andyshellam.eu/dev/alpha/

- they never equal because the ending slash is stripped out of $startpath.
I believe this is when the code that determines $len executes, $startpath does not have an ending slash, therefore when the slash is added at line 254, $len does not reflect this.

So, what I did, was to add 1 to $len at line 254 as follows:

if(substr(strtolower(str_replace('//','/',$startpath."/")), 0, $len+1)!=strtolower(str_replace('//','/',$filemanager_path."/"))) {

Seems to work - and the rev.1533 problem of gif’s being denied upload is also fixed grin

so I’m a happy bunny again and off to keep playing...

194 Posts

Pandy06269 Reply #212, 20 years ago

Another fix...

Still on manager/actions/files.dynamic.php, on line 17 is a variable $newfolderaccessmode which is assigned the value 0777 and used throughout the "create folder" functions and "unzip .zip file" functions, however, the access permissions for a new file are hard-coded.

Therefore, I propose the following changes:

Add a new variable at line 18 "$newfileaccessmode = 0777;"

Change the current line 177, from:

@chmod($_POST['path']."/".$userfile['name'], 0777);

to:

@chmod($_POST['path']."/".$userfile['name'], $newfileaccessmode);

so that it takes the permissions set at line 18.
Or, even better, set these chmod permissions in the manager configuration interface.

I personally think 0777 is too dangerous, and should be at least restricted to 0755.
Also, shouldn’t files be 0666 (ie. write, not execute?)
(My default server permissions are writeable by webserver user/group only, and other users can’t even read - ie. 0770 for folders, 0660 for files.)

Just come across an issue - the $newfolderaccessmode variable isn’t taken note of - I changed it to 0770, created a folder, but it was still world-writeable.

So, I’ve changed Raymond’s create folder function to the following - this is lines 330-340.

// Create folder here
	if($_REQUEST['mode']=="newfolder") {
		$old_umask = umask(0);
		$foldername =  str_replace("..\\","",str_replace("../","",$_REQUEST['name']));
		if(!mkdirs($startpath."/$foldername",$newfolderaccessmode)) {
			echo "<span class='warning'><b>".$_lang['file_folder_not_created']."</b></span><br /><br />";
		} else {
			if (!@chmod($startpath."/$foldername",$newfolderaccessmode)) {
				echo "<span class='success'><b>".$_lang['file_folder_not_created']."</b></span><br /><br />";
			} else {
				echo "<span class='success'><b>".$_lang['file_folder_created']."</b></span><br /><br />";
			}
		}
		umask($old_umask);
	}

What this will do is create a folder and chmod it to the $newfolderaccessmode variable using a separate function call - I’ve checked and it works. What will need to be done however, is the second "$_lang[’file_folder_not_created’]" will need to be changed to a new language setting like "Folder was created, but couldn’t set it’s access permissions.")

MODX Staff
10,725 Posts

opengeek Reply #213, 20 years ago

Great feedback, everyone, but I have just one request...

Please, if you have requests to change functionality or you are confident you’ve found a bug in the beta’s, please add an appropriate bug report/feature request with your suggested code changes there, unless you are unsure if it is a bug or good idea, and you are just asking questions. Feel free to link to forum discussions about specific issues, but I’d really like to see every proposed change to MODx code be accompanied by a Flyspray bug number. Otherwise, it’s likely many of these good catches and great additions to the code will be overlooked at some point, either in this release, or in other branches that are being developed in parallel.

Let’s try and keep this forum limited to questions or quick informative statements like "Here’s a bug I found in 1533, ...[some details]..., see FS#967 for a suggested fix", and save more detailed error information and suggested code changes for the bug report itself. This will help us tackle some of these issues much more quickly...in other words, if I don’t see a bug report or feature request, you’re not allowed to cry if the fix or feature doesn’t make it into a release.

Stay tuned for a new public beta today...

Jason Coward
Chief Architect @ MODX
http://www.jasoncoward.com | http://twitter.com/drumshaman | https://github.com/opengeek

283 Posts

OncleBen31 Reply #214, 20 years ago

OK understood wink

,

So it will be good to have no hard-coded list for user and webuser States. I have create a task FS#583

FS#583 — user States information hard coded

Follow my contributions to open source on Oncleben31.cc

194 Posts

Pandy06269 Reply #215, 20 years ago

Okies, sorry up until now I hadn’t realised the Bug Tracker existed, so... sounds like a plan

FS#584 [BUG] for the file manager ’access denied’ issue.
FS#585 [FEATURE] to add "uploaded file/folder permissions mode" to system config
FS#586 [BUG] created folders don’t take the value of $newfolderaccessmode

502 Posts

ajayre Reply #216, 20 years ago

OK, TinyMCE problem added #587: http://modxcms.com/bugs/task/587

Andy

MODX Staff
12,272 Posts

Ryan Thrash Reply #217, 20 years ago

Public beta 3 testing is now closed. If you have unresolved issues with the beta, please download the latest public beta 5 release, which contains MANY updates and tweaks. There is likewise a new Public beta 5 support thread; if anything remains from this long forum topic, please address it under the new guidelines there.

Thanks for all your feedback to date!

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me