From 478edfa0c616bf00f6d0a4098e950cad39a7e20b Mon Sep 17 00:00:00 2001 From: Devlin Date: Tue, 21 Oct 2014 11:28:18 +0200 Subject: [PATCH] BUG Upload: File versioning with existing files reinsert oldFilePath = relativeFilePath in while loop --- filesystem/Upload.php | 3 ++ tests/filesystem/UploadTest.php | 64 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/filesystem/Upload.php b/filesystem/Upload.php index 370116ff3..7905803d9 100644 --- a/filesystem/Upload.php +++ b/filesystem/Upload.php @@ -173,6 +173,8 @@ class Upload extends Controller { } while(file_exists("$base/$relativeFilePath")) { $i = isset($i) ? ($i+1) : 2; + $oldFilePath = $relativeFilePath; + $pattern = '/([0-9]+$)/'; if(preg_match($pattern, $fileTitle)) { $fileTitle = preg_replace($pattern, $i, $fileTitle); @@ -180,6 +182,7 @@ class Upload extends Controller { $fileTitle .= $i; } $relativeFilePath = $relativeFolderPath . $fileTitle . $fileSuffix; + if($oldFilePath == $relativeFilePath && $i > 2) { user_error("Couldn't fix $relativeFilePath with $i tries", E_USER_ERROR); } diff --git a/tests/filesystem/UploadTest.php b/tests/filesystem/UploadTest.php index 4416f85e0..1686d8e04 100644 --- a/tests/filesystem/UploadTest.php +++ b/tests/filesystem/UploadTest.php @@ -560,6 +560,70 @@ class UploadTest extends SapphireTest { $image->delete(); } + public function testFileVersioningWithAnExistingFile() { + $upload = function($tmpFileName) { + // create tmp file + $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' => 'jpg', + 'error' => UPLOAD_ERR_OK, + ); + + $v = new UploadTest_Validator(); + + // test upload into default folder + $u = new Upload(); + $u->setReplaceFile(false); + $u->setValidator($v); + $u->load($tmpFile); + return $u->getFile(); + }; + + $file1 = $upload('UploadTest-testUpload.jpg'); + $this->assertEquals( + 'UploadTest-testUpload.jpg', + $file1->Name, + 'File does not receive new name' + ); + + $file2 = $upload('UploadTest-testUpload.jpg'); + $this->assertEquals( + 'UploadTest-testUpload2.jpg', + $file2->Name, + 'File does receive new name' + ); + + $file3 = $upload('UploadTest-testUpload.jpg'); + $this->assertEquals( + 'UploadTest-testUpload3.jpg', + $file3->Name, + 'File does receive new name' + ); + + $file4 = $upload('UploadTest-testUpload3.jpg'); + $this->assertEquals( + 'UploadTest-testUpload4.jpg', + $file4->Name, + 'File does receive new name' + ); + + $file1->delete(); + $file2->delete(); + $file3->delete(); + $file4->delete(); + } + } class UploadTest_Validator extends Upload_Validator implements TestOnly {