Follow-up to r71944: Interoducing MimeMagic::improveTypeFromExtension() for two reasons:
authorDaniel Kinzler <daniel@users.mediawiki.org>
Tue, 31 Aug 2010 13:47:24 +0000 (13:47 +0000)
committerDaniel Kinzler <daniel@users.mediawiki.org>
Tue, 31 Aug 2010 13:47:24 +0000 (13:47 +0000)
a) avoid redundant inspection of file contents when validating uploads, caused by multiple calls to guessMimeType
b) deprecated obscure use of the file extension when guessing mime types, using an explicit call to improveTypeFromExtension() instead

Note that File::getPropsFromPath() will now return an additional field: $props['file-mime'] contains the mime type as determined solely from the file's content, $props['mime'] contains the type that was derived considering the file extension too.

includes/MimeMagic.php
includes/filerepo/File.php
includes/upload/UploadBase.php

index 813db38..da9d1ab 100644 (file)
@@ -409,18 +409,70 @@ class MimeMagic {
                return in_array( strtolower( $extension ), $types );
        }
 
+       /** improves a mime type using the file extension. Some file formats are very generic,
+       * so their mime type is not very meaningful. A more useful mime type can be derived 
+       * by looking at the file extension. Typically, this method would be called on the 
+       * result of guessMimeType().
+       * 
+       * Currently, this method does the following:
+       *
+       * If $mime is "unknown/unknown" and isRecognizableExtension( $ext ) returns false,
+       * return the result of guessTypesForExtension($ext). 
+       *
+       * If $mime is "application/x-opc+zip" and isMatchingExtension( $ext, $mime )
+       * gives true, return the result of guessTypesForExtension($ext). 
+       *
+       * @param $mime String: the mime type, typically guessed from a file's content.
+       * @param $ext String: the file extension, as taken from the file name
+       *
+       * @return string the mime type
+       */
+       function improveTypeFromExtension( $mime, $ext ) {
+               if ( $mime === "unknown/unknown" ) {
+                       if( $this->isRecognizableExtension( $ext ) ) {
+                               wfDebug( __METHOD__. ": refusing to guess mime type for .$ext file, we should have recognized it\n" );
+                       } else {
+                               /* Not something we can detect, so simply 
+                               * trust the file extension */
+                               $mime = $this->guessTypesForExtension( $ext );
+                       }
+               }
+               else if ( $mime === "application/x-opc+zip" ) {
+                       if ( $this->isMatchingExtension( $ext, $mime ) ) {
+                               /* A known file extension for an OPC file,
+                               * find the proper mime type for that file extension */
+                               $mime = $this->guessTypesForExtension( $ext );
+                       } else {
+                               wfDebug( __METHOD__. ": refusing to guess better type for $mime file, .$ext is not a known OPC extension.\n" );
+                               $mime = "application/zip";
+                       }
+               }
+
+               if ( isset( $this->mMimeTypeAliases[$mime] ) ) {
+                       $mime = $this->mMimeTypeAliases[$mime];
+               }
+
+               wfDebug(__METHOD__.": improved mime type for .$ext: $mime\n");
+               return $mime;
+       }
 
        /** mime type detection. This uses detectMimeType to detect the mime type of the file,
        * but applies additional checks to determine some well known file formats that may be missed
-       * or misinterpreter by the default mime detection (namely xml based formats like XHTML or SVG).
+       * or misinterpreter by the default mime detection (namely XML based formats like XHTML or SVG,
+       * as well as ZIP based formats like OPC/ODF files).
        *
        * @param $file String: the file to check
-       * @param $ext Mixed: the file extension, or true to extract it from the filename.
-       *             Set it to false to ignore the extension.
+       * @param $ext Mixed: the file extension, or true (default) to extract it from the filename.
+       *             Set it to false to ignore the extension. DEPRECATED! Set to false, use 
+       *             improveTypeFromExtension($mime, $ext) later to improve mime type.
        *
        * @return string the mime type of $file
        */
        function guessMimeType( $file, $ext = true ) {
+               if( $ext ) { # TODO: make $ext default to false. Or better, remove it.
+                       wfDebug( __METHOD__.": WARNING: use of the \$ext parameter is deprecated. Use improveTypeFromExtension(\$mime, \$ext) instead.\n" );
+               }
+
                $mime = $this->doGuessMimeType( $file, $ext );
 
                if( !$mime ) {
@@ -432,11 +484,11 @@ class MimeMagic {
                        $mime = $this->mMimeTypeAliases[$mime];
                }
 
-               wfDebug(__METHOD__.": final mime type of $file: $mime\n");
+               wfDebug(__METHOD__.": guessed mime type of $file: $mime\n");
                return $mime;
        }
 
-       function doGuessMimeType( $file, $ext = true ) {
+       private function doGuessMimeType( $file, $ext ) { # TODO: remove $ext param
                // Read a chunk of the file
                wfSuppressWarnings();
                $f = fopen( $file, "rt" );
@@ -447,6 +499,8 @@ class MimeMagic {
                $tail = fread( $f, 65558 ); // 65558 = maximum size of a zip EOCDR
                fclose( $f );
 
+               wfDebug( __METHOD__ . ": analyzing head and tail of $file for magic numbers.\n" );
+
                // Hardcode a few magic number checks...
                $headers = array(
                        // Multimedia...
@@ -602,11 +656,16 @@ class MimeMagic {
         * @param $header String: some reasonably-sized chunk of file header
         * @param $tail   String: the tail of the file
         * @param $ext Mixed: the file extension, or true to extract it from the filename.
-        *             Set it to false to ignore the extension.
+        *             Set it to false (default) to ignore the extension. DEPRECATED! Set to false, 
+        *             use improveTypeFromExtension($mime, $ext) later to improve mime type.
         *
         * @return string
         */
        function detectZipType( $header, $tail = null, $ext = false ) {
+               if( $ext ) { # TODO: remove $ext param
+                       wfDebug( __METHOD__.": WARNING: use of the \$ext parameter is deprecated. Use improveTypeFromExtension(\$mime, \$ext) instead.\n" );
+               }
+
                $mime = 'application/zip';
                $opendocTypes = array(
                        'chart-template',
@@ -637,7 +696,8 @@ class MimeMagic {
                        wfDebug( __METHOD__.": detected $mime from ZIP archive\n" );
                } elseif( preg_match( $openxmlRegex, substr( $header, 30 ) ) ) {
                        $mime = "application/x-opc+zip";
-                       if( $ext !== true && $ext !== false ) {
+                       # TODO: remove the block below, as soon as improveTypeFromExtension is used everywhere 
+                       if( $ext !== true && $ext !== false ) { 
                                /** This is the mode used by getPropsFromPath
                                * These mime's are stored in the database, where we don't really want
                                * x-opc+zip, because we use it only for internal purposes
@@ -695,15 +755,20 @@ class MimeMagic {
        * If no mime type can be determined, this function returns "unknown/unknown".
        *
        * @param $file String: the file to check
-       * @param $ext Mixed: the file extension, or true to extract it from the filename.
-       *             Set it to false to ignore the extension.
+       * @param $ext Mixed: the file extension, or true (default) to extract it from the filename.
+       *             Set it to false to ignore the extension. DEPRECATED! Set to false, use 
+       *             improveTypeFromExtension($mime, $ext) later to improve mime type.
        *
        * @return string the mime type of $file
        * @access private
        */
-       function detectMimeType( $file, $ext = true ) {
+       private function detectMimeType( $file, $ext = true ) {
                global $wgMimeDetectorCommand;
 
+               if( $ext ) { # TODO:  make $ext default to false. Or better, remove it.
+                       wfDebug( __METHOD__.": WARNING: use of the \$ext parameter is deprecated. Use improveTypeFromExtension(\$mime, \$ext) instead.\n" );
+               }
+
                $m = null;
                if ( $wgMimeDetectorCommand ) {
                        $fn = wfEscapeShellArg( $file );
index a3b62f8..2e3fcef 100644 (file)
@@ -1186,7 +1186,14 @@ abstract class File {
                if ( $info['fileExists'] ) {
                        $magic = MimeMagic::singleton();
 
-                       $info['mime'] = $magic->guessMimeType( $path, $ext );
+                       if ( $ext === true ) {
+                               $i = strrpos( $path, '.' );
+                               $ext = strtolower( $i ? substr( $path, $i + 1 ) : '' );
+                       }
+
+                       $info['file-mime'] = $magic->guessMimeType( $path, false ); # mime type according to file contents
+                       $info['mime'] = $magic->improveTypeFromExtension( $info['file-mime'], $ext ); # logical mime type
+
                        list( $info['major_mime'], $info['minor_mime'] ) = self::splitMime( $info['mime'] );
                        $info['media_type'] = $magic->getMediaType( $path, $info['mime'] );
 
index aa47e10..ccd43ce 100644 (file)
@@ -302,7 +302,7 @@ abstract class UploadBase {
         * @param $mime string representing the mime
         * @return mixed true if the file is verified, an array otherwise
         */
-       protected function verifyMimeType( $magic, $mime ) {
+       protected function verifyMimeType( $mime ) {
                global $wgVerifyMimeType;
                if ( $wgVerifyMimeType ) {
                        wfDebug ( "\n\nmime: <$mime> extension: <{$this->mFinalExtension}>\n\n");
@@ -319,6 +319,8 @@ abstract class UploadBase {
                        $fp = fopen( $this->mTempPath, 'rb' );
                        $chunk = fread( $fp, 256 );
                        fclose( $fp );
+
+                       $magic = MimeMagic::singleton();
                        $extMime = $magic->guessTypesForExtension( $this->mFinalExtension );
                        $ieTypes = $magic->getIEMimeTypes( $this->mTempPath, $chunk, $extMime );
                        foreach ( $ieTypes as $ieType ) {
@@ -344,12 +346,9 @@ abstract class UploadBase {
                $this->mFileProps = File::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
                $this->checkMacBinary();
 
-               # magically determine mime type
-               $magic = MimeMagic::singleton();
-               $mime = $magic->guessMimeType( $this->mTempPath, false );
-
                # check mime type, if desired
-               $status = $this->verifyMimeType( $magic, $mime );
+               $mime = $this->mFileProps[ 'file-mime' ];
+               $status = $this->verifyMimeType( $mime );
                if ( $status !== true ) {
                        return $status;
                }