From de290cd02db7150549da5cc66c9af3de6933a68b Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 13 Nov 2014 22:47:06 -0800
Subject: [PATCH] Improve MIME detection in FileBackend

The content type detector will now inspect the file contents
to better handle extensionless files.

Also dependency inject the callback and make the default one
use FileInfo.

Change-Id: Iad59bf6c6a416b706f976a4c425763fd30e2debb
---
 includes/filebackend/FileBackendGroup.php     | 24 +++++++++++++
 includes/filebackend/FileBackendStore.php     | 20 +++++++----
 .../includes/filebackend/FileBackendTest.php  | 36 +++++++++++++++++++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/includes/filebackend/FileBackendGroup.php b/includes/filebackend/FileBackendGroup.php
index b6ddbad256..c043106acf 100644
--- a/includes/filebackend/FileBackendGroup.php
+++ b/includes/filebackend/FileBackendGroup.php
@@ -166,6 +166,7 @@ class FileBackendGroup {
 				? FileJournal::factory( $config['fileJournal'], $name )
 				: FileJournal::factory( array( 'class' => 'NullFileJournal' ), $name );
 			$config['wanCache'] = ObjectCache::getMainWANInstance();
+			$config['mimeCallback'] = array( $this, 'guessMimeInternal' );
 
 			$this->backends[$name]['instance'] = new $class( $config );
 		}
@@ -203,4 +204,27 @@ class FileBackendGroup {
 
 		return null;
 	}
+
+	/**
+	 * @param string $storagePath
+	 * @param string|null $content
+	 * @param string|null $fsPath
+	 * @return string
+	 * @since 1.27
+	 */
+	public function guessMimeInternal( $storagePath, $content, $fsPath ) {
+		$magic = MimeMagic::singleton();
+		// Trust the extension of the storage path (caller must validate)
+		$ext = FileBackend::extensionFromPath( $storagePath );
+		$type = $magic->guessTypesForExtension( $ext );
+		// For files without a valid extension (or one at all), inspect the contents
+		if ( !$type && $fsPath ) {
+			$type = $magic->guessMimeType( $fsPath, false );
+		} elseif ( !$type && strlen( $content ) ) {
+			$tmpFile = TempFSFile::factory( 'mime_' );
+			file_put_contents( $tmpFile->getPath(), $content );
+			$type = $magic->guessMimeType( $tmpFile->getPath(), false );
+		}
+		return $type ?: 'unknown/unknown';
+	}
 }
diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index e5ce968a97..8113ec213b 100644
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -58,7 +58,7 @@ abstract class FileBackendStore extends FileBackend {
 	/**
 	 * @see FileBackend::__construct()
 	 * Additional $config params include:
-	 *   - wanCache     : WANOBjectCache object to use for persistent caching.
+	 *   - wanCache     : WANObjectCache object to use for persistent caching.
 	 *   - mimeCallback : Callback that takes (storage path, content, file system path) and
 	 *                    returns the MIME type of the file or 'unknown/unknown'. The file
 	 *                    system path parameter should be used if the content one is null.
@@ -69,10 +69,7 @@ abstract class FileBackendStore extends FileBackend {
 		parent::__construct( $config );
 		$this->mimeCallback = isset( $config['mimeCallback'] )
 			? $config['mimeCallback']
-			: function ( $storagePath, $content, $fsPath ) {
-				// @todo handle the case of extension-less files using the contents
-				return StreamFile::contentTypeFromPath( $storagePath ) ?: 'unknown/unknown';
-			};
+			: null;
 		$this->memCache = WANObjectCache::newEmpty(); // disabled by default
 		$this->cheapCache = new ProcessCacheLRU( self::CACHE_CHEAP_SIZE );
 		$this->expensiveCache = new ProcessCacheLRU( self::CACHE_EXPENSIVE_SIZE );
@@ -1823,7 +1820,18 @@ abstract class FileBackendStore extends FileBackend {
 	 * @return string MIME type
 	 */
 	protected function getContentType( $storagePath, $content, $fsPath ) {
-		return call_user_func_array( $this->mimeCallback, func_get_args() );
+		if ( $this->mimeCallback ) {
+			return call_user_func_array( $this->mimeCallback, func_get_args() );
+		}
+
+		$mime = null;
+		if ( $fsPath !== null && function_exists( 'finfo_file' ) ) {
+			$finfo = finfo_open( FILEINFO_MIME_TYPE );
+			$mime = finfo_file( $finfo, $fsPath );
+			finfo_close( $finfo );
+		}
+
+		return is_string( $mime ) ? $mime : 'unknown/unknown';
 	}
 }
 
diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php
index 0d15b75bfb..09b3728813 100644
--- a/tests/phpunit/includes/filebackend/FileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/FileBackendTest.php
@@ -2397,6 +2397,42 @@ class FileBackendTest extends MediaWikiTestCase {
 			"Scoped unlocking of files succeeded with OK status ($backendName)." );
 	}
 
+	/**
+	 * @dataProvider provider_testGetContentType
+	 */
+	public function testGetContentType( $mimeCallback, $mimeFromString ) {
+		global $IP;
+
+		$be = TestingAccessWrapper::newFromObject( new MemoryFileBackend(
+			array(
+				'name' => 'testing',
+				'class' => 'MemoryFileBackend',
+				'wikiId' => 'meow',
+				'mimeCallback' => $mimeCallback
+			)
+		) );
+
+		$dst = 'mwstore://testing/container/path/to/file_no_ext';
+		$src = "$IP/tests/phpunit/data/media/srgb.jpg";
+		$this->assertEquals( 'image/jpeg', $be->getContentType( $dst, null, $src ) );
+		$this->assertEquals(
+			$mimeFromString ? 'image/jpeg' : 'unknown/unknown',
+			$be->getContentType( $dst, file_get_contents( $src ), null ) );
+
+		$src = "$IP/tests/phpunit/data/media/Png-native-test.png";
+		$this->assertEquals( 'image/png', $be->getContentType( $dst, null, $src ) );
+		$this->assertEquals(
+			$mimeFromString ? 'image/png' : 'unknown/unknown',
+			$be->getContentType( $dst, file_get_contents( $src ), null ) );
+	}
+
+	public static function provider_testGetContentType() {
+		return array(
+			array( null, false ),
+			array( array( FileBackendGroup::singleton(), 'guessMimeInternal' ), true )
+		);
+	}
+
 	public function testReadAffinity() {
 		$be = TestingAccessWrapper::newFromObject(
 			new FileBackendMultiWrite( array(
-- 
2.20.1