diff --git a/filesystem/Folder.php b/filesystem/Folder.php index 60c70f6c4..4984d34c0 100755 --- a/filesystem/Folder.php +++ b/filesystem/Folder.php @@ -250,6 +250,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 {