3
\$\begingroup\$

This function is used to get the mime type of any given file. The intended purpose was to have it working within any of our servers that vary from the latest PHP version down to PHP 4.

/**
 * Get the mime type for any given file.
 *
 * @param str $filename Path to the file including the filename
 *
 * @return mixed bol|arr Either false or the array provided by the PHP function
 */
function getMimeType($filename) {
  $mimetype = false;

  if (function_exists('finfo_fopen')) {
    $mimetype = finfo_fopen($filename);
  } elseif (function_exists('getimagesize')) {
    $mimetype = getimagesize($filename);
  } elseif (function_exists('exif_imagetype')) {
    $mimetype = exif_imagetype($filename);
  } elseif (function_exists('mime_content_type')) {
    $mimetype = mime_content_type($filename);
  }

  return $mimetype;
}

While the function is already tested on different PHP versions, I'm trying to ascertain if the methodology implemented can be improved, thus leading to code reduction and preventing any redundant verifications.

Can this function receive any type of improvement?

\$\endgroup\$
2
  • \$\begingroup\$ In my opinion, this code is broken and should be revised before being reviewed. In particular: @return mixed bol|arr Either false or the array provided by the PHP function That's a major flaw. How is the consumer of this function to know what to expect? Also, the documentation is a lie. exif_imagetype returns an integer, not an array. Similarly, finfo_fopen returns a resource, and you're calling it incorrectly. It's basically impossible to use this function unless you repeat the if-elseif tree in the consuming code to know how to interpret the return value. \$\endgroup\$
    – Corbin
    Sep 21, 2012 at 4:54
  • \$\begingroup\$ This is an old function that was on the "storage", while in production on several projects, you're right, there are several issues to be solved before it can be properly reviewed. Thank you for your time, I'll update my answer as soon as I deal with this. \$\endgroup\$
    – Zuul
    Sep 21, 2012 at 13:17

1 Answer 1

2
\$\begingroup\$

General Issues

If you are going to call something getMimeType then it should return exactly that. This is probably best defined by Multipurpose Internet Mail Extensions RFC 2046. Read the wikipedia page for more general information and related RFCs.

As Corbin commented on this implementation is broken due to the number of assumptions that are required to consume the information provided by this function. You really should return just a string with the mime type or an array with the primary type and subtype. Whatever you return should be consistent with your documentation.

You could consider some of the following instead of returning false:

  • Throw an exception.
  • Trigger an error and default the MIME type.
  • Create a second function hasMimeType to avoid exception/errors.

Specific Issues

finfo_fopen

I think finfo_fopen should probably be finfo_open.

Its usage is something like:

$finfo = finfo_open(FILEINFO_MIME_TYPE);
return finfo_file($finfo, $filename);

However there are some catches. FILEINFO_MIME_TYPE has only been defined since PHP 5.3.0. You can use version_compare to determine which version of php you are using (If the version_compare function doesn't exist you are using PHP < 4.1.0).

if (version_compare(PHP_VERSION, '5.3.0') >= 0) { /* Do Stuff */ }

getimagesize

Only the mime index from the returned array should be used.

exif_imagetype

The string returned from this does not conform to the MIME type format. You will have to massage this data to return something appropriate.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the analysis, I'll be updating my question taking into consideration your notes. The function is old and isn't properly structured. I'll have to deal with it and run some tests before returning to this topic. \$\endgroup\$
    – Zuul
    Sep 21, 2012 at 13:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.