Skip to content

[Filesystem] improve messages on failure #40143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 33 additions & 45 deletions src/Symfony/Component/Filesystem/Filesystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ public function copy(string $originFile, string $targetFile, bool $overwriteNewe

if ($doCopy) {
// https://bugs.php.net/64634
if (false === $source = @fopen($originFile, 'r')) {
throw new IOException(sprintf('Failed to copy "%s" to "%s" because source file could not be opened for reading.', $originFile, $targetFile), 0, null, $originFile);
if (!$source = self::box('fopen', $originFile, 'r')) {
throw new IOException(sprintf('Failed to copy "%s" to "%s" because source file could not be opened for reading: ', $originFile, $targetFile).self::$lastError, 0, null, $originFile);
}

// Stream context created to allow files overwrite when using FTP stream wrapper - disabled by default
if (false === $target = @fopen($targetFile, 'w', null, stream_context_create(['ftp' => ['overwrite' => true]]))) {
throw new IOException(sprintf('Failed to copy "%s" to "%s" because target file could not be opened for writing.', $originFile, $targetFile), 0, null, $originFile);
if (!$target = self::box('fopen', $targetFile, 'w', null, stream_context_create(['ftp' => ['overwrite' => true]]))) {
throw new IOException(sprintf('Failed to copy "%s" to "%s" because target file could not be opened for writing: ', $originFile, $targetFile).self::$lastError, 0, null, $originFile);
}

$bytesCopied = stream_copy_to_stream($source, $target);
Expand All @@ -70,7 +70,7 @@ public function copy(string $originFile, string $targetFile, bool $overwriteNewe

if ($originIsLocal) {
// Like `cp`, preserve executable permission bits
@chmod($targetFile, fileperms($targetFile) | (fileperms($originFile) & 0111));
self::box('chmod', $targetFile, fileperms($targetFile) | (fileperms($originFile) & 0111));

if ($bytesCopied !== $bytesOrigin = filesize($originFile)) {
throw new IOException(sprintf('Failed to copy the whole content of "%s" to "%s" (%g of %g bytes copied).', $originFile, $targetFile, $bytesCopied, $bytesOrigin), 0, null, $originFile);
Expand All @@ -93,14 +93,8 @@ public function mkdir($dirs, int $mode = 0777)
continue;
}

if (!self::box('mkdir', $dir, $mode, true)) {
if (!is_dir($dir)) {
// The directory was not created by a concurrent process. Let's throw an exception with a developer friendly error message if we have one
if (self::$lastError) {
throw new IOException(sprintf('Failed to create "%s": ', $dir).self::$lastError, 0, null, $dir);
}
throw new IOException(sprintf('Failed to create "%s".', $dir), 0, null, $dir);
}
if (!self::box('mkdir', $dir, $mode, true) && !is_dir($dir)) {
throw new IOException(sprintf('Failed to create "%s": ', $dir).self::$lastError, 0, null, $dir);
}
}
}
Expand Down Expand Up @@ -141,9 +135,8 @@ public function exists($files)
public function touch($files, int $time = null, int $atime = null)
{
foreach ($this->toIterable($files) as $file) {
$touch = $time ? @touch($file, $time, $atime) : @touch($file);
if (true !== $touch) {
throw new IOException(sprintf('Failed to touch "%s".', $file), 0, null, $file);
if (!($time ? self::box('touch', $file, $time, $atime) : self::box('touch', $file))) {
throw new IOException(sprintf('Failed to touch "%s": ', $file).self::$lastError, 0, null, $file);
}
}
}
Expand Down Expand Up @@ -194,8 +187,8 @@ public function remove($files)
public function chmod($files, int $mode, int $umask = 0000, bool $recursive = false)
{
foreach ($this->toIterable($files) as $file) {
if ((\PHP_VERSION_ID < 80000 || \is_int($mode)) && true !== @chmod($file, $mode & ~$umask)) {
throw new IOException(sprintf('Failed to chmod file "%s".', $file), 0, null, $file);
if ((\PHP_VERSION_ID < 80000 || \is_int($mode)) && !self::box('chmod', $file, $mode & ~$umask)) {
throw new IOException(sprintf('Failed to chmod file "%s": ', $file).self::$lastError, 0, null, $file);
}
if ($recursive && is_dir($file) && !is_link($file)) {
$this->chmod(new \FilesystemIterator($file), $mode, $umask, true);
Expand All @@ -219,12 +212,12 @@ public function chown($files, $user, bool $recursive = false)
$this->chown(new \FilesystemIterator($file), $user, true);
}
if (is_link($file) && \function_exists('lchown')) {
if (true !== @lchown($file, $user)) {
throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file);
if (!self::box('lchown', $file, $user)) {
throw new IOException(sprintf('Failed to chown file "%s": ', $file).self::$lastError, 0, null, $file);
}
} else {
if (true !== @chown($file, $user)) {
throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file);
if (!self::box('chown', $file, $user)) {
throw new IOException(sprintf('Failed to chown file "%s": ', $file).self::$lastError, 0, null, $file);
}
}
}
Expand All @@ -246,12 +239,12 @@ public function chgrp($files, $group, bool $recursive = false)
$this->chgrp(new \FilesystemIterator($file), $group, true);
}
if (is_link($file) && \function_exists('lchgrp')) {
if (true !== @lchgrp($file, $group)) {
throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file);
if (!self::box('lchgrp', $file, $group)) {
throw new IOException(sprintf('Failed to chgrp file "%s": ', $file).self::$lastError, 0, null, $file);
}
} else {
if (true !== @chgrp($file, $group)) {
throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file);
if (!self::box('chgrp', $file, $group)) {
throw new IOException(sprintf('Failed to chgrp file "%s": ', $file).self::$lastError, 0, null, $file);
}
}
}
Expand All @@ -270,15 +263,15 @@ public function rename(string $origin, string $target, bool $overwrite = false)
throw new IOException(sprintf('Cannot rename because the target "%s" already exists.', $target), 0, null, $target);
}

if (true !== @rename($origin, $target)) {
if (!self::box('rename', $origin, $target)) {
if (is_dir($origin)) {
// See https://bugs.php.net/54097 & https://php.net/rename#113943
$this->mirror($origin, $target, null, ['override' => $overwrite, 'delete' => $overwrite]);
$this->remove($origin);

return;
}
throw new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target), 0, null, $target);
throw new IOException(sprintf('Cannot rename "%s" to "%s": ', $origin, $target).self::$lastError, 0, null, $target);
}
}

Expand Down Expand Up @@ -372,7 +365,7 @@ private function linkException(string $origin, string $target, string $linkType)
throw new IOException(sprintf('Unable to create "%s" link due to error code 1314: \'A required privilege is not held by the client\'. Do you have the required Administrator-rights?', $linkType), 0, null, $target);
}
}
throw new IOException(sprintf('Failed to create "%s" link from "%s" to "%s".', $linkType, $origin, $target), 0, null, $target);
throw new IOException(sprintf('Failed to create "%s" link from "%s" to "%s": ', $linkType, $origin, $target).self::$lastError, 0, null, $target);
}

/**
Expand Down Expand Up @@ -594,18 +587,16 @@ public function tempnam(string $dir, string $prefix/*, string $suffix = ''*/)

// If no scheme or scheme is "file" or "gs" (Google Cloud) create temp file in local filesystem
if ((null === $scheme || 'file' === $scheme || 'gs' === $scheme) && '' === $suffix) {
$tmpFile = @tempnam($hierarchy, $prefix);

// If tempnam failed or no scheme return the filename otherwise prepend the scheme
if (false !== $tmpFile) {
if ($tmpFile = self::box('tempnam', $hierarchy, $prefix)) {
if (null !== $scheme && 'gs' !== $scheme) {
return $scheme.'://'.$tmpFile;
}

return $tmpFile;
}

throw new IOException('A temporary file could not be created.');
throw new IOException('A temporary file could not be created: '.self::$lastError);
}

// Loop until we create a valid temp file or have reached 10 attempts
Expand All @@ -615,20 +606,17 @@ public function tempnam(string $dir, string $prefix/*, string $suffix = ''*/)

// Use fopen instead of file_exists as some streams do not support stat
// Use mode 'x+' to atomically check existence and create to avoid a TOCTOU vulnerability
$handle = @fopen($tmpFile, 'x+');

// If unsuccessful restart the loop
if (false === $handle) {
if (!$handle = self::box('fopen', $tmpFile, 'x+')) {
continue;
}

// Close the file if it was successfully opened
@fclose($handle);
self::box('fclose', $handle);

return $tmpFile;
}

throw new IOException('A temporary file could not be created.');
throw new IOException('A temporary file could not be created: '.self::$lastError);
}

/**
Expand Down Expand Up @@ -659,16 +647,16 @@ public function dumpFile(string $filename, $content)
$tmpFile = $this->tempnam($dir, basename($filename));

try {
if (false === @file_put_contents($tmpFile, $content)) {
throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename);
if (false === self::box('file_put_contents', $tmpFile, $content)) {
throw new IOException(sprintf('Failed to write file "%s": ', $filename).self::$lastError, 0, null, $filename);
}

@chmod($tmpFile, file_exists($filename) ? fileperms($filename) : 0666 & ~umask());
self::box('chmod', $tmpFile, file_exists($filename) ? fileperms($filename) : 0666 & ~umask());

$this->rename($tmpFile, $filename, true);
} finally {
if (file_exists($tmpFile)) {
@unlink($tmpFile);
self::box('unlink', $tmpFile);
}
}
}
Expand Down Expand Up @@ -696,8 +684,8 @@ public function appendToFile(string $filename, $content)
throw new IOException(sprintf('Unable to write to the "%s" directory.', $dir), 0, null, $dir);
}

if (false === @file_put_contents($filename, $content, \FILE_APPEND)) {
throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename);
if (false === self::box('file_put_contents', $filename, $content, \FILE_APPEND)) {
throw new IOException(sprintf('Failed to write file "%s": ', $filename).self::$lastError, 0, null, $filename);
}
}

Expand Down