Issues

ZF-11784: Zend_Validate_File_MimeType does not permit user to disable the auto discovery of magic files

Description

Since version 5.3.x (poorly documented) PHP has included its own libmagic library, including custom patches. It presumably also includes a built-in mime file definition (not documented anywhere, but observed anecdotally by myself and another: http://php.net/manual/en/…).

A problem occurs when PHP is in use on systems that use a version of libmagic and the associated mime magic files that conflicts with what PHP is using internally. When using finfo_open() and explicitly specifying the system magic file to use then most (all?) files will be identified as "application/octet-stream" instead of the proper type. However, when using finfo_open() and omitting the second parameter PHP is able to successfully identify file mime types as expected.

Note that despite the finfo_open() documentation regarding the second parameter: "If this variable is not set either, /usr/share/misc/magic is used by default", is incorrect. That, or PHP is behaving differently when implicitly using the default magic file than when that same file is explicitly specified. This can be demonstrated and tested using the simple script located here: https://gist.github.com/1252125

This occurs in CentOS 5.6 and 5.7 with IUS PHP packages, and potentially other environments.

Sorry for the amount of background info. The problem that Zend_Validate_File_MimeType has is that it does not permit the developer to specify that we don't want it to attempt to use the list of "common" locations for mime magic files. Nor does it permit us to modify or override them in any fashion.

Zend_Validate_File_MimeType::getMagicFile() currently will always attempt to utilize the $_magicFiles list if the magic file has not been explicitly set. ZVFMT::setMagicFile() does not permit passing "false" or "null" to indicate that we do not desire those files be used.

I'm thinking a ZVFMT::disableAutoMagicFileUsage() method that simply sets a boolean property (default true) is the simplest and best solution.

Comments

Fixed in r24486

Your change is not correct and incomplete.

The same issue was solved several weeks ago with GH-365. See ZF-10461.

I would ask you to search for identical issues which have been solved before adding new behaviour. Additionally, when adding new behaviour and methods, you should document them and add unittests for the new behaviour.

Please remove your change and back-port the change from ZF2 when you want to fix this issue. This would be better than adding new behaviour which is not needed in ZF2.

{quote} Your change is not correct and incomplete. {quote}

Would you care to elaborate? The change does exactly what it is intended to do, and is complete.

{quote} The same issue was solved several weeks ago with GH-365. See ZF-10461. {quote}

Great! I considered a similar approach in fixing this. I did not see this issue since I did not check the ZF2 repository for a fix to a bug that existed in ZF1.

{quote} I would ask you to search for identical issues which have been solved before adding new behavior. {quote}

That's a completely reasonable expectation to have. I did give an honest effort in searching for an existing bug, but since both ZF-10461 and the related ZF-9635 are classified as part of the Zend_Validate component I did not see them when I was filtering on the more fine-grained Zend_Validate_File component.

{quote} Additionally, when adding new behaviour and methods, you should document them and add unittests for the new behavior. {quote}

I did add unit tests for the newly added behavior. I also provided what I consider very thorough inline documentation. Are you referring to user documentation? I apologize for not doing that; I typically don't make changes to the documentation, nor do I see any existing documentation for Zend_Validate_File_MimeType.

{quote} Please remove your change and back-port the change from ZF2 when you want to fix this issue. This would be better than adding new behaviour which is not needed in ZF2. {quote}

I don't mean to be difficult here, but would you care to elaborate why your change in the ZF2 branch is better suited to address this issue than mine? The problems I see with your solution are as follows:

  • No additions to or modification of unit tests
  • No documentation or explanation provided for the change
  • The $file param to Zend_Validate_File_MimeType::setMagicFile() now causes different behavior depending on whether it is the boolean value false, a non-empty string, an empty string, or a NULL value -- This is not intuitive ** Nor does the doc block properly reflect the interface
  • The behavior of $validate->setMagicFile(false) is ambiguous and not intuitive

According ZF2 repository: When you search for issues you dont have to look into repositories. You will see if there is a fix for an issue within the issue tracker. ZF-10461 and ZF-9635 can be found by simply searching for "MimeType". That you found them yourself shows me that they are still part of the issue tracker.

According documentation: As this validators are subcomponents of Zend_File all doucmentation for this classes can be found in the related chapter of Zend_File. But you are correct that the special "false" behaviour of fileinfo is not documented in the online manual which describes ZF1.

According behaviour: That the existing and expected behaviour (false was described and intended to work since ZF 1.9 and the auto-search feature was added by the dev-team about 1.8) is not "nice" or intuitive is one thing. But an existing solution should not be the reason to add additional methods to solve already solved problems.

Personally I find


$valid = new Zend_Validator_File_MimeType(array('magicfile' => false);

more intuitive and shorter than


$valid = new Zend_Validator_File_MimeType();
$valid->setTryCommonMagicFileFlag(false);

because it can be used within Zend_Form using the array syntax without adding seperated lines of code.