?
Solved

Performance: Check if Safe to Modify File

Posted on 2009-02-10
10
Medium Priority
?
223 Views
Last Modified: 2012-05-06
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;
    } 
 
?>

Open in new window

0
Comment
Question by:hankknight
  • 6
  • 4
10 Comments
 
LVL 111

Expert Comment

by:Ray Paseur
ID: 23601910
How often (hits/second) do you hit this segment of code?

Maybe combine into one function?
0
 
LVL 16

Author Comment

by:hankknight
ID: 23602004
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))
0
 
LVL 111

Expert Comment

by:Ray Paseur
ID: 23602008

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

Open in new window

0
Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
LVL 111

Expert Comment

by:Ray Paseur
ID: 23602048
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!
0
 
LVL 16

Author Comment

by:hankknight
ID: 23603819
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:
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-writable
That is why I used both is_writable() and file_exists() however I suppse I could use !is_dir() instead.  What are you thoughts?
0
 
LVL 111

Expert Comment

by:Ray Paseur
ID: 23603869
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?
0
 
LVL 16

Author Comment

by:hankknight
ID: 23604459
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?
0
 
LVL 111

Expert Comment

by:Ray Paseur
ID: 23605632
"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?
0
 
LVL 16

Author Comment

by:hankknight
ID: 23621736
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.
<?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;
}
 
?>

Open in new window

windowsFolder.png
0
 
LVL 111

Accepted Solution

by:
Ray Paseur earned 2000 total points
ID: 23622577
Thanks, I didn't say it could not exist -- I just said I have never seen it and I would not find it acceptable.  Perhaps I have just been fortunate in my design choices?   Anyway, I would not permit a client to do something like creating a directory that had a dot in the name.  

To me this seems a little like having a flushing toilet in your family sedan.  Sure, you could do it.  But I've never seen it.  It's common on boats, but not in cars.

Good luck with your project.  It seems to have taken on an additional level of complexity now that you have that toilet to deal with!  Hope it doesn't slop over when you go around a curve.

;-)

Ray
0

Featured Post

Vote for the Most Valuable Expert

It’s time to recognize experts that go above and beyond with helpful solutions and engagement on site. Choose from the top experts in the Hall of Fame or on the right rail of your favorite topic page. Look for the blue “Nominate” button on their profile to vote.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Things That Drive Us Nuts Have you noticed the use of the reCaptcha feature at EE and other web sites?  It wants you to read and retype something that looks like this. Insanity!  It's not EE's fault - that's just the way reCaptcha works.  But it i…
It’s a season to be thankful, and we’re thankful for users like you who engage on site, solve technology problems, and network with others in the industry. What tech are we most thankful for? Keep reading.
The viewer will learn how to dynamically set the form action using jQuery.
The viewer will learn how to count occurrences of each item in an array.
Suggested Courses
Course of the Month16 days, 5 hours left to enroll

850 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question