From ce594eece7801e7ba11d7b99701c1315cf70449b Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Sun, 3 Jul 2016 17:30:46 +0900 Subject: [PATCH] Throw user warnings when a file operation fails dangerously --- common/framework/storage.php | 89 +++++++++++++++++++++++++--- tests/unit/framework/StorageTest.php | 2 +- 2 files changed, 81 insertions(+), 10 deletions(-) diff --git a/common/framework/storage.php b/common/framework/storage.php index 07a804a36..e743152cf 100644 --- a/common/framework/storage.php +++ b/common/framework/storage.php @@ -168,12 +168,18 @@ class Storage { if ($stream) { - return @fopen($filename, 'r'); + $result = @fopen($filename, 'r'); } else { - return @file_get_contents($filename); + $result = @file_get_contents($filename); } + + if ($result === false) + { + trigger_error('Cannot read file: ' . $filename, \E_USER_WARNING); + } + return $result; } else { @@ -224,6 +230,7 @@ class Storage $mkdir_success = self::createDirectory($destination_dir); if (!$mkdir_success && !self::exists($destination_dir)) { + trigger_error('Cannot create directory to write file: ' . $filename, \E_USER_WARNING); return false; } } @@ -234,7 +241,7 @@ class Storage $filename = $filename . '.tmp.' . microtime(true); } - if ($fp = fopen($filename, $mode)) + if ($fp = @fopen($filename, $mode)) { flock($fp, \LOCK_EX); if (is_resource($content)) @@ -248,9 +255,16 @@ class Storage fflush($fp); flock($fp, \LOCK_UN); fclose($fp); + + if (!$result) + { + trigger_error('Cannot write file: ' . (isset($original_filename) ? $original_filename : $filename), \E_USER_WARNING); + return false; + } } else { + trigger_error('Cannot write file: ' . (isset($original_filename) ? $original_filename : $filename), \E_USER_WARNING); return false; } @@ -264,7 +278,7 @@ class Storage if (!$rename_success) { @unlink($filename); - throw new Exception('foo'); + trigger_error('Cannot write file: ' . (isset($original_filename) ? $original_filename : $filename), \E_USER_WARNING); return false; } } @@ -318,12 +332,14 @@ class Storage $destination = rtrim($destination, '/\\'); if (!self::exists($source)) { + trigger_error('Cannot copy because the source does not exist: ' . $source, \E_USER_WARNING); return false; } $destination_dir = dirname($destination); if (!self::exists($destination_dir) && !self::createDirectory($destination_dir)) { + trigger_error('Cannot create directory to copy into: ' . $destination_dir, \E_USER_WARNING); return false; } elseif (self::isDirectory($destination)) @@ -340,6 +356,7 @@ class Storage $copy_success = @copy($source, $destination); if (!$copy_success) { + trigger_error('Cannot copy ' . $source . ' to ' . (isset($original_destination) ? $original_destination : $destination), \E_USER_WARNING); return false; } @@ -353,7 +370,7 @@ class Storage if (!$rename_success) { @unlink($destination); - throw new Exception('foo'); + trigger_error('Cannot copy ' . $source . ' to ' . (isset($original_destination) ? $original_destination : $destination), \E_USER_WARNING); return false; } } @@ -395,12 +412,14 @@ class Storage $destination = rtrim($destination, '/\\'); if (!self::exists($source)) { + trigger_error('Cannot move because the source does not exist: ' . $source, \E_USER_WARNING); return false; } $destination_dir = dirname($destination); if (!self::exists($destination_dir) && !self::createDirectory($destination_dir)) { + trigger_error('Cannot create directory to move into: ' . $destination_dir, \E_USER_WARNING); return false; } elseif (self::isDirectory($destination)) @@ -409,13 +428,19 @@ class Storage } $result = @rename($source, $destination); + if (!$result) + { + trigger_error('Cannot move ' . $source . ' to ' . $destination, \E_USER_WARNING); + return false; + } + if (function_exists('opcache_invalidate') && substr($source, -4) === '.php') { @opcache_invalidate($source, true); } clearstatcache(true, $destination); - return $result; + return true; } /** @@ -430,7 +455,17 @@ class Storage public static function delete($filename) { $filename = rtrim($filename, '/\\'); - $result = @self::exists($filename) && @is_file($filename) && @unlink($filename); + if (!self::exists($filename)) + { + return false; + } + + $result = @is_file($filename) && @unlink($filename); + if (!$result) + { + trigger_error('Cannot delete file: ' . $filename, \E_USER_WARNING); + } + if (function_exists('opcache_invalidate') && substr($filename, -4) === '.php') { @opcache_invalidate($filename, true); @@ -451,7 +486,24 @@ class Storage { $mode = 0777 & ~umask(); } - return @mkdir($dirname, $mode, true); + + $result = @mkdir($dirname, $mode, true); + if (!$result) + { + if (!is_dir($dirname)) + { + trigger_error('Cannot create directory: ' . $dirname, \E_USER_WARNING); + } + else + { + @chmod($dirname, $mode); + } + return false; + } + else + { + return true; + } } /** @@ -477,6 +529,7 @@ class Storage } catch (\UnexpectedValueException $e) { + trigger_error('Cannot read directory: ' . $dirname, \E_USER_WARNING); return false; } @@ -510,10 +563,12 @@ class Storage $destination = rtrim($destination, '/\\'); if (!self::isDirectory($source)) { + trigger_error('Cannot copy because the source does not exist: ' . $source, \E_USER_WARNING); return false; } if (!self::isDirectory($destination) && !self::createDirectory($destination)) { + trigger_error('Cannot create directory to copy into: ' . $destination, \E_USER_WARNING); return false; } @@ -577,8 +632,13 @@ class Storage public static function deleteDirectory($dirname, $delete_self = true) { $dirname = rtrim($dirname, '/\\'); + if (!self::exists($dirname)) + { + return false; + } if (!self::isDirectory($dirname)) { + trigger_error('Delete target is not a directory: ' . $dirname, \E_USER_WARNING); return false; } @@ -592,6 +652,7 @@ class Storage { if (!@rmdir($path->getPathname())) { + trigger_error('Cannot delete directory: ' . $path->getPathname(), \E_USER_WARNING); return false; } } @@ -599,6 +660,7 @@ class Storage { if (!@unlink($path->getPathname())) { + trigger_error('Cannot delete file: ' . $path->getPathname(), \E_USER_WARNING); return false; } } @@ -606,7 +668,16 @@ class Storage if ($delete_self) { - return @rmdir($dirname); + $result = @rmdir($dirname); + if (!$result) + { + trigger_error('Cannot delete directory: ' . $dirname, \E_USER_WARNING); + return false; + } + else + { + return true; + } } else { diff --git a/tests/unit/framework/StorageTest.php b/tests/unit/framework/StorageTest.php index 83bbe8429..9311a0657 100644 --- a/tests/unit/framework/StorageTest.php +++ b/tests/unit/framework/StorageTest.php @@ -253,7 +253,7 @@ class StorageTest extends \Codeception\TestCase\Test $this->assertTrue(Rhymix\Framework\Storage::copyDirectory($sourcedir, $targetdir)); $this->assertTrue(file_exists($targetdir . '/bar')); $this->assertTrue(file_exists($targetdir . '/subdir/baz')); - $this->assertFalse(Rhymix\Framework\Storage::copyDirectory($sourcedir, '/opt/nonexistent.foobar')); + $this->assertFalse(@Rhymix\Framework\Storage::copyDirectory($sourcedir, '/opt/nonexistent.foobar')); } public function testMoveDirectory()