From 27c5a978f91e7aa447a2acca040fd173baba53b9 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 8 Apr 2014 17:17:48 +0200 Subject: [PATCH] we no longer need to handle the Shared folder different from any other folder --- apps/files/js/file-upload.js | 4 +- apps/files/js/fileactions.js | 8 +-- apps/files/js/filelist.js | 2 +- apps/files/js/files.js | 6 +- apps/files/lib/app.php | 11 +-- apps/files/tests/ajax_rename.php | 82 ---------------------- apps/files/tests/js/filesSpec.js | 35 --------- lib/private/connector/sabre/directory.php | 12 ---- lib/private/connector/sabre/file.php | 11 --- lib/private/connector/sabre/objecttree.php | 3 - 10 files changed, 8 insertions(+), 166 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 3879aa6588..03ebdccb32 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -235,7 +235,7 @@ OC.Upload = { var file = data.files[0]; try { // FIXME: not so elegant... need to refactor that method to return a value - Files.isFileNameValid(file.name, FileList.getCurrentDirectory()); + Files.isFileNameValid(file.name); } catch (errorMessage) { data.textStatus = 'invalidcharacters'; @@ -555,8 +555,6 @@ OC.Upload = { throw t('files', 'URL cannot be empty'); } else if (type !== 'web' && !Files.isFileNameValid(filename)) { // Files.isFileNameValid(filename) throws an exception itself - } else if (FileList.getCurrentDirectory() === '/' && filename.toLowerCase() === 'shared') { - throw t('files', 'In the home folder \'Shared\' is a reserved filename'); } else if (FileList.inList(filename)) { throw t('files', '{new_name} already exists', {new_name: filename}); } else { diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 631aebea95..ecdfa72a47 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -118,10 +118,6 @@ var FileActions = { }; var addAction = function (name, action, displayName) { - // NOTE: Temporary fix to prevent rename action in root of Shared directory - if (name === 'Rename' && $('#dir').val() === '/Shared') { - return true; - } if ((name === 'Download' || action !== defaultAction) && name !== 'Delete') { @@ -160,7 +156,7 @@ var FileActions = { addAction(name, ah, displayName); } }); - if(actions.Share && !($('#dir').val() === '/' && file === 'Shared')){ + if(actions.Share){ displayName = t('files', 'Share'); addAction('Share', actions.Share, displayName); } @@ -223,7 +219,7 @@ $(document).ready(function () { $('#fileList tr').each(function () { FileActions.display($(this).children('td.filename')); }); - + $('#fileList').trigger(jQuery.Event("fileActionsReady")); }); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 9c749bb8f3..343da21741 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -580,7 +580,7 @@ window.FileList = { var filename = input.val(); if (filename !== oldname) { // Files.isFileNameValid(filename) throws an exception itself - Files.isFileNameValid(filename, FileList.getCurrentDirectory()); + Files.isFileNameValid(filename); if (FileList.inList(filename)) { throw t('files', '{new_name} already exists', {new_name: filename}); } diff --git a/apps/files/js/files.js b/apps/files/js/files.js index 9f38263bef..5e669a796a 100644 --- a/apps/files/js/files.js +++ b/apps/files/js/files.js @@ -87,11 +87,9 @@ var Files = { * Throws a string exception with an error message if * the file name is not valid */ - isFileNameValid: function (name, root) { + isFileNameValid: function (name) { var trimmedName = name.trim(); - if (trimmedName === '.' - || trimmedName === '..' - || (root === '/' && trimmedName.toLowerCase() === 'shared')) + if (trimmedName === '.' || trimmedName === '..') { throw t('files', '"{name}" is an invalid file name.', {name: name}); } else if (trimmedName.length === 0) { diff --git a/apps/files/lib/app.php b/apps/files/lib/app.php index adfca66957..ed4aa32c66 100644 --- a/apps/files/lib/app.php +++ b/apps/files/lib/app.php @@ -54,13 +54,8 @@ class App { 'data' => NULL ); - // rename to "/Shared" is denied - if( $dir === '/' and $newname === 'Shared' ) { - $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)) { + if (!$this->view->file_exists($dir)) { $result['data'] = array('message' => (string)$this->l10n->t( 'The target folder has been moved or deleted.', array($dir)), @@ -68,7 +63,7 @@ class App { ); // rename to existing file is denied } else if ($this->view->file_exists($dir . '/' . $newname)) { - + $result['data'] = array( 'message' => $this->l10n->t( "The name %s is already used in the folder %s. Please choose a different name.", @@ -77,8 +72,6 @@ class App { } else if ( // rename to "." is denied $newname !== '.' and - // rename of "/Shared" is denied - !($dir === '/' and $oldname === 'Shared') and // THEN try to rename $this->view->rename($dir . '/' . $oldname, $dir . '/' . $newname) ) { diff --git a/apps/files/tests/ajax_rename.php b/apps/files/tests/ajax_rename.php index cb62d22a7e..74ca1e4495 100644 --- a/apps/files/tests/ajax_rename.php +++ b/apps/files/tests/ajax_rename.php @@ -55,88 +55,6 @@ class Test_OC_Files_App_Rename extends \PHPUnit_Framework_TestCase { \OC\Files\Filesystem::tearDown(); } - /** - * @brief test rename of file/folder named "Shared" - */ - function testRenameSharedFolder() { - $dir = '/'; - $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, - 'data' => array('message' => '%s could not be renamed') - ); - - $this->assertEquals($expected, $result); - } - - /** - * @brief test rename of file/folder named "Shared" - */ - function testRenameSharedFolderInSubdirectory() { - $dir = '/test'; - $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(new \OC\Files\FileInfo( - '/test', - null, - '/test', - array( - 'fileid' => 123, - 'type' => 'dir', - 'mimetype' => 'httpd/unix-directory', - 'mtime' => 0, - 'permissions' => 31, - 'size' => 18, - 'etag' => 'abcdef', - 'directory' => '/', - 'name' => 'new_name', - )))); - - $result = $this->files->rename($dir, $oldname, $newname); - - $this->assertTrue($result['success']); - $this->assertEquals(123, $result['data']['id']); - $this->assertEquals('new_name', $result['data']['name']); - $this->assertEquals(18, $result['data']['size']); - $this->assertEquals('httpd/unix-directory', $result['data']['mimetype']); - $icon = \OC_Helper::mimetypeIcon('dir'); - $icon = substr($icon, 0, -3) . 'svg'; - $this->assertEquals($icon, $result['data']['icon']); - } - - /** - * @brief test rename of file/folder to "Shared" - */ - function testRenameFolderToShared() { - $dir = '/'; - $oldname = 'oldname'; - $newname = 'Shared'; - - $result = $this->files->rename($dir, $oldname, $newname); - $expected = array( - 'success' => false, - 'data' => array('message' => "Invalid folder name. Usage of 'Shared' is reserved.") - ); - - $this->assertEquals($expected, $result); - } - /** * @brief test rename of file/folder */ diff --git a/apps/files/tests/js/filesSpec.js b/apps/files/tests/js/filesSpec.js index 95bf87e03e..018c8ef0f3 100644 --- a/apps/files/tests/js/filesSpec.js +++ b/apps/files/tests/js/filesSpec.js @@ -48,41 +48,6 @@ describe('Files tests', function() { expect(error).toEqual(false); } }); - it('Validates correct file names do not create Shared folder in root', function() { - // create shared file in subfolder - var error = false; - try { - expect(Files.isFileNameValid('shared', '/foo')).toEqual(true); - expect(Files.isFileNameValid('Shared', '/foo')).toEqual(true); - } - catch (e) { - error = e; - } - expect(error).toEqual(false); - - // create shared file in root - var threwException = false; - try { - Files.isFileNameValid('Shared', '/'); - console.error('Invalid file name not detected'); - } - catch (e) { - threwException = true; - } - expect(threwException).toEqual(true); - - // create shared file in root - var threwException = false; - try { - Files.isFileNameValid('shared', '/'); - console.error('Invalid file name not detected'); - } - catch (e) { - threwException = true; - } - expect(threwException).toEqual(true); - - }); it('Detects invalid file names', function() { var fileNames = [ '', diff --git a/lib/private/connector/sabre/directory.php b/lib/private/connector/sabre/directory.php index 70f45aa1e7..5d386a772a 100644 --- a/lib/private/connector/sabre/directory.php +++ b/lib/private/connector/sabre/directory.php @@ -50,10 +50,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function createFile($name, $data = null) { - if (strtolower($name) === 'shared' && empty($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - // for chunked upload also updating a existing file is a "createFile" // because we create all the chunks before reasamble them to the existing file. if (isset($_SERVER['HTTP_OC_CHUNKED'])) { @@ -86,10 +82,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function createDirectory($name) { - if (strtolower($name) === 'shared' && empty($this->path)) { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!\OC\Files\Filesystem::isCreatable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } @@ -196,10 +188,6 @@ class OC_Connector_Sabre_Directory extends OC_Connector_Sabre_Node implements Sa */ public function delete() { - if ($this->path === 'Shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!\OC\Files\Filesystem::isDeletable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } diff --git a/lib/private/connector/sabre/file.php b/lib/private/connector/sabre/file.php index 750d646a8f..433b4d805c 100644 --- a/lib/private/connector/sabre/file.php +++ b/lib/private/connector/sabre/file.php @@ -71,13 +71,6 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D // mark file as partial while uploading (ignored by the scanner) $partpath = $this->path . '.ocTransferId' . rand() . '.part'; - // if file is located in /Shared we write the part file to the users - // root folder because we can't create new files in /shared - // we extend the name with a random number to avoid overwriting a existing file - if (dirname($partpath) === 'Shared') { - $partpath = pathinfo($partpath, PATHINFO_FILENAME) . rand() . '.part'; - } - try { $putOkay = $fs->file_put_contents($partpath, $data); if ($putOkay === false) { @@ -149,10 +142,6 @@ class OC_Connector_Sabre_File extends OC_Connector_Sabre_Node implements Sabre_D public function delete() { $fs = $this->getFS(); - if ($this->path === 'Shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } - if (!$fs->isDeletable($this->path)) { throw new \Sabre_DAV_Exception_Forbidden(); } diff --git a/lib/private/connector/sabre/objecttree.php b/lib/private/connector/sabre/objecttree.php index accf020daa..d2fa425b22 100644 --- a/lib/private/connector/sabre/objecttree.php +++ b/lib/private/connector/sabre/objecttree.php @@ -94,9 +94,6 @@ class ObjectTree extends \Sabre_DAV_ObjectTree { } if ($sourceDir !== $destinationDir) { // for a full move we need update privileges on sourcePath and sourceDir as well as destinationDir - if (ltrim($destinationDir, '/') === '' && strtolower($sourceNode->getName()) === 'shared') { - throw new \Sabre_DAV_Exception_Forbidden(); - } if (!$fs->isUpdatable($sourceDir)) { throw new \Sabre_DAV_Exception_Forbidden(); }