Merge pull request #335 from thekid/fix/unlink-symlinks

Fix removing directories with symlinks inside
xp-framework · Oct 9, 2023 · 95ecf1b · 95ecf1b
2 parents 4c5c52d + c9e27a8
commit 95ecf1b
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 17 deletions.
diff --git a/src/main/php/io/Folder.class.php b/src/main/php/io/Folder.class.php
@@ -144,27 +144,26 @@ public function create($permissions= 0700) {
    * @throws  io.IOException in case one of the entries could'nt be deleted
    */
   public function unlink($uri= null) {
-    if (null === $uri) $uri= $this->uri; // We also use this recursively
-
-    if (false === ($d= dir($uri))) {
+    $uri= null === $uri ? $this->uri : rtrim($uri, DIRECTORY_SEPARATOR).DIRECTORY_SEPARATOR;
+    if (false === ($d= opendir($uri))) {
       throw new IOException('Directory '.$uri.' does not exist');
     }
-
-    while (false !== ($e= $d->read())) {
-      if ('.' == $e || '..' == $e) continue;
-
-      $fn= $d->path.$e;
-      if (!is_dir($fn)) {
-        $ret= unlink($fn);
-      } else {
-        $ret= $this->unlink($fn.DIRECTORY_SEPARATOR);
+
+    while (false !== ($e= readdir($d))) {
+      if ('.' === $e || '..' === $e) continue;
+
+      // Recurse into subdirectories (but not if they are symlinks!)
+      $fn= $uri.$e;
+      if (0x4000 === (lstat($fn)['mode'] & 0xf000)) {
+        $this->unlink($fn);
+      } else if (false === unlink($fn)) {
+        closedir($d);
+        throw new IOException("Deleting '{$fn}' failed");
       }
-      if (false === $ret) throw new IOException(sprintf('unlink of "%s" failed', $fn));
     }
-    $d->close();
+    closedir($d);
 
-    if (false === rmdir($uri)) throw new IOException(sprintf('unlink of "%s" failed', $uri));
-
+    if (false === rmdir($uri)) throw new IOException("Deleting '{$uri}' failed");
     return true;
   }
 

diff --git a/src/test/php/io/unittest/FolderTest.class.php b/src/test/php/io/unittest/FolderTest.class.php
@@ -1,7 +1,8 @@
 <?php namespace io\unittest;
 
-use io\{Folder, FolderEntries, IOException, Path};
+use io\{Folder, FolderEntries, File, Files, IOException, Path};
 use lang\Environment;
+use test\verify\Runtime;
 use test\{After, Assert, Expect, Test};
 
 class FolderTest {
@@ -69,6 +70,61 @@ public function unlink() {
     Assert::false($f->exists());
   }
 
+  #[Test, Expect(IOException::class)]
+  public function unlink_nonexistant() {
+    $f= new Folder($this->tempFolder());
+    $f->unlink();
+  }
+
+  #[Test]
+  public function unlink_with_subdir() {
+    $f= new Folder($this->tempFolder());
+    $f->create();
+
+    $s= new Folder($f, 'sub');
+    $s->create();
+
+    $f->unlink();
+    Assert::false($f->exists());
+  }
+
+  #[Test]
+  public function unlink_with_file() {
+    $f= new Folder($this->tempFolder());
+    $f->create();
+
+    Files::write(new File($f, 'file'), 'Test');
+
+    $f->unlink();
+    Assert::false($f->exists());
+  }
+
+  #[Test, Runtime(os: 'Linux')]
+  public function unlink_with_file_symlink() {
+    $f= new Folder($this->tempFolder());
+    $f->create();
+
+    $target= new File($f, '.default.ini');
+    Files::write($target, 'key=value');
+    symlink('.default.ini', $f->getURI().'config.ini');
+
+    $f->unlink();
+    Assert::false($f->exists());
+  }
+
+  #[Test, Runtime(os: 'Linux')]
+  public function unlink_with_folder_symlink() {
+    $f= new Folder($this->tempFolder());
+    $f->create();
+
+    $target= new Folder($f, '.default');
+    $target->create();
+    symlink('.default', $f->getURI().'config');
+
+    $f->unlink();
+    Assert::false($f->exists());
+  }
+
   #[Test]
   public function uriOfNonExistantFolder() {
     $temp= $this->tempFolder();