From addee909062ec3eb5b587d30f134aaf8ff60da3f Mon Sep 17 00:00:00 2001
From: catch <6915-catch@users.noreply.drupalcode.org>
Date: Mon, 17 Feb 2025 17:02:23 +0000
Subject: [PATCH] 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 ed78b812e179b3bc52371098cb033156ec577903)
---
core/lib/Drupal/Core/File/FileSystem.php | 17 ++-
.../Core/File/FileDeleteRecursiveTest.php | 104 ++++++++++++++++++
.../KernelTests/Core/File/FileDeleteTest.php | 33 ++++++
.../Drupal/KernelTests/KernelTestBase.php | 9 ++
4 files changed, 161 insertions(+), 2 deletions(-)
diff --git a/core/lib/Drupal/Core/File/FileSystem.php b/core/lib/Drupal/Core/File/FileSystem.php
index be1d70ad6e2b..06d67bf38090 100644
--- a/core/lib/Drupal/Core/File/FileSystem.php
+++ b/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 == '..') {
diff --git a/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php b/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php
index 03f01e30fa10..8fa28491f6a4 100644
--- a/core/tests/Drupal/KernelTests/Core/File/FileDeleteRecursiveTest.php
+++ b/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);
+ }
+
}
diff --git a/core/tests/Drupal/KernelTests/Core/File/FileDeleteTest.php b/core/tests/Drupal/KernelTests/Core/File/FileDeleteTest.php
index 1b6cbd1df171..80e9de42bc30 100644
--- a/core/tests/Drupal/KernelTests/Core/File/FileDeleteTest.php
+++ b/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);
+ }
+
}
diff --git a/core/tests/Drupal/KernelTests/KernelTestBase.php b/core/tests/Drupal/KernelTests/KernelTestBase.php
index 22b7bb0d7d03..af77f47a1324 100644
--- a/core/tests/Drupal/KernelTests/KernelTestBase.php
+++ b/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;
--
GitLab