From 3781ecc784b9255ff164f87547559e009c152c8b Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Tue, 29 Jun 2010 04:59:45 +0000
Subject: [PATCH] MINOR Made File::get_file_extension() more readable, and
 added unit test

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@107267 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 filesystem/File.php           | 14 +++++++++++---
 tests/filesystem/FileTest.php | 22 ++++++++++++++++++----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/filesystem/File.php b/filesystem/File.php
index 192e68926..335022f51 100755
--- a/filesystem/File.php
+++ b/filesystem/File.php
@@ -598,12 +598,19 @@ class File extends DataObject {
 	/**
 	 * Gets the extension of a filepath or filename,
 	 * by stripping away everything before the last "dot".
+	 * Caution: Only returns the last extension in "double-barrelled"
+	 * extensions (e.g. "gz" for "tar.gz").
+	 * 
+	 * Examples:
+	 * - "myfile" returns ""
+	 * - "myfile.txt" returns "txt"
+	 * - "myfile.tar.gz" returns "gz"
 	 *
 	 * @param string $filename
 	 * @return string
 	 */
 	public static function get_file_extension($filename) {
-		return strtolower(substr($filename,strrpos($filename,'.')+1));
+		return pathinfo($filename, PATHINFO_EXTENSION);
 	}
 	
 	/**
@@ -737,8 +744,9 @@ class File extends DataObject {
 	
 	function validate() {
 		if(File::$apply_restrictions_to_admin || !Permission::check('ADMIN')) {
-			$extension = strtolower(pathinfo($this->Name, PATHINFO_EXTENSION));
-			
+			// Extension validation
+			// TODO Merge this with Upload_Validator
+			$extension = $this->getExtension();
 			if($extension && !in_array($extension, self::$allowed_extensions)) {
 				$exts =  self::$allowed_extensions;
 				sort($exts);
diff --git a/tests/filesystem/FileTest.php b/tests/filesystem/FileTest.php
index c2d234886..fc32bd72d 100644
--- a/tests/filesystem/FileTest.php
+++ b/tests/filesystem/FileTest.php
@@ -4,21 +4,35 @@
  * Tests for the File class
  */
 class FileTest extends SapphireTest {
+	
 	static $fixture_file = 'sapphire/tests/filesystem/FileTest.yml';
 	
+	function testGetExtension() {
+		$this->assertEquals('', File::get_file_extension('myfile'), 'No extension');
+		$this->assertEquals('txt', File::get_file_extension('myfile.txt'), 'Simple extension');
+		$this->assertEquals('gz', File::get_file_extension('myfile.tar.gz'), 'Double-barrelled extension only returns last bit');
+	}
+	
 	function testValidateExtension() {
-		$file = $this->objFromFixture('File', 'asdf');
-
-		// Invalid
+		Session::set('loggedInAs', null);
+		
+		$origExts = File::$allowed_extensions;
+		File::$allowed_extensions = array('txt');
+		
+		$file = $this->objFromFixture('File', 'asdf'); 
+	
+		// Invalid ext
 		$file->Name = 'asdf.php';
 		$v = $file->validate();
 		$this->assertFalse($v->valid());
 		$this->assertContains('Extension is not allowed', $v->message());
 		
-		// Valid
+		// Valid ext
 		$file->Name = 'asdf.txt';
 		$v = $file->validate();
 		$this->assertTrue($v->valid());
+		
+		File::$allowed_extensions = $origExts;
 	}
 	
 	function testLinkAndRelativeLink() {