A piece of PHP code I'm not so sure about.

Question

I have here a piece of PHP code that deletes a directory and all files in it if present. However, I'm not too sure about it, it looks to me like it'll delete all sub-maps too and then files in those and so on...

I basically want to give and optional true/false parameter to select wheter or not to delete sub directories. Or would it be better practice to make 2 functions? The first to completely empty the folder and the seconds the 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 can go wrong here? Is there a situation where this piece of code can seriously screw up? I've been debugging it with Netbeans for a few hours now, and tried a lot of different scenarios. Now I'm kinda stuck and wondering if the guys at StackoverFlow can find a flaw in the code?

Answer 1

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

If the dir contains a symlink to somewhere else, it would be followed (unless I'm mistaken - You should check this). That could lead to a wildfire, where you basically wipe out your entire file system. You would probably want to safeguard against this, using realpath

Answer 2

I basically want to give and optional true/false parameter to select whether or not to delete sub directories.

If you don't delete the sub-directories then you can't delete the parent directory (since it must be empty first). If you build your delete_directory function such that it will only sometimes delete a directory, then you're asking for problems down the track.

If you need to delete all the files in a directory and its sub-directories, then I'd create a function for that, call it deleteDirectoryFiles() or something. The deleteDirectory function could then call that, and then just clean up the empty directories.

Answer 3

I would provide an optional "levels" parameter that allowed you to limit how many levels deep you want to go, in the event you only want to delete up to a certain number of levels.

Another thought would be to have a "filesonly" flag that allowed you to keep the directory structure but remove all the files.

Answer 4

What can go wrong? Many years ago I wrote a function to do something like this (recursively delete a temporary working directory). However, a bug during development caused it to remove my entire source code directory instead! This was long before I knew what source control was, and my backups were out of date, so I had to use the DOS Norton Utilities to do my best to undelete my source code.

Always test such a function very, very carefully.

Answer 5

PHP is_dir returns true even if the argument is a symlink pointing to directory, which is really dangerous. So just add a !is_link test in the first if statement should make it a lot safer.

Answer 6

As others have said:

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

should be

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

Answer 7

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

I replaced

while($file = readdir($dir_handle))

with

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

as per the PHP recomendations and that fixed it.


I also used the following as others mentioned:

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