From c1e0f98f87fa58edf7967d818732c7467cf47d80 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 27 Feb 2014 12:26:17 +1300 Subject: [PATCH] BUG Fix case where setFolder('/') would break UploadField::fileexists BUG Prevent Upload from writing to the site root folder --- filesystem/Upload.php | 4 +- forms/UploadField.php | 4 +- tests/forms/uploadfield/UploadFieldTest.php | 58 ++++++++++++++++++++- 3 files changed, 63 insertions(+), 3 deletions(-) diff --git a/filesystem/Upload.php b/filesystem/Upload.php index 90cb343dd..e669905d9 100644 --- a/filesystem/Upload.php +++ b/filesystem/Upload.php @@ -134,7 +134,9 @@ class Upload extends Controller { $file = $nameFilter->filter($tmpFile['name']); $fileName = basename($file); - $relativeFilePath = $parentFolder ? $parentFolder->getRelativePath() . "$fileName" : $fileName; + $relativeFilePath = $parentFolder + ? $parentFolder->getRelativePath() . "$fileName" + : ASSETS_DIR . "/" . $fileName; // Create a new file record (or try to retrieve an existing one) if(!$this->file) { diff --git a/forms/UploadField.php b/forms/UploadField.php index a09dc496a..362b9fc8d 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -1276,7 +1276,9 @@ class UploadField extends FileField { // Resolve expected folder name $folderName = $this->getFolderName(); $folder = Folder::find_or_make($folderName); - $parentPath = BASE_PATH."/".$folder->getFilename(); + $parentPath = $folder + ? BASE_PATH."/".$folder->getFilename() + : ASSETS_PATH."/"; // check if either file exists $exists = false; diff --git a/tests/forms/uploadfield/UploadFieldTest.php b/tests/forms/uploadfield/UploadFieldTest.php index 87bf460dd..af831abf7 100644 --- a/tests/forms/uploadfield/UploadFieldTest.php +++ b/tests/forms/uploadfield/UploadFieldTest.php @@ -643,6 +643,45 @@ class UploadFieldTest extends FunctionalTest { $this->assertNotContains($fileSubfolder->ID, $itemIDs, 'Does not contain file in subfolder'); } + /** + * Tests that UploadField::fileexist works + */ + public function testFileExists() { + $this->loginWithPermission('ADMIN'); + + // Check that fileexist works on subfolders + $nonFile = uniqid().'.txt'; + $responseEmpty = $this->mockFileExists('NoRelationField', $nonFile); + $responseEmptyData = json_decode($responseEmpty->getBody()); + $this->assertFalse($responseEmpty->isError()); + $this->assertFalse($responseEmptyData->exists); + + // Check that filexists works on root folder + $responseRoot = $this->mockFileExists('RootFolderTest', $nonFile); + $responseRootData = json_decode($responseRoot->getBody()); + $this->assertFalse($responseRoot->isError()); + $this->assertFalse($responseRootData->exists); + + // Check that uploaded files can be detected in the root + $tmpFileName = 'testUploadBasic.txt'; + $response = $this->mockFileUpload('RootFolderTest', $tmpFileName); + $this->assertFalse($response->isError()); + $this->assertFileExists(ASSETS_PATH . "/$tmpFileName"); + $responseExists = $this->mockFileExists('RootFolderTest', $tmpFileName); + $responseExistsData = json_decode($responseExists->getBody()); + $this->assertFalse($responseExists->isError()); + $this->assertTrue($responseExistsData->exists); + + // Check that uploaded files can be detected + $response = $this->mockFileUpload('NoRelationField', $tmpFileName); + $this->assertFalse($response->isError()); + $this->assertFileExists(ASSETS_PATH . "/UploadFieldTest/$tmpFileName"); + $responseExists = $this->mockFileExists('NoRelationField', $tmpFileName); + $responseExistsData = json_decode($responseExists->getBody()); + $this->assertFalse($responseExists->isError()); + $this->assertTrue($responseExistsData->exists); + } + protected function getMockForm() { return new Form(new Controller(), 'Form', new FieldList(), new FieldList()); } @@ -717,6 +756,12 @@ class UploadFieldTest extends FunctionalTest { ); } + protected function mockFileExists($fileField, $fileName) { + return $this->get( + "UploadFieldTest_Controller/Form/field/{$fileField}/fileexists?filename=".urlencode($fileName) + ); + } + /** * Simulates a physical file deletion * @@ -773,7 +818,14 @@ class UploadFieldTest extends FunctionalTest { } // Remove left over folders and any files that may exist - if(file_exists('../assets/UploadFieldTest')) Filesystem::removeFolder('../assets/UploadFieldTest'); + if(file_exists(ASSETS_PATH.'/UploadFieldTest')) { + Filesystem::removeFolder(ASSETS_PATH.'/UploadFieldTest'); + } + + // Remove file uploaded to root folder + if(file_exists(ASSETS_PATH.'/testUploadBasic.txt')) { + unlink(ASSETS_PATH.'/testUploadBasic.txt'); + } } } @@ -859,6 +911,9 @@ class UploadFieldTestForm extends Form implements TestOnly { if(empty($controller)) { $controller = new UploadFieldTest_Controller(); } + + $fieldRootFolder = UploadField::create('RootFolderTest') + ->setFolderName('/'); $fieldNoRelation = UploadField::create('NoRelationField') ->setFolderName('UploadFieldTest'); @@ -911,6 +966,7 @@ class UploadFieldTestForm extends Form implements TestOnly { $fieldAllowedExtensions->getValidator()->setAllowedExtensions(array('txt')); $fields = new FieldList( + $fieldRootFolder, $fieldNoRelation, $fieldHasOne, $fieldHasOneMaxOne,