From ebeb663ddf9cd254eb076333b492d5a989f13135 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian.mooyman@gmail.com>
Date: Thu, 20 Feb 2014 14:03:40 +1300
Subject: [PATCH] BUG Fixed critical issue with Folder::find_or_make failing to
 handle invalid filename characters BUG Fix UploadField duplicate checking
 with invalid folderName

---
 filesystem/Folder.php           | 23 +++++++++++++----------
 forms/UploadField.php           |  8 ++++++--
 tests/filesystem/FolderTest.php | 17 +++++++++++++++++
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/filesystem/Folder.php b/filesystem/Folder.php
index 5bfcd0422..3af480f51 100644
--- a/filesystem/Folder.php
+++ b/filesystem/Folder.php
@@ -57,20 +57,23 @@ class Folder extends File {
 
 		$parentID = 0;
 		$item = null;
+		$filter = FileNameFilter::create();
 		foreach($parts as $part) {
 			if(!$part) continue; // happens for paths with a trailing slash
-			$item = DataObject::get_one(
-				"Folder", 
-				sprintf(
-					"\"Name\" = '%s' AND \"ParentID\" = %d",
-					Convert::raw2sql($part), 
-					(int)$parentID
-				)
-			);
+			
+			// Ensure search includes folders with illegal characters removed, but
+			// err in favour of matching existing folders if $folderPath
+			// includes illegal characters itself.
+			$partSafe = $filter->filter($part);
+			$item = Folder::get()->filter(array(
+				'ParentID' => $parentID,
+				'Name' => array($partSafe, $part)
+			))->first();
+			
 			if(!$item) {
 				$item = new Folder();
 				$item->ParentID = $parentID;
-				$item->Name = $part;
+				$item->Name = $partSafe;
 				$item->Title = $part;
 				$item->write();
 			}
@@ -460,7 +463,7 @@ class Folder extends File {
 	 * Get the children of this folder that are also folders.
 	 */
 	public function ChildFolders() {
-		return DataObject::get("Folder", "\"ParentID\" = " . (int)$this->ID);
+		return Folder::get()->filter('ParentID', $this->ID);
 	}
 	
 	/**
diff --git a/forms/UploadField.php b/forms/UploadField.php
index a740a362c..a09dc496a 100644
--- a/forms/UploadField.php
+++ b/forms/UploadField.php
@@ -1273,11 +1273,15 @@ class UploadField extends FileField {
 		$nameFilter = FileNameFilter::create();
 		$filteredFile = basename($nameFilter->filter($originalFile));
 		
+		// Resolve expected folder name
+		$folderName = $this->getFolderName();
+		$folder = Folder::find_or_make($folderName);
+		$parentPath = BASE_PATH."/".$folder->getFilename();
+		
 		// check if either file exists
-		$folder = $this->getFolderName();
 		$exists = false;
 		foreach(array($originalFile, $filteredFile) as $file) {
-			if(file_exists(ASSETS_PATH."/$folder/$file")) {
+			if(file_exists($parentPath.$file)) {
 				$exists = true;
 				break;
 			}
diff --git a/tests/filesystem/FolderTest.php b/tests/filesystem/FolderTest.php
index 229f8a048..d37d69b2f 100644
--- a/tests/filesystem/FolderTest.php
+++ b/tests/filesystem/FolderTest.php
@@ -338,4 +338,21 @@ class FolderTest extends SapphireTest {
 		))->count());
 	}
 	
+	public function testIllegalFilenames() {
+		
+		// Test that generating a filename with invalid characters generates a correctly named folder.
+		$folder = Folder::find_or_make('/FolderTest/EN_US Lang');
+		$this->assertEquals(ASSETS_DIR.'/FolderTest/EN-US-Lang/', $folder->getRelativePath());
+		
+		// Test repeatitions of folder
+		$folder2 = Folder::find_or_make('/FolderTest/EN_US Lang');
+		$this->assertEquals($folder->ID, $folder2->ID);
+		
+		$folder3 = Folder::find_or_make('/FolderTest/EN--US_L!ang');
+		$this->assertEquals($folder->ID, $folder3->ID);
+		
+		$folder4 = Folder::find_or_make('/FolderTest/EN-US-Lang');
+		$this->assertEquals($folder->ID, $folder4->ID);
+	}
+	
 }