Delete target file for unsuccessful copy/rename

2 files changed, 88 insertions, 7 deletions

diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 3bc9fdff1ee..d877d8dafaa 100644
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -451,8 +451,11 @@ class View {
 	}
 
 	/**
-	 * @param string $path1
-	 * @param string $path2
+	 * Rename/move a file or folder from the source path to target path.
+	 *
+	 * @param string $path1 source path
+	 * @param string $path2 target path
+	 *
 	 * @return bool|mixed
 	 */
 	public function rename($path1, $path2) {
@@ -494,7 +497,7 @@ class View {
 				$mount = $manager->find($absolutePath1 . $postFix1);
 				$storage1 = $mount->getStorage();
 				$internalPath1 = $mount->getInternalPath($absolutePath1 . $postFix1);
-				list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
+				list($storage2, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
 				if ($internalPath1 === '' and $mount instanceof MoveableMount) {
 					if ($this->isTargetAllowed($absolutePath2)) {
 						/**
@@ -523,8 +526,10 @@ class View {
 					} else {
 						$source = $this->fopen($path1 . $postFix1, 'r');
 						$target = $this->fopen($path2 . $postFix2, 'w');
-						list($count, $result) = \OC_Helper::streamCopy($source, $target);
-						$this->touch($path2, $this->filemtime($path1));
+						list(, $result) = \OC_Helper::streamCopy($source, $target);
+						if ($result !== false) {
+							$this->touch($path2, $this->filemtime($path1));
+						}
 
 						// close open handle - especially $source is necessary because unlink below will
 						// throw an exception on windows because the file is locked
@@ -533,6 +538,11 @@ class View {
 
 						if ($result !== false) {
 							$result &= $storage1->unlink($internalPath1);
+						} else {
+							// delete partially written target file
+							$storage2->unlink($internalPath2);
+							// delete cache entry that was created by fopen
+							$storage2->getCache()->remove($internalPath2);
 						}
 					}
 				}
@@ -564,6 +574,15 @@ class View {
 		}
 	}
 
+	/**
+	 * Copy a file/folder from the source path to target path
+	 *
+	 * @param string $path1 source path
+	 * @param string $path2 target path
+	 * @param bool $preserveMtime whether to preserve mtime on the copy
+	 *
+	 * @return bool|mixed
+	 */
 	public function copy($path1, $path2, $preserveMtime = false) {
 		$postFix1 = (substr($path1, -1, 1) === '/') ? '/' : '';
 		$postFix2 = (substr($path2, -1, 1) === '/') ? '/' : '';
@@ -603,6 +622,11 @@ class View {
 					list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
 					if ($storage) {
 						$result = $storage->copy($internalPath1, $internalPath2);
+						if (!$result) {
+							// delete partially written target file
+							$storage->unlink($internalPath2);
+							$storage->getCache()->remove($internalPath2);
+						}
 					} else {
 						$result = false;
 					}
@@ -620,14 +644,20 @@ class View {
 							}
 						}
 					} else {
+						list($storage2, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
 						$source = $this->fopen($path1 . $postFix1, 'r');
 						$target = $this->fopen($path2 . $postFix2, 'w');
-						list($count, $result) = \OC_Helper::streamCopy($source, $target);
-						if($preserveMtime) {
+						list(, $result) = \OC_Helper::streamCopy($source, $target);
+						if($result && $preserveMtime) {
 							$this->touch($path2, $this->filemtime($path1));
 						}
 						fclose($source);
 						fclose($target);
+						if (!$result) {
+							// delete partially written target file
+							$storage2->unlink($internalPath2);
+							$storage2->getCache()->remove($internalPath2);
+						}
 					}
 				}
 				$this->updater->update($path2);
diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php
index f6af59d52be..704aabf2ad6 100644
--- a/tests/lib/files/view.php
+++ b/tests/lib/files/view.php
@@ -872,6 +872,57 @@ class View extends \Test\TestCase {
 		$this->assertEquals($time, $view->filemtime('/test/sub/storage/foo/bar.txt'));
 	}
 
+	public function testRenameFailDeleteTargetKeepSource() {
+		$this->doTestCopyRenameFail('rename');
+	}
+
+	public function testCopyFailDeleteTargetKeepSource() {
+		$this->doTestCopyRenameFail('copy');
+	}
+
+	private function doTestCopyRenameFail($operation) {
+		$storage1 = new Temporary(array());
+		$storage2 = new Temporary(array());
+		$storage2 = new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage2, 'quota' => 9));
+		$storage1->mkdir('sub');
+		$storage1->file_put_contents('foo.txt', '0123456789ABCDEFGH');
+		$storage1->mkdir('dirtomove');
+		$storage1->file_put_contents('dirtomove/indir1.txt', '0123456'); // fits
+		$storage1->file_put_contents('dirtomove/indir2.txt', '0123456789ABCDEFGH'); // doesn't fit 
+		$storage2->file_put_contents('existing.txt', '0123');
+		$storage1->getScanner()->scan('');
+		$storage2->getScanner()->scan('');
+		\OC\Files\Filesystem::mount($storage1, array(), '/test/');
+		\OC\Files\Filesystem::mount($storage2, array(), '/test/sub/storage');
+
+		// move file
+		$view = new \OC\Files\View('');
+		$this->assertTrue($storage1->file_exists('foo.txt'));
+		$this->assertFalse($storage2->file_exists('foo.txt'));
+		$this->assertFalse($view->$operation('/test/foo.txt', '/test/sub/storage/foo.txt'));
+		$this->assertFalse($storage2->file_exists('foo.txt'));
+		$this->assertFalse($storage2->getCache()->get('foo.txt'));
+		$this->assertTrue($storage1->file_exists('foo.txt'));
+
+		// if target exists, it will be deleted too
+		$this->assertFalse($view->$operation('/test/foo.txt', '/test/sub/storage/existing.txt'));
+		$this->assertFalse($storage2->file_exists('existing.txt'));
+		$this->assertFalse($storage2->getCache()->get('existing.txt'));
+		$this->assertTrue($storage1->file_exists('foo.txt'));
+
+		// move folder
+		$this->assertFalse($view->$operation('/test/dirtomove/', '/test/sub/storage/dirtomove/'));
+		// since the move failed, the full source tree is kept
+		$this->assertTrue($storage1->file_exists('dirtomove/indir1.txt'));
+		// but the target file stays
+		$this->assertTrue($storage2->file_exists('dirtomove/indir1.txt'));
+		// second file not moved/copied
+		$this->assertTrue($storage1->file_exists('dirtomove/indir2.txt'));
+		$this->assertFalse($storage2->file_exists('dirtomove/indir2.txt'));
+		$this->assertFalse($storage2->getCache()->get('dirtomove/indir2.txt'));
+
+	}
+
 	public function testDeleteFailKeepCache() {
 		/**
 		 * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage