diff --git a/lib/private/Files/Storage/Local.php b/lib/private/Files/Storage/Local.php index a4f0d1cc9bf91..8b69d650ba827 100644 --- a/lib/private/Files/Storage/Local.php +++ b/lib/private/Files/Storage/Local.php @@ -99,36 +99,51 @@ public function rmdir(string $path): bool { if (!$this->isDeletable($path)) { return false; } + + $sourcePath = $this->getSourcePath($path); + clearstatcache(true, $sourcePath); + + if (is_link($sourcePath)) { + return unlink($sourcePath); + } + try { $it = new \RecursiveIteratorIterator( - new \RecursiveDirectoryIterator($this->getSourcePath($path)), + new \RecursiveDirectoryIterator($sourcePath), \RecursiveIteratorIterator::CHILD_FIRST ); + /** - * RecursiveDirectoryIterator on an NFS path isn't iterable with foreach - * This bug is fixed in PHP 5.5.9 or before - * See #8376 + * RecursiveDirectoryIterator on an NFS path isn't iterable with foreach. + * This bug is fixed in PHP 5.5.9 or before. + * See #8376. */ $it->rewind(); while ($it->valid()) { - /** - * @var \SplFileInfo $file - */ + /** @var \SplFileInfo $file */ $file = $it->current(); clearstatcache(true, $file->getRealPath()); - if (in_array($file->getBasename(), ['.', '..'])) { + + if (in_array($file->getBasename(), ['.', '..'], true)) { $it->next(); continue; - } elseif ($file->isFile() || $file->isLink()) { + } + + if ($file->isFile() || $file->isLink()) { unlink($file->getPathname()); } elseif ($file->isDir()) { rmdir($file->getPathname()); } + $it->next(); } - unset($it); // Release iterator and thereby its potential directory lock (e.g. in case of VirtualBox shared folders) - clearstatcache(true, $this->getSourcePath($path)); - return rmdir($this->getSourcePath($path)); + + // Release the iterator and its potential directory lock, + // for example on VirtualBox shared folders. + unset($it); + + clearstatcache(true, $sourcePath); + return rmdir($sourcePath); } catch (\UnexpectedValueException $e) { return false; } diff --git a/tests/lib/Files/Storage/LocalTest.php b/tests/lib/Files/Storage/LocalTest.php index e676e6718ac31..6aa44e894b2c1 100644 --- a/tests/lib/Files/Storage/LocalTest.php +++ b/tests/lib/Files/Storage/LocalTest.php @@ -102,6 +102,50 @@ public function testDisallowSymlinksInsideDatadir(): void { $this->addToAssertionCount(1); } + public static function directorySymlinkRemovalProvider(): array { + return [ + 'rmdir' => ['rmdir'], + 'unlink' => ['unlink'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('directorySymlinkRemovalProvider')] + public function testRemovingDirectorySymlinkPreservesTarget(string $operation): void { + $targetPath = $this->tmpDir . 'target'; + $linkPath = $this->tmpDir . 'link'; + + $this->assertTrue($this->instance->mkdir('target')); + $this->assertSame( + strlen('target contents'), + $this->instance->file_put_contents('target/file.txt', 'target contents'), + ); + + if (!@symlink($targetPath, $linkPath)) { + $this->markTestSkipped('Failed to create directory symlink'); + } + + $this->assertTrue(is_link($linkPath)); + $this->assertTrue($this->instance->$operation('link')); + + $this->assertFalse( + is_link($linkPath), + 'The symbolic link should be removed', + ); + $this->assertDirectoryExists( + $targetPath, + 'The target directory should be preserved', + ); + $this->assertFileExists( + $targetPath . '/file.txt', + 'Files inside the target directory should be preserved', + ); + $this->assertSame( + 'target contents', + file_get_contents($targetPath . '/file.txt'), + 'The target file contents should remain unchanged', + ); + } + public function testWriteUmaskFilePutContents(): void { $oldMask = umask(0333); $this->instance->file_put_contents('test.txt', 'sad');