From 55547c56e437d4c72cfbc55a4a0126cf9763a491 Mon Sep 17 00:00:00 2001
From: Kijin Sung <kijin@kijinsung.com>
Date: Mon, 11 Jul 2016 11:14:00 +0900
Subject: [PATCH 1/2] Do not use safe_overwrite if target directory is not
 writable

---
 common/framework/storage.php | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/common/framework/storage.php b/common/framework/storage.php
index 969d880d9..88367e8bc 100644
--- a/common/framework/storage.php
+++ b/common/framework/storage.php
@@ -240,11 +240,16 @@ class Storage
 			}
 		}
 		
-		if (self::$safe_overwrite && strncasecmp($mode, 'a', 1))
+		if (self::$safe_overwrite && strncasecmp($mode, 'a', 1) && @is_writable($destination_dir))
 		{
+			$use_atomic_rename = true;
 			$original_filename = $filename;
 			$filename = $filename . '.tmp.' . microtime(true);
 		}
+		else
+		{
+			$use_atomic_rename = false;
+		}
 		
 		if ($fp = @fopen($filename, $mode))
 		{
@@ -275,7 +280,7 @@ class Storage
 		
 		@chmod($filename, ($perms === null ? (0666 & ~self::getUmask()) : $perms));
 		
-		if (self::$safe_overwrite && strncasecmp($mode, 'a', 1))
+		if ($use_atomic_rename)
 		{
 			$rename_success = @rename($filename, $original_filename);
 			if (!$rename_success)
@@ -350,14 +355,20 @@ class Storage
 		}
 		elseif (self::isDirectory($destination))
 		{
+			$destination_dir = $destination;
 			$destination = $destination . '/' . basename($source);
 		}
 		
-		if (self::$safe_overwrite)
+		if (self::$safe_overwrite && @is_writable($destination_dir))
 		{
+			$use_atomic_rename = true;
 			$original_destination = $destination;
 			$destination = $destination . '.tmp.' . microtime(true);
 		}
+		else
+		{
+			$use_atomic_rename = false;
+		}
 		
 		$copy_success = @copy($source, $destination);
 		if (!$copy_success)
@@ -382,7 +393,7 @@ class Storage
 			@chmod($destination, $destination_perms);
 		}
 		
-		if (self::$safe_overwrite)
+		if ($use_atomic_rename)
 		{
 			$rename_success = @rename($destination, $original_destination);
 			if (!$rename_success)

From 59b33e8f29cc58e26ae26ebee977cf7be2be2e6e Mon Sep 17 00:00:00 2001
From: Kijin Sung <kijin@kijinsung.com>
Date: Mon, 11 Jul 2016 11:19:04 +0900
Subject: [PATCH 2/2] Add more tests for Storage::copy()

---
 tests/unit/framework/StorageTest.php | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/unit/framework/StorageTest.php b/tests/unit/framework/StorageTest.php
index f12b797a1..d7dd46ddb 100644
--- a/tests/unit/framework/StorageTest.php
+++ b/tests/unit/framework/StorageTest.php
@@ -9,11 +9,13 @@ class StorageTest extends \Codeception\TestCase\Test
 	
 	public function _after()
 	{
+		@chmod(\RX_BASEDIR . 'tests/_output', 0755);
 		Rhymix\Framework\Storage::deleteDirectory(\RX_BASEDIR . 'tests/_output', false);
 	}
 	
 	public function _failed()
 	{
+		@chmod(\RX_BASEDIR . 'tests/_output', 0755);
 		Rhymix\Framework\Storage::deleteDirectory(\RX_BASEDIR . 'tests/_output', false);
 	}
 	
@@ -191,13 +193,28 @@ class StorageTest extends \Codeception\TestCase\Test
 	{
 		$source = \RX_BASEDIR . 'tests/_output/copy.source.txt';
 		$target = \RX_BASEDIR . 'tests/_output/copy.target.txt';
+		$target_dir = \RX_BASEDIR . 'tests/_output';
 		file_put_contents($source, 'foobarbaz');
 		chmod($source, 0646);
 		
+		// Copy with exact destination filename
 		$this->assertTrue(Rhymix\Framework\Storage::copy($source, $target));
 		$this->assertTrue(file_exists($target));
 		$this->assertTrue(file_get_contents($target) === 'foobarbaz');
 		
+		// Copy into directory with source filename
+		$this->assertTrue(Rhymix\Framework\Storage::copy($source, $target_dir));
+		$this->assertTrue(file_exists($target_dir . '/copy.source.txt'));
+		$this->assertTrue(file_get_contents($target_dir . '/copy.source.txt') === 'foobarbaz');
+		
+		// Copy into directory with no write permissions
+		chmod($target_dir, 0555);
+		file_put_contents($source, 'foobarbaz has changed');
+		$this->assertTrue(Rhymix\Framework\Storage::copy($source, $target));
+		$this->assertTrue(file_exists($target));
+		$this->assertTrue(file_get_contents($target) === 'foobarbaz has changed');
+		chmod($target_dir, 0755);
+		
 		if (strncasecmp(\PHP_OS, 'Win', 3) !== 0)
 		{
 			$this->assertEquals(0646, fileperms($target) & 0777);