Merge "Improve MIME detection in FileBackend"

diff --combined includes/filebackend/FileBackendStore.php
index 4ec81ec,8113ec2..1432bb9
--- 1/includes/filebackend/FileBackendStore.php
--- 2/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@@ -58,7 -58,7 +58,7 @@@ abstract class FileBackendStore extend
        /**
         * @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 +69,7 @@@
                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 );
@@@ -1066,7 -1063,7 +1063,7 @@@
                $status = Status::newGood();
  
                // Fix up custom header name/value pairs...
 -              $ops = array_map( array( $this, 'stripInvalidHeadersFromOp' ), $ops );
 +              $ops = array_map( array( $this, 'sanitizeOpHeaders' ), $ops );
  
                // Build up a list of FileOps...
                $performOps = $this->getOperationsInternal( $ops );
@@@ -1133,7 -1130,7 +1130,7 @@@
                $status = Status::newGood();
  
                // Fix up custom header name/value pairs...
 -              $ops = array_map( array( $this, 'stripInvalidHeadersFromOp' ), $ops );
 +              $ops = array_map( array( $this, 'sanitizeOpHeaders' ), $ops );
  
                // Clear any file cache entries
                $this->clearCache();
@@@ -1230,9 -1227,7 +1227,9 @@@
        }
  
        /**
 -       * Strip long HTTP headers from a file operation.
 +       * Normalize and filter HTTP headers from a file operation
 +       *
 +       * This normalizes and strips long HTTP headers from a file operation.
         * Most headers are just numbers, but some are allowed to be long.
         * This function is useful for cleaning up headers and avoiding backend
         * specific errors, especially in the middle of batch file operations.
@@@ -1240,21 -1235,18 +1237,21 @@@
         * @param array $op Same format as doOperation()
         * @return array
         */
 -      protected function stripInvalidHeadersFromOp( array $op ) {
 -              static $longs = array( 'Content-Disposition' );
 +      protected function sanitizeOpHeaders( array $op ) {
 +              static $longs = array( 'content-disposition' );
 +
                if ( isset( $op['headers'] ) ) { // op sets HTTP headers
 +                      $newHeaders = array();
                        foreach ( $op['headers'] as $name => $value ) {
 +                              $name = strtolower( $name );
                                $maxHVLen = in_array( $name, $longs ) ? INF : 255;
                                if ( strlen( $name ) > 255 || strlen( $value ) > $maxHVLen ) {
                                        trigger_error( "Header '$name: $value' is too long." );
 -                                      unset( $op['headers'][$name] );
 -                              } elseif ( !strlen( $value ) ) {
 -                                      $op['headers'][$name] = ''; // null/false => ""
 +                              } else {
 +                                      $newHeaders[$name] = strlen( $value ) ? $value : ''; // null/false => ""
                                }
                        }
 +                      $op['headers'] = $newHeaders;
                }
  
                return $op;
@@@ -1828,7 -1820,18 +1825,18 @@@
         * @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 --combined tests/phpunit/includes/filebackend/FileBackendTest.php
index e7d092f,09b3728..95c6092
--- 1/tests/phpunit/includes/filebackend/FileBackendTest.php
--- 2/tests/phpunit/includes/filebackend/FileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/FileBackendTest.php
@@@ -2395,6 -2395,42 +2395,42 @@@ class FileBackendTest extends MediaWiki
                        "Scoped unlocking of files succeeded ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
                        "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() {
@@@ -2495,37 -2531,6 +2531,37 @@@
                );
        }
  
 +      public function testSanitizeOpHeaders() {
 +              $be = TestingAccessWrapper::newFromObject( new MemoryFileBackend( array(
 +                      'name' => 'localtesting',
 +                      'wikiId' => wfWikiID()
 +              ) ) );
 +
 +              $name = wfRandomString( 300 );
 +
 +              $input = array(
 +                      'headers' => array(
 +                              'content-Disposition' => FileBackend::makeContentDisposition( 'inline', $name ),
 +                              'Content-dUration' => 25.6,
 +                              'X-LONG-VALUE' => str_pad( '0', 300 ),
 +                              'CONTENT-LENGTH' => 855055,
 +                      )
 +              );
 +              $expected = array(
 +                      'headers' => array(
 +                              'content-disposition' => FileBackend::makeContentDisposition( 'inline', $name ),
 +                              'content-duration' => 25.6,
 +                              'content-length' => 855055
 +                      )
 +              );
 +
 +              MediaWiki\suppressWarnings();
 +              $actual = $be->sanitizeOpHeaders( $input );
 +              MediaWiki\restoreWarnings();
 +
 +              $this->assertEquals( $expected, $actual, "Header sanitized properly" );
 +      }
 +
        // helper function
        private function listToArray( $iter ) {
                return is_array( $iter ) ? $iter : iterator_to_array( $iter );