Delete target file for unsuccessful copy/rename

diff --git a/lib/private/files/view.php b/lib/private/files/view.php
index 7de7ef7b32c46ca7ec17b9ef78b3d40634eccb8b..e4732f8bb3d27200e6ae81ea1b3ee9294379f802 100644 (file)
--- a/lib/private/files/view.php
+++ b/lib/private/files/view.php
@@ -574,8 +574,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) {
@@ -617,7 +620,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)) {
                                                /**
@@ -646,8 +649,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
@@ -656,6 +661,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);
                                                }
                                        }
                                }
@@ -688,9 +698,12 @@ class View {
        }
 
        /**
-        * @param string $path1
-        * @param string $path2
-        * @param bool $preserveMtime
+        * 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) {
@@ -732,6 +745,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;
                                        }
@@ -749,14 +767,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 b4b6d0deb2ea69be34782db07505f88bce8d3a3e..0d88ec1d66a8607b130eac21acec1bd6fe023f26 100644 (file)
--- 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