Issue #3482449 by alexpott, smustgrave, nicxvan:...

Issue #3482449 by alexpott, smustgrave, nicxvan: \Drupal\Core\File\FileSystem::deleteRecursive() will follow symlinks and remove files outside the directory it is deleting

(cherry picked from commit ed78b812)

core/lib/Drupal/Core/File/FileSystem.php

@@ -314,6 +314,13 @@ public function copy($source, $destination, /* FileExists */$fileExists = FileEx
    * {@inheritdoc}
    */
   public function delete($path) {
+    if (is_link($path)) {
+      // See https://bugs.php.net/52176.
+      if (!($this->unlink($path) || '\\' !== \DIRECTORY_SEPARATOR || $this->rmdir($path)) && file_exists($path)) {
+        throw new FileException("Failed to unlink symlink '$path'.");
+      }
+      return TRUE;
+    }
     if (is_file($path)) {
       if (!$this->unlink($path)) {
         throw new FileException("Failed to unlink file '$path'.");
@@ -340,15 +347,21 @@ public function delete($path) {
    * {@inheritdoc}
    */
   public function deleteRecursive($path, ?callable $callback = NULL) {
+    // Ensure paths are local paths when a recursive delete is started.
+    if ($this->streamWrapperManager->isValidUri($path)) {
+      $path = $this->realpath($path);
+    }
+
     if ($callback) {
       call_user_func($callback, $path);
     }
 
-    if (!file_exists($path)) {
+    // Allow broken links to be removed.
+    if (!file_exists($path) && !is_link($path)) {
       return TRUE;
     }
 
-    if (is_dir($path)) {
+    if (is_dir($path) && !is_link($path)) {
       $dir = dir($path);
       while (($entry = $dir->read()) !== FALSE) {
         if ($entry == '.' || $entry == '..') {

core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php

@@ -74,4 +74,108 @@ public function testSubDirectory(): void {
     $this->assertDirectoryDoesNotExist($directory);
   }
 
+  /**
+   * Tests symlinks in directories do not result in unexpected deletions.
+   */
+  public function testSymlinksInDirectory(): void {
+    // Create files to link to.
+    mkdir($this->siteDirectory . '/dir1');
+    touch($this->siteDirectory . '/dir1/test1.txt');
+    touch($this->siteDirectory . '/test2.txt');
+
+    // Create directory to be deleted.
+    mkdir($this->siteDirectory . '/dir2');
+    // Symlink to a directory outside dir2.
+    symlink(realpath($this->siteDirectory . '/dir1'), $this->siteDirectory . '/dir2/subdir');
+    // Symlink to a file outside dir2.
+    symlink(realpath($this->siteDirectory . '/test2.txt'), $this->siteDirectory . '/dir2/test2.text');
+    $this->assertFileExists($this->siteDirectory . '/dir2/subdir/test1.txt');
+    $this->assertFileExists($this->siteDirectory . '/dir2/test2.text');
+
+    $this->container->get('file_system')->deleteRecursive($this->siteDirectory . '/dir2');
+    $this->assertFileExists($this->siteDirectory . '/dir1/test1.txt');
+    $this->assertFileExists($this->siteDirectory . '/test2.txt');
+    $this->assertDirectoryDoesNotExist($this->siteDirectory . '/dir2');
+  }
+
+  /**
+   * Tests symlinks in directories do not result in unexpected deletions.
+   */
+  public function testSymlinksInDirectoryViaStreamWrappers(): void {
+    // Create files to link to.
+    mkdir($this->siteDirectory . '/files/dir1');
+    touch($this->siteDirectory . '/files/dir1/test1.txt');
+    touch($this->siteDirectory . '/files/test2.txt');
+
+    // Create directory to be deleted.
+    mkdir($this->siteDirectory . '/files/dir2');
+    // Symlink to a directory outside dir2.
+    symlink(realpath($this->siteDirectory . '/files/dir1'), $this->siteDirectory . '/files/dir2/subdir');
+    // Symlink to a file outside dir2.
+    symlink(realpath($this->siteDirectory . '/files/test2.txt'), $this->siteDirectory . '/files/dir2/test2.text');
+    $this->assertFileExists($this->siteDirectory . '/files/dir2/subdir/test1.txt');
+    $this->assertFileExists($this->siteDirectory . '/files/dir2/test2.text');
+
+    // Use the stream wrapper to delete.
+    $this->container->get('file_system')->deleteRecursive('public://dir2');
+    $this->assertFileExists($this->siteDirectory . '/files/dir1/test1.txt');
+    $this->assertFileExists($this->siteDirectory . '/files/test2.txt');
+    $this->assertDirectoryDoesNotExist($this->siteDirectory . '/files/dir2');
+  }
+
+  /**
+   * Tests symlinks to directories do not result in unexpected deletions.
+   */
+  public function testSymlinksToDirectory(): void {
+    // Create files to link to.
+    mkdir($this->siteDirectory . '/dir1');
+    touch($this->siteDirectory . '/dir1/test1.txt');
+    // Symlink to a directory outside dir2.
+    symlink(realpath($this->siteDirectory . '/dir1'), $this->siteDirectory . '/dir2');
+    $this->assertFileExists($this->siteDirectory . '/dir2/test1.txt');
+
+    $this->container->get('file_system')->deleteRecursive($this->siteDirectory . '/dir2');
+    $this->assertFileExists($this->siteDirectory . '/dir1/test1.txt');
+    $this->assertDirectoryDoesNotExist($this->siteDirectory . '/dir2');
+  }
+
+  /**
+   * Tests trying to delete symlinks to directories via stream wrappers.
+   *
+   * Note that this tests unexpected behavior.
+   */
+  public function testSymlinksToDirectoryViaStreamWrapper(): void {
+    $file_system = $this->container->get('file_system');
+
+    // Create files to link to.
+    $file_system->mkdir('public://dir1');
+    file_put_contents('public://dir1/test1.txt', 'test');
+
+    // Symlink to a directory outside dir2.
+    $public_path = realpath($this->siteDirectory . '/files');
+    symlink($public_path . '/dir1', $public_path . '/dir2');
+    $this->assertFileExists($public_path . '/dir1/test1.txt');
+    $this->assertFileExists('public://dir2/test1.txt');
+
+    // The stream wrapper system resolves 'public://dir2' to 'files/dir1'.
+    // Therefore, this call results in removing dir1 and does not remove the
+    // dir2 symlink.
+    $this->container->get('file_system')->deleteRecursive('public://dir2');
+    $this->assertFileDoesNotExist($public_path . '/dir1/test1.txt');
+    // The directory is now a broken link.
+    $this->assertTrue(is_link($public_path . '/dir2'));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function tearDown(): void {
+    $this->assertDirectoryExists($this->siteDirectory);
+    parent::tearDown();
+
+    // Ensure \Drupal\KernelTests\KernelTestBase::tearDown() has cleaned up the
+    // file system.
+    $this->assertDirectoryDoesNotExist($this->siteDirectory);
+  }
+
 }

core/tests/Drupal/KernelTests/Core/File/FileDeleteTest.php

@@ -51,4 +51,37 @@ public function testDirectory(): void {
     $this->assertDirectoryExists($directory);
   }
 
+  /**
+   * Tests deleting a symlink to a directory.
+   */
+  public function testSymlinkDirectory(): void {
+    // A directory to operate on.
+    $directory = \Drupal::service('file_system')->realpath($this->createDirectory());
+    $link = dirname($directory) . '/' . $this->randomMachineName();
+    symlink($directory, $link);
+    $this->assertDirectoryExists($link);
+
+    \Drupal::service('file_system')->delete($link);
+    $this->assertDirectoryExists($directory);
+    $this->assertDirectoryDoesNotExist($link);
+  }
+
+  /**
+   * Tests deleting using a symlinked directory using stream wrappers.
+   *
+   * Note that this does not work because the path will be resolved to the real
+   * path in the stream wrapper and not the link.
+   */
+  public function testSymlinkDirectoryStreamWrappers(): void {
+    // A directory to operate on.
+    $directory = $this->createDirectory();
+    $link = 'public://' . $this->randomMachineName();
+    symlink(\Drupal::service('file_system')->realpath($directory), \Drupal::service('file_system')->realpath($link));
+    $this->assertDirectoryExists($link);
+
+    $this->expectExceptionMessage("Cannot delete '$link' because it is a directory. Use deleteRecursive() instead.");
+    $this->expectException(NotRegularFileException::class);
+    \Drupal::service('file_system')->delete($link);
+  }
+
 }

core/tests/Drupal/KernelTests/KernelTestBase.php

@@ -717,6 +717,15 @@ protected function tearDown(): void {
       }
     }
 
+    // If the test used the regular file system, remove any files created.
+    if (!str_starts_with($this->siteDirectory, 'vfs://')) {
+      // Delete test site directory.
+      $callback = function (string $path) {
+        @chmod($path, 0700);
+      };
+      \Drupal::service('file_system')->deleteRecursive($this->siteDirectory, $callback);
+    }
+
     // Free up memory: Own properties.
     $this->classLoader = NULL;
     $this->vfsRoot = NULL;