hankknight
asked on
Performance: Check if Safe to Modify File
My code works nicely however I wanted to know if I would gain performance benefits by writing it differently.
<?php
$path = "assets/test.html";
if (checkFile ($path)==TRUE)
echo 'You may write to this file.';
else echo 'You may NOT write to this file.';
function checkFile ($path) {
if (file_exists($path) && is_writable($path) && (checkExt($path,array('php','inc','hta','ini', 'cms', 'dat', 'exe', 'dll','cgi','pl'))===FALSE))
return TRUE;
else return FALSE;
}
function checkExt ( $file, $ext ) {
$found = false;
foreach( $ext as $value ) {
if( @strpos ( strtolower( $value ), strtolower(pathinfo($file,PATHINFO_EXTENSION))) !==FALSE ) {
$found = true;
break;
}
}
return $found;
}
?>
ASKER
This may be called 5 times a second tops.
Which is better?
if (!file_exists($path) return FALSE;
if(!is_writable($path) return FALSE;
if(checkExt($path!==FALSE) return FALSE;
return TRUE;
OR
if (file_exists($path) && is_writable($path) && (checkExt($path)===FALSE))
Which is better?
if (!file_exists($path) return FALSE;
if(!is_writable($path) return FALSE;
if(checkExt($path!==FALSE)
return TRUE;
OR
if (file_exists($path) && is_writable($path) && (checkExt($path)===FALSE))
<?php // RAY_temp_hankcheck.php
function checkFile($path)
{
$bad = array('php','inc','hta','ini', 'cms', 'dat', 'exe', 'dll','cgi','pl');
$ext = pathinfo($path, PATHINFO_EXTENSION);
if (empty($ext)) return FALSE;
if (in_array($ext, $bad)) return FALSE;
if (!is_writable($path)) return FALSE;
return TRUE;
}
is_writable() includes a check for file_exists() so it is not necessary to check twice.
But there is a philosophical consideration here. The function looks for a set of illegal file extensions and will not permit writing to those. A better test might be a list of acceptable file extensions. The filters that accept "known good" values are almost always safer than the filters that reject known bad values. All it takes for a catastrophe in the latter structure is to have forgotten one of the bad values!
But there is a philosophical consideration here. The function looks for a set of illegal file extensions and will not permit writing to those. A better test might be a list of acceptable file extensions. The filters that accept "known good" values are almost always safer than the filters that reject known bad values. All it takes for a catastrophe in the latter structure is to have forgotten one of the bad values!
ASKER
Ray_Paseur, you said that is_writable() includes a check for file_exists() so it is not necessary to check twice.
The PHP website says:
The PHP website says:
Returns TRUE if the filename exists and is writable. The filename argument may be a directory name allowing you to check if a directory is writable. -- http://php.net/is-writableThat is why I used both is_writable() and file_exists() however I suppse I could use !is_dir() instead. What are you thoughts?
I don't know where the $path variable comes from. What you do with checking is at least partially dependent on the prior validation of that field.
I think we covered the possibility that it was a directory by trying to get the file extension, right? Line 6?
I think we covered the possibility that it was a directory by trying to get the file extension, right? Line 6?
ASKER
The user may change $path so I need good checks in place.
You said that we covered the possibility that it was a directory by trying to get the file extension.
Can't a directory have a dot in the directory name in some operating systems?
You said that we covered the possibility that it was a directory by trying to get the file extension.
Can't a directory have a dot in the directory name in some operating systems?
"Can't a directory have a dot in the directory name in some operating systems?" I've never seen it. It may be possible, but I would never let that happen. Have you tried getting a file extension from a directory?
ASKER
Ray, you said you've never seen a dot in the directory name in any operating system.
I tested this on Windows and you may be surprised by the result.
I tested this on Windows and you may be surprised by the result.
<?php
echo pathinfo('this.is.a.windows.xp.folder.txt', PATHINFO_EXTENSION);
// returns txt even though this.is.a.windows.xp.folder.txt is a folder
if (checkfile('this.is.a.windows.xp.folder.txt')==TRUE) echo "TRUE"; else echo "FALSE";
// Returns true though this.is.a.windows.xp.folder.txt is a folder
function checkFile($path)
{
$bad = array('php','inc','hta','ini', 'cms', 'dat', 'exe', 'dll','cgi','pl');
$ext = pathinfo($path, PATHINFO_EXTENSION);
if (empty($ext)) return FALSE;
if (in_array($ext, $bad)) return FALSE;
if (!is_writable($path)) return FALSE;
return TRUE;
}
?>
windowsFolder.png
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Maybe combine into one function?