Skip to content

Commit

Permalink
bug #18054 [Filesystem] Fix false positive in ->remove() (nicolas-gre…
Browse files Browse the repository at this point in the history 
…kas)

This PR was merged into the 2.3 branch.

Discussion
----------

[Filesystem] Fix false positive in ->remove()

| Q             | A
| ------------- | ---
| Branch        | 2.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

3efd900 [Filesystem] Fix false positive in ->remove()
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Mar 8, 2016
2 parents 35666f0 + 3efd900 commit 1bbcff0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 21 deletions.
23 changes: 11 additions & 12 deletions src/Symfony/Component/Filesystem/Filesystem.php
View file
Expand Up @@ -143,24 +143,23 @@ public function remove($files)
$files = iterator_to_array($this->toIterator($files));
$files = array_reverse($files);
foreach ($files as $file) {
if (@(unlink($file) || rmdir($file))) {
continue;
}
if (is_link($file)) {
// Workaround https://bugs.php.net/52176
if (!@unlink($file) && !@rmdir($file)) {
$error = error_get_last();
throw new IOException(sprintf('Failed to remove symlink "%s": %s.', $file, $error['message']));
}
// See https://bugs.php.net/52176
$error = error_get_last();
throw new IOException(sprintf('Failed to remove symlink "%s": %s.', $file, $error['message']));
} elseif (is_dir($file)) {
$this->remove(new \FilesystemIterator($file));

if (!@rmdir($file)) {
$error = error_get_last();
throw new IOException(sprintf('Failed to remove directory "%s": %s.', $file, $error['message']));
}
} elseif ($this->exists($file)) {
if (!@unlink($file)) {
$error = error_get_last();
throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message']));
}
} elseif (file_exists($file)) {
$error = error_get_last();
throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message']));
}
}
}
Expand Down Expand Up @@ -403,7 +402,7 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o
}

$copyOnWindows = false;
if (isset($options['copy_on_windows']) && !function_exists('symlink')) {
if (isset($options['copy_on_windows'])) {
$copyOnWindows = $options['copy_on_windows'];
}

Expand All @@ -420,7 +419,7 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o
$target = str_replace($originDir, $targetDir, $file->getPathname());

if ($copyOnWindows) {
if (is_link($file) || is_file($file)) {
if (is_file($file)) {
$this->copy($file, $target, isset($options['override']) ? $options['override'] : false);
} elseif (is_dir($file)) {
$this->mkdir($target);
Expand Down
19 changes: 10 additions & 9 deletions src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
View file
Expand Up @@ -38,10 +38,8 @@ public static function setUpBeforeClass()
if ('\\' === DIRECTORY_SEPARATOR && null === self::$symlinkOnWindows) {
$target = tempnam(sys_get_temp_dir(), 'sl');
$link = sys_get_temp_dir().'/sl'.microtime(true).mt_rand();
if (@symlink($target, $link)) {
self::$symlinkOnWindows = @is_link($link);
unlink($link);
}
self::$symlinkOnWindows = @symlink($target, $link) && is_link($link);
@unlink($link);
unlink($target);
}
}
Expand All @@ -61,6 +59,7 @@ protected function tearDown()
foreach ($this->longPathNamesWindows as $path) {
exec('DEL '.$path);
}
$this->longPathNamesWindows = array();
}

$this->filesystem->remove($this->workspace);
Expand Down Expand Up @@ -350,7 +349,7 @@ public function testRemoveCleansInvalidLinks()

// create symlink to nonexistent dir
rmdir($basePath.'dir');
$this->assertFalse(is_dir($basePath.'dir-link'));
$this->assertFalse('\\' === DIRECTORY_SEPARATOR ? @readlink($basePath.'dir-link') : is_dir($basePath.'dir-link'));

$this->filesystem->remove($basePath);

Expand Down Expand Up @@ -742,6 +741,8 @@ public function testRemoveSymlink()
$this->filesystem->remove($link);

$this->assertTrue(!is_link($link));
$this->assertTrue(!is_file($link));
$this->assertTrue(!is_dir($link));
}

public function testSymlinkIsOverwrittenIfPointsToDifferentTarget()
Expand Down Expand Up @@ -915,7 +916,7 @@ public function testMirrorCopiesLinks()
$this->filesystem->mirror($sourcePath, $targetPath);

$this->assertTrue(is_dir($targetPath));
$this->assertFileEquals($sourcePath.'file1', $targetPath.DIRECTORY_SEPARATOR.'link1');
$this->assertFileEquals($sourcePath.'file1', $targetPath.'link1');
$this->assertTrue(is_link($targetPath.DIRECTORY_SEPARATOR.'link1'));
}

Expand All @@ -935,7 +936,7 @@ public function testMirrorCopiesLinkedDirectoryContents()
$this->filesystem->mirror($sourcePath, $targetPath);

$this->assertTrue(is_dir($targetPath));
$this->assertFileEquals($sourcePath.'/nested/file1.txt', $targetPath.DIRECTORY_SEPARATOR.'link1/file1.txt');
$this->assertFileEquals($sourcePath.'/nested/file1.txt', $targetPath.'link1/file1.txt');
$this->assertTrue(is_link($targetPath.DIRECTORY_SEPARATOR.'link1'));
}

Expand All @@ -959,7 +960,7 @@ public function testMirrorCopiesRelativeLinkedContents()
$this->filesystem->mirror($sourcePath, $targetPath);

$this->assertTrue(is_dir($targetPath));
$this->assertFileEquals($sourcePath.'/nested/file1.txt', $targetPath.DIRECTORY_SEPARATOR.'link1/file1.txt');
$this->assertFileEquals($sourcePath.'/nested/file1.txt', $targetPath.'link1/file1.txt');
$this->assertTrue(is_link($targetPath.DIRECTORY_SEPARATOR.'link1'));
$this->assertEquals('nested', readlink($targetPath.DIRECTORY_SEPARATOR.'link1'));
}
Expand Down Expand Up @@ -1089,7 +1090,7 @@ private function markAsSkippedIfChmodIsMissing()

private function markAsSkippedIfPosixIsMissing()
{
if ('\\' === DIRECTORY_SEPARATOR || !function_exists('posix_isatty')) {
if (!function_exists('posix_isatty')) {
$this->markTestSkipped('POSIX is not supported');
}
}
Expand Down

0 comments on commit 1bbcff0

Please sign in to comment.