Some of the PHP code I'm not so sure about

I have a PHP code snippet that deletes a directory and all files in it, if any. However, I'm not too sure about this, it seems to me that it will also delete all sub-cards and then files in them, etc.

Basically I want to give and optional true / false parameter to select getter or not remove sub directories. Or would it be better to do two functions? The first is to completely delete the folder and seconds to delete both the folder and everything in it.

Here's the code:

function delete_directory($dirname) {
   if (is_dir($dirname)) {
     $dir_handle = opendir($dirname);
     if (!$dir_handle) return false;
     while($file = readdir($dir_handle)) {
       if ($file != "." && $file != "..") {
         if (!is_dir($dirname."/".$file)){
         @unlink($dirname."/".$file);
       }else {
         delete_directory($dirname.'/'.$file);
         }
       }
     }
     closedir($dir_handle);
   }
   @rmdir($dirname) or die("Could not remove directory.");
   return true;
 }

      

And what I'm basically wondering is: what could go wrong here? Is there a situation where this part of the code can seriously deteriorate? I've been debugging it with Netbeans for hours and tried many different scenarios. Now I'm a little stuck and wondering if the guys at StackoverFlow can find flaws in the code?

+1


source to share


7 replies


And what I'm basically wondering is: what could go wrong here? Is there a situation where this piece of code could seriously go wrong?



If the directory contains a symbolic link elsewhere, it will follow (if I'm not mistaken - you should check that). This can lead to a fire where you basically wipe out your entire filesystem. You probably want to protect against this by usingrealpath

+5


source


As others have said:

   if ($file != "." && $file != "..") {

      



it should be

   if ($file != "." && $file != ".." && !is_link($file)) {

      

+3


source


PHP is_dir returns true even if the argument is a symbolic link pointing to a directory, which is really dangerous. So just add a test! Is_link in the first case, if the assertion should make it much safer.

+2


source


Basically I want to give and an optional true / false parameter to select whether or not to remove sub directories.

If you don't remove subdirectories, you cannot remove the parent directory (since it must be empty first). If you design your delete_directory function so that it sometimes deletes only a directory, then you are asking for problems down the track.

If you need to delete all files in a directory and its subdirectories, I would create a function for that, name it deleteDirectoryFiles()

or whatever. The function deleteDirectory

could then call that and then just clean up the empty directories.

+1


source


What could go wrong? Many years ago I wrote a function to do something like this (delete the temporary working directory recursively). However, a mistake during development resulted in deleting our entire source directory instead! It was long before I knew what source control was and my backups were out of date, so I had to use DOS Norton Utilities to do my best to restore the original code.

Always test this function very carefully.

+1


source


I tried using this code but it was missing files in the tree and was unable to delete the directory.

I replaced

while($file = readdir($dir_handle))

      

from

while(false !== ($file = readdir($dir_handle)))

      

as per PHP guidelines and who fixed it.


I also mentioned the following:

if ($file != "." && $file != ".." && !is_link($file)) {

      

+1


source


I would provide an optional "levels" parameter that would allow you to limit the number of levels you want to go to, in case you only want to remove to a certain number of levels.

Another thought would be to have a "filesonly" flag, which would allow you to keep the directory structure but delete all files.

0


source







All Articles