diff --git a/core/lib/Drupal/Core/File/FileSystem.php b/core/lib/Drupal/Core/File/FileSystem.php index be1d70ad6e2b1a5d9b6ce80bb05ebf79b0418ac9..06d67bf38090ae19ef4e1301c86cd3df7b47bef0 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 03f01e30fa10a9a8cbc3db9bdf8c1edbef01f4d4..8fa28491f6a4784233c4b4d526b7610831663a2c 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 1b6cbd1df171fddd04488fcbb64efb67b5000f3f..80e9de42bc30d9aed8ae92c9aa8ff4e37bcf07cd 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 22b7bb0d7d03b19ae42d9ac989d7a9841af78978..af77f47a1324350530414751b048b224040e1f6c 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;