Properly handle copy/move failures in cross storage copy/move

diff --git a/lib/private/files/storage/common.php b/lib/private/files/storage/common.php
index ca898bcc0b3f0b4bedbefc50aee07c8cb775823e..66ed713e22d1d0a99dce1b95b545c01d02be0351 100644 (file)
--- a/lib/private/files/storage/common.php
+++ b/lib/private/files/storage/common.php
@@ -537,7 +537,7 @@ abstract class Common implements Storage {
                        $dh = $sourceStorage->opendir($sourceInternalPath);
                        $result = $this->mkdir($targetInternalPath);
                        if (is_resource($dh)) {
-                               while (($file = readdir($dh)) !== false) {
+                               while ($result and ($file = readdir($dh)) !== false) {
                                        if (!Filesystem::isIgnoredDir($file)) {
                                                $result &= $this->copyFromStorage($sourceStorage, $sourceInternalPath . '/' . $file, $targetInternalPath . '/' . $file);
                                        }
@@ -547,13 +547,20 @@ abstract class Common implements Storage {
                        $source = $sourceStorage->fopen($sourceInternalPath, 'r');
                        $target = $this->fopen($targetInternalPath, 'w');
                        list(, $result) = \OC_Helper::streamCopy($source, $target);
-                       if ($preserveMtime) {
+                       if ($result and $preserveMtime) {
                                $this->touch($targetInternalPath, $sourceStorage->filemtime($sourceInternalPath));
                        }
                        fclose($source);
                        fclose($target);
+
+                       if (!$result) {
+                               // delete partially written target file
+                               $this->unlink($targetInternalPath);
+                               // delete cache entry that was created by fopen
+                               $this->getCache()->remove($targetInternalPath);
+                       }
                }
-               return $result;
+               return (bool)$result;
        }
 
        /**

diff --git a/tests/lib/files/view.php b/tests/lib/files/view.php
index 3b5e3aa3b0815d8feb200871023a8d6644132435..269b8b23e7d3b595fefd34b2eb3cce65eb89e1ae 100644 (file)
--- a/tests/lib/files/view.php
+++ b/tests/lib/files/view.php
@@ -944,8 +944,19 @@ class View extends \Test\TestCase {
 
        private function doTestCopyRenameFail($operation) {
                $storage1 = new Temporary(array());
-               $storage2 = new Temporary(array());
-               $storage2 = new \OC\Files\Storage\Wrapper\Quota(array('storage' => $storage2, 'quota' => 9));
+               /** @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage2 */
+               $storage2 = $this->getMockBuilder('\Test\Files\TemporaryNoCross')
+                       ->setConstructorArgs([[]])
+                       ->setMethods(['fopen'])
+                       ->getMock();
+
+               $storage2->expects($this->any())
+                       ->method('fopen')
+                       ->will($this->returnCallback(function($path, $mode) use($storage2) {
+                               $source = fopen($storage2->getSourcePath($path), $mode);
+                               return \OC\Files\Stream\Quota::wrap($source, 9);
+                       }));
+
                $storage1->mkdir('sub');
                $storage1->file_put_contents('foo.txt', '0123456789ABCDEFGH');
                $storage1->mkdir('dirtomove');
@@ -976,10 +987,8 @@ class View extends \Test\TestCase {
                $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'));
+               // second file not moved/copied
                $this->assertFalse($storage2->file_exists('dirtomove/indir2.txt'));
                $this->assertFalse($storage2->getCache()->get('dirtomove/indir2.txt'));