From 58c7042e708172b8e2fe252fc53abe87bcf8c1f1 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 22 Jan 2014 17:17:58 +0100 Subject: [PATCH] Added error message for when target folder was removed Whent trying to upload/rename/create files in a folder that was removed or rename, the correct error message is now shown. In the case of upload of multiple files, the upload is cancelled. This situation can happen if the target folder was renamed or removed from another browser window or client. --- apps/files/ajax/newfile.php | 9 ++++++ apps/files/ajax/newfolder.php | 9 ++++++ apps/files/ajax/upload.php | 6 ++-- apps/files/js/file-upload.js | 7 +++++ apps/files/lib/app.php | 7 +++++ apps/files/tests/ajax_rename.php | 49 ++++++++++++++++++++++++++++++-- 6 files changed, 83 insertions(+), 4 deletions(-) diff --git a/apps/files/ajax/newfile.php b/apps/files/ajax/newfile.php index ec5b716fb2a..1853098c507 100644 --- a/apps/files/ajax/newfile.php +++ b/apps/files/ajax/newfile.php @@ -64,6 +64,15 @@ if(strpos($filename, '/') !== false) { exit(); } +if (!\OC\Files\Filesystem::file_exists($dir . '/')) { + $result['data'] = array('message' => (string)$l10n->t( + 'The target folder has been moved or deleted.'), + 'code' => 'targetnotfound' + ); + OCP\JSON::error($result); + exit(); +} + //TODO why is stripslashes used on foldername in newfolder.php but not here? $target = $dir.'/'.$filename; diff --git a/apps/files/ajax/newfolder.php b/apps/files/ajax/newfolder.php index 2cbc8cfeba5..4cfcae3090d 100644 --- a/apps/files/ajax/newfolder.php +++ b/apps/files/ajax/newfolder.php @@ -29,6 +29,15 @@ if(strpos($foldername, '/') !== false) { exit(); } +if (!\OC\Files\Filesystem::file_exists($dir . '/')) { + $result['data'] = array('message' => (string)$l10n->t( + 'The target folder has been moved or deleted.'), + 'code' => 'targetnotfound' + ); + OCP\JSON::error($result); + exit(); +} + //TODO why is stripslashes used on foldername here but not in newfile.php? $target = $dir . '/' . stripslashes($foldername); diff --git a/apps/files/ajax/upload.php b/apps/files/ajax/upload.php index bdaf6a77d14..8f6c42d6620 100644 --- a/apps/files/ajax/upload.php +++ b/apps/files/ajax/upload.php @@ -8,6 +8,7 @@ OCP\JSON::setContentTypeHeader('text/plain'); // If no token is sent along, rely on login only $allowedPermissions = OCP\PERMISSION_ALL; +$errorCode = null; $l = OC_L10N::get('files'); if (empty($_POST['dirToken'])) { @@ -125,7 +126,8 @@ if (strpos($dir, '..') === false) { $meta = \OC\Files\Filesystem::getFileInfo($target); if ($meta === false) { - $error = $l->t('Upload failed. Could not get file info.'); + $error = $l->t('The target folder has been moved or deleted.'); + $errorCode = 'targetnotfound'; } else { $result[] = array('status' => 'success', 'mime' => $meta['mimetype'], @@ -177,5 +179,5 @@ if ($error === false) { OCP\JSON::encodedPrint($result); exit(); } else { - OCP\JSON::error(array(array('data' => array_merge(array('message' => $error), $storageStats)))); + OCP\JSON::error(array(array('data' => array_merge(array('message' => $error, 'code' => $errorCode), $storageStats)))); } diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 225c3319107..486273a910c 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -315,6 +315,13 @@ $(document).ready(function() { } else { // HTTP connection problem OC.Notification.show(data.errorThrown); + if (data.result) { + var result = JSON.parse(data.result); + if (result && result[0] && result[0].data && result[0].data.code === 'targetnotfound') { + // abort upload of next files if any + OC.Upload.cancelUploads(); + } + } } //hide notification after 10 sec setTimeout(function() { diff --git a/apps/files/lib/app.php b/apps/files/lib/app.php index 1ac266073db..fea88faa92a 100644 --- a/apps/files/lib/app.php +++ b/apps/files/lib/app.php @@ -59,6 +59,13 @@ class App { $result['data'] = array( 'message' => $this->l10n->t("Invalid folder name. Usage of 'Shared' is reserved.") ); + // rename to non-existing folder is denied + } else if (!$this->view->file_exists($dir)) { + $result['data'] = array('message' => (string)$this->l10n->t( + 'The target folder has been moved or deleted.', + array($dir)), + 'code' => 'targetnotfound' + ); // rename to existing file is denied } else if ($this->view->file_exists($dir . '/' . $newname)) { diff --git a/apps/files/tests/ajax_rename.php b/apps/files/tests/ajax_rename.php index 350ff5d3687..a1a5c8983ba 100644 --- a/apps/files/tests/ajax_rename.php +++ b/apps/files/tests/ajax_rename.php @@ -38,7 +38,7 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $l10nMock->expects($this->any()) ->method('t') ->will($this->returnArgument(0)); - $viewMock = $this->getMock('\OC\Files\View', array('rename', 'normalizePath', 'getFileInfo'), array(), '', false); + $viewMock = $this->getMock('\OC\Files\View', array('rename', 'normalizePath', 'getFileInfo', 'file_exists'), array(), '', false); $viewMock->expects($this->any()) ->method('normalizePath') ->will($this->returnArgument(0)); @@ -63,6 +63,11 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $oldname = 'Shared'; $newname = 'new_name'; + $this->viewMock->expects($this->at(0)) + ->method('file_exists') + ->with('/') + ->will($this->returnValue(true)); + $result = $this->files->rename($dir, $oldname, $newname); $expected = array( 'success' => false, @@ -80,6 +85,11 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $oldname = 'Shared'; $newname = 'new_name'; + $this->viewMock->expects($this->at(0)) + ->method('file_exists') + ->with('/test') + ->will($this->returnValue(true)); + $this->viewMock->expects($this->any()) ->method('getFileInfo') ->will($this->returnValue(array( @@ -129,6 +139,11 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $oldname = 'oldname'; $newname = 'newname'; + $this->viewMock->expects($this->at(0)) + ->method('file_exists') + ->with('/') + ->will($this->returnValue(true)); + $this->viewMock->expects($this->any()) ->method('getFileInfo') ->will($this->returnValue(array( @@ -141,7 +156,6 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { 'name' => 'new_name', ))); - $result = $this->files->rename($dir, $oldname, $newname); $this->assertTrue($result['success']); @@ -154,4 +168,35 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { $this->assertEquals(\OC_Helper::mimetypeIcon('dir'), $result['data']['icon']); $this->assertFalse($result['data']['isPreviewAvailable']); } + + /** + * Test rename inside a folder that doesn't exist any more + */ + function testRenameInNonExistingFolder() { + $dir = '/unexist'; + $oldname = 'oldname'; + $newname = 'newname'; + + $this->viewMock->expects($this->at(0)) + ->method('file_exists') + ->with('/unexist') + ->will($this->returnValue(false)); + + $this->viewMock->expects($this->any()) + ->method('getFileInfo') + ->will($this->returnValue(array( + 'fileid' => 123, + 'type' => 'dir', + 'mimetype' => 'httpd/unix-directory', + 'size' => 18, + 'etag' => 'abcdef', + 'directory' => '/unexist', + 'name' => 'new_name', + ))); + + $result = $this->files->rename($dir, $oldname, $newname); + + $this->assertFalse($result['success']); + $this->assertEquals('targetnotfound', $result['data']['code']); + } } -- 2.39.5