8000 bug #38652 [Filesystem] Check if failed unlink was caused by permissi… · symfony/symfony@3ff9384 · GitHub
[go: up one dir, main page]
Skip to content

Commit 3ff9384

Browse files
fabpotfabpot
committed
bug #38652 [Filesystem] Check if failed unlink was caused by permission denied (Nyholm)
This PR was submitted for the 4.4 branch but it was squashed and merged into the 3.4 branch instead. Discussion ---------- [Filesystem] Check if failed unlink was caused by permission denied | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38650 | License | MIT | Doc PR | Check why we failed to unlink the file. If the file was missing, then no exception should be thrown. However, if we failed to unlink it because of permissions, we should throw an exception. Commits ------- 1cde6ca [Filesystem] Check if failed unlink was caused by permission denied
2 parents 3ed5ec0 + 1cde6ca commit 3ff9384

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

‎src/Symfony/Component/Filesystem/Filesystem.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ public function remove($files)
179179
if (!self::box('rmdir', $file) && file_exists($file)) {
180180
throw new IOException(sprintf('Failed to remove directory "%s": ', $file).self::$lastError);
181181
}
182-
} elseif (!self::box('unlink', $file) && file_exists($file)) {
182+
} elseif (!self::box('unlink', $file) && (false !== strpos(self::$lastError, 'Permission denied') || file_exists($file))) {
183183
throw new IOException(sprintf('Failed to remove file "%s": ', $file).self::$lastError);
184184
}
185185
}

‎src/Symfony/Component/Filesystem/Tests/FilesystemTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Filesystem\Tests;
1313

14+
use Symfony\Component\Filesystem\Exception\IOException;
15+
1416
/**
1517
* Test class for Filesystem.
1618
*/
@@ -334,6 +336,28 @@ public function testRemoveIgnoresNonExistingFiles()
334336
$this->assertFileDoesNotExist($basePath.'dir');
335337
}
336338

339+
public function testRemoveThrowsExceptionOnPermissionDenied()
340+
{
341+
$this->markAsSkippedIfChmodIsMissing();
342+
343+
$basePath = $this->workspace.\DIRECTORY_SEPARATOR.'dir_permissions';
344+
mkdir($basePath);
345+
$file = $basePath.\DIRECTORY_SEPARATOR.'file';
346+
touch($file);
347+
chmod($basePath, 0400);
348+
349+
try {
350+
$this->filesystem->remove($file);
351+
$this->fail('Filesystem::remove() should throw an exception');
352+
} catch (IOException $e) {
353+
$this->assertStringContainsString('Failed to remove file "'.$file.'"', $e->getMessage());
354+
$this->assertStringContainsString('Permission denied', $e->getMessage());
355+
} finally {
356+
// Make sure we can clean up this file
357+
chmod($basePath, 0777);
358+
}
359+
}
360+
337361
public function testRemoveCleansInvalidLinks()
338362
{
339363
$this->markAsSkippedIfSymlinkIsMissing();

0 commit comments

Comments
 (0)
0