diff --git a/lib/private/files/view.php b/lib/private/files/view.php index 7de7ef7b32..e4732f8bb3 100644 --- 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 b4b6d0deb2..0d88ec1d66 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