Commit 9baf7581 authored by Lukas Reschke's avatar Lukas Reschke
Browse files

Merge pull request #14503 from owncloud/quota-preventdatalossforfailedmove

Fix file move/copy when storage space is not enough
parents 2d9886d1 232de3bd
...@@ -574,8 +574,11 @@ class View { ...@@ -574,8 +574,11 @@ class View {
} }
/** /**
* @param string $path1 * Rename/move a file or folder from the source path to target path.
* @param string $path2 *
* @param string $path1 source path
* @param string $path2 target path
*
* @return bool|mixed * @return bool|mixed
*/ */
public function rename($path1, $path2) { public function rename($path1, $path2) {
...@@ -617,7 +620,7 @@ class View { ...@@ -617,7 +620,7 @@ class View {
$mount = $manager->find($absolutePath1 . $postFix1); $mount = $manager->find($absolutePath1 . $postFix1);
$storage1 = $mount->getStorage(); $storage1 = $mount->getStorage();
$internalPath1 = $mount->getInternalPath($absolutePath1 . $postFix1); $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 ($internalPath1 === '' and $mount instanceof MoveableMount) {
if ($this->isTargetAllowed($absolutePath2)) { if ($this->isTargetAllowed($absolutePath2)) {
/** /**
...@@ -646,8 +649,10 @@ class View { ...@@ -646,8 +649,10 @@ class View {
} else { } else {
$source = $this->fopen($path1 . $postFix1, 'r'); $source = $this->fopen($path1 . $postFix1, 'r');
$target = $this->fopen($path2 . $postFix2, 'w'); $target = $this->fopen($path2 . $postFix2, 'w');
list($count, $result) = \OC_Helper::streamCopy($source, $target); list(, $result) = \OC_Helper::streamCopy($source, $target);
$this->touch($path2, $this->filemtime($path1)); if ($result !== false) {
$this->touch($path2, $this->filemtime($path1));
}
// close open handle - especially $source is necessary because unlink below will // close open handle - especially $source is necessary because unlink below will
// throw an exception on windows because the file is locked // throw an exception on windows because the file is locked
...@@ -656,6 +661,11 @@ class View { ...@@ -656,6 +661,11 @@ class View {
if ($result !== false) { if ($result !== false) {
$result &= $storage1->unlink($internalPath1); $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 { ...@@ -688,9 +698,12 @@ class View {
} }
/** /**
* @param string $path1 * Copy a file/folder from the source path to target path
* @param string $path2 *
* @param bool $preserveMtime * @param string $path1 source path
* @param string $path2 target path
* @param bool $preserveMtime whether to preserve mtime on the copy
*
* @return bool|mixed * @return bool|mixed
*/ */
public function copy($path1, $path2, $preserveMtime = false) { public function copy($path1, $path2, $preserveMtime = false) {
...@@ -732,6 +745,11 @@ class View { ...@@ -732,6 +745,11 @@ class View {
list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2); list(, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
if ($storage) { if ($storage) {
$result = $storage->copy($internalPath1, $internalPath2); $result = $storage->copy($internalPath1, $internalPath2);
if (!$result) {
// delete partially written target file
$storage->unlink($internalPath2);
$storage->getCache()->remove($internalPath2);
}
} else { } else {
$result = false; $result = false;
} }
...@@ -749,14 +767,20 @@ class View { ...@@ -749,14 +767,20 @@ class View {
} }
} }
} else { } else {
list($storage2, $internalPath2) = Filesystem::resolvePath($absolutePath2 . $postFix2);
$source = $this->fopen($path1 . $postFix1, 'r'); $source = $this->fopen($path1 . $postFix1, 'r');
$target = $this->fopen($path2 . $postFix2, 'w'); $target = $this->fopen($path2 . $postFix2, 'w');
list($count, $result) = \OC_Helper::streamCopy($source, $target); list(, $result) = \OC_Helper::streamCopy($source, $target);
if($preserveMtime) { if($result && $preserveMtime) {
$this->touch($path2, $this->filemtime($path1)); $this->touch($path2, $this->filemtime($path1));
} }
fclose($source); fclose($source);
fclose($target); fclose($target);
if (!$result) {
// delete partially written target file
$storage2->unlink($internalPath2);
$storage2->getCache()->remove($internalPath2);
}
} }
} }
$this->updater->update($path2); $this->updater->update($path2);
......
...@@ -594,13 +594,23 @@ class OC_Helper { ...@@ -594,13 +594,23 @@ class OC_Helper {
if (!$source or !$target) { if (!$source or !$target) {
return array(0, false); return array(0, false);
} }
$bufSize = 8192;
$result = true; $result = true;
$count = 0; $count = 0;
while (!feof($source)) { while (!feof($source)) {
if (($c = fwrite($target, fread($source, 8192))) === false) { $buf = fread($source, $bufSize);
$bytesWritten = fwrite($target, $buf);
if ($bytesWritten !== false) {
$count += $bytesWritten;
}
// note: strlen is expensive so only use it when necessary,
// on the last block
if ($bytesWritten === false
|| ($bytesWritten < $bufSize && $bytesWritten < strlen($buf))
) {
// write error, could be disk full ?
$result = false; $result = false;
} else { break;
$count += $c;
} }
} }
return array($count, $result); return array($count, $result);
......
...@@ -99,6 +99,28 @@ class Quota extends \Test\Files\Storage\Storage { ...@@ -99,6 +99,28 @@ class Quota extends \Test\Files\Storage\Storage {
$this->assertEquals('foobarqwe', $instance->file_get_contents('foo')); $this->assertEquals('foobarqwe', $instance->file_get_contents('foo'));
} }
public function testStreamCopyWithEnoughSpace() {
$instance = $this->getLimitedStorage(16);
$inputStream = fopen('data://text/plain,foobarqwerty', 'r');
$outputStream = $instance->fopen('foo', 'w+');
list($count, $result) = \OC_Helper::streamCopy($inputStream, $outputStream);
$this->assertEquals(12, $count);
$this->assertTrue($result);
fclose($inputStream);
fclose($outputStream);
}
public function testStreamCopyNotEnoughSpace() {
$instance = $this->getLimitedStorage(9);
$inputStream = fopen('data://text/plain,foobarqwerty', 'r');
$outputStream = $instance->fopen('foo', 'w+');
list($count, $result) = \OC_Helper::streamCopy($inputStream, $outputStream);
$this->assertEquals(9, $count);
$this->assertFalse($result);
fclose($inputStream);
fclose($outputStream);
}
public function testReturnFalseWhenFopenFailed() { public function testReturnFalseWhenFopenFailed() {
$failStorage = $this->getMock( $failStorage = $this->getMock(
'\OC\Files\Storage\Local', '\OC\Files\Storage\Local',
......
...@@ -872,6 +872,57 @@ class View extends \Test\TestCase { ...@@ -872,6 +872,57 @@ class View extends \Test\TestCase {
$this->assertEquals($time, $view->filemtime('/test/sub/storage/foo/bar.txt')); $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() { public function testDeleteFailKeepCache() {
/** /**
* @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage * @var \PHPUnit_Framework_MockObject_MockObject | \OC\Files\Storage\Temporary $storage
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment