From f19048c0c6247a2203c5a139fefb962da87bd1a9 Mon Sep 17 00:00:00 2001 From: Sean Harvey Date: Mon, 15 Mar 2010 06:31:40 +0000 Subject: [PATCH] BUGFIX #5188 Upload and Folder don't handle the duplicate naming of files that have no extension git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@101050 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- filesystem/Folder.php | 9 +++ filesystem/Upload.php | 14 +++- tests/filesystem/UploadTest.php | 118 ++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) diff --git a/filesystem/Folder.php b/filesystem/Folder.php index 86220f2ba..3a5a3ea39 100755 --- a/filesystem/Folder.php +++ b/filesystem/Folder.php @@ -214,6 +214,15 @@ class Folder extends File { $i++; $oldFile = $file; $file = "$origFile-$i"; + + if(strpos($file, '.') !== false) { + $file = ereg_replace('[0-9]*(\.[^.]+$)',$i . '\\1', $file); + } else if (strpos($file, '_') !== false) { + $file = ereg_replace('_([^_]+$)', '_'.$i, $file); + } else { + $file .= "_$i"; + } + if($oldFile == $file && $i > 2) user_error("Couldn't fix $file$ext with $i", E_USER_ERROR); } diff --git a/filesystem/Upload.php b/filesystem/Upload.php index c76fbfec0..44addfe90 100644 --- a/filesystem/Upload.php +++ b/filesystem/Upload.php @@ -127,8 +127,12 @@ class Upload extends Controller { if(substr($relativeFilePath, strlen($relativeFilePath) - strlen('.tar.gz')) == '.tar.gz' || substr($relativeFilePath, strlen($relativeFilePath) - strlen('.tar.bz2')) == '.tar.bz2') { $relativeFilePath = ereg_replace('[0-9]*(\.tar\.[^.]+$)',$i . '\\1', $relativeFilePath); - } else { + } else if (strpos($relativeFilePath, '.') !== false) { $relativeFilePath = ereg_replace('[0-9]*(\.[^.]+$)',$i . '\\1', $relativeFilePath); + } else if (strpos($relativeFilePath, '_') !== false) { + $relativeFilePath = ereg_replace('_([^_]+$)', '_'.$i, $relativeFilePath); + } else { + $relativeFilePath .= "_$i"; } if($oldFilePath == $relativeFilePath && $i > 2) user_error("Couldn't fix $relativeFilePath with $i tries", E_USER_ERROR); } @@ -433,7 +437,13 @@ class Upload_Validator { */ public function isValidExtension() { $pathInfo = pathinfo($this->tmpFile['name']); - return (!count($this->allowedExtensions) || in_array(strtolower($pathInfo['extension']), $this->allowedExtensions)); + + // Special case for filenames with an extension + if(!isset($pathInfo['extension'])) { + return (in_array('', $this->allowedExtensions, true)) ? true : false; + } else { + return (!count($this->allowedExtensions) || in_array(strtolower($pathInfo['extension']), $this->allowedExtensions)); + } } /** diff --git a/tests/filesystem/UploadTest.php b/tests/filesystem/UploadTest.php index 77f12e29b..e21bea865 100644 --- a/tests/filesystem/UploadTest.php +++ b/tests/filesystem/UploadTest.php @@ -127,6 +127,124 @@ class UploadTest extends SapphireTest { $file->delete(); } + function testUploadDeniesNoExtensionFilesIfNoEmptyStringSetForValidatorExtensions() { + // create tmp file + $tmpFileName = 'UploadTest_testUpload'; + $tmpFilePath = TEMP_FOLDER . '/' . $tmpFileName; + $tmpFileContent = ''; + for($i=0; $i<10000; $i++) $tmpFileContent .= '0'; + file_put_contents($tmpFilePath, $tmpFileContent); + + // emulates the $_FILES array + $tmpFile = array( + 'name' => $tmpFileName, + 'type' => 'text/plaintext', + 'size' => filesize($tmpFilePath), + 'tmp_name' => $tmpFilePath, + 'extension' => 'txt', + 'error' => UPLOAD_ERR_OK, + ); + + $v = new UploadTest_Validator(); + $v->setAllowedExtensions(array('txt')); + + // test upload into default folder + $u = new Upload(); + $result = $u->load($tmpFile); + + $this->assertFalse($result, 'Load failed because extension was not accepted'); + $this->assertEquals(1, count($u->getErrors()), 'There is a single error of the file extension'); + + } + + function testUploadTarGzFileTwiceAppendsNumber() { + // create tmp file + $tmpFileName = 'UploadTest_testUpload.tar.gz'; + $tmpFilePath = TEMP_FOLDER . '/' . $tmpFileName; + $tmpFileContent = ''; + for($i=0; $i<10000; $i++) $tmpFileContent .= '0'; + file_put_contents($tmpFilePath, $tmpFileContent); + + // emulates the $_FILES array + $tmpFile = array( + 'name' => $tmpFileName, + 'type' => 'text/plaintext', + 'size' => filesize($tmpFilePath), + 'tmp_name' => $tmpFilePath, + 'extension' => 'txt', + 'error' => UPLOAD_ERR_OK, + ); + + // test upload into default folder + $u = new Upload(); + $u->load($tmpFile); + $file = $u->getFile(); + $this->assertEquals( + 'UploadTesttestUpload.tar.gz', + $file->Name, + 'File has a name without a number because it\'s not a duplicate' + ); + + $u = new Upload(); + $u->load($tmpFile); + $file2 = $u->getFile(); + $this->assertEquals( + 'UploadTesttestUpload2.tar.gz', + $file2->Name, + 'File receives a number attached to the end before the extension' + ); + + $file->delete(); + $file2->delete(); + } + + function testUploadFileWithNoExtensionTwiceAppendsNumber() { + // create tmp file + $tmpFileName = 'UploadTest_testUpload'; + $tmpFilePath = TEMP_FOLDER . '/' . $tmpFileName; + $tmpFileContent = ''; + for($i=0; $i<10000; $i++) $tmpFileContent .= '0'; + file_put_contents($tmpFilePath, $tmpFileContent); + + // emulates the $_FILES array + $tmpFile = array( + 'name' => $tmpFileName, + 'type' => 'text/plaintext', + 'size' => filesize($tmpFilePath), + 'tmp_name' => $tmpFilePath, + 'extension' => 'txt', + 'error' => UPLOAD_ERR_OK, + ); + + $v = new UploadTest_Validator(); + $v->setAllowedExtensions(array('')); + + // test upload into default folder + $u = new Upload(); + $u->setValidator($v); + $u->load($tmpFile); + $file = $u->getFile(); + + $this->assertEquals( + 'UploadTesttestUpload', + $file->Name, + 'File is uploaded without extension' + ); + + $u = new Upload(); + $u->setValidator($v); + $u->load($tmpFile); + $file2 = $u->getFile(); + $this->assertEquals( + 'UploadTesttestUpload_2', + $file2->Name, + 'File receives a number attached to the end' + ); + + $file->delete(); + $file2->delete(); + } + } class UploadTest_Validator extends Upload_Validator implements TestOnly {