Problem/Motivation

Config exports fail on systems without the PHP fileinfo extension (see original report, #2). In response to this, a patch was committed that requires the extension (#13, #18). This subsequently caused Drupal 8 installation problems on various shared hosts (#20, #21, #22, others).

However, after closer inspection, it turns out that this requirement is actually a separate bug: #2387443: BinaryFileResponse can fail because the core MIME guessing is not added to the MimeType singleton Therefore, proposed resolution is to revert the originally committed patch and fix the underlying cause in that issue instead.

Original report by @amool

With the latest 8.x-dev version, when I'm trying to export the configuration, the admin/config/development/configuration/full/export-download is crashing with the message:

If you have just changed code (for example deployed a new module or moved an existing one) read http://drupal.org/documentation/rebuild

No, I didn't change the code, neither deployed a new module nor moved an existing module. Its a fresh installation of Drupal 8.x-dev clone.

Watch the steps here:
http://screencast.com/t/jQWpdvn6c

But just because it said, I even tried all the instructions given on d.o/documentation/rebuild. Still its showing the same error.

Files: 
CommentFileSizeAuthor
#43 fileinfo-2290261-43.patch368 byteswebchick
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,053 pass(es). View

Comments

aspilicious’s picture

Priority: Critical » Normal

You need to rebuild drupal each time you fetch a new version.
If that isn't working, drupal probably changed to much and you should reinstall.
(don't forget to remove the files directory)

I'm confident this will fix your issue. Had the same problem today, reinstalled everything.
Moving to normal for now.

amool’s picture

@aspilicious: No, its not fixing it. I'm talking about the fresh (newly cloned) installation of drupal 8.x-dev. Watch this 30 sec screencast.
http://screencast.com/t/jQWpdvn6c

amool’s picture

Issue summary: View changes
tim.plunkett’s picture

Can you go to admin/reports/dblog ("Recent log messages" under the "Reports" section) and see if there are any messages there? It should be stuff about modules being enabled and the install completing, and then hopefully some clue.

If that doesn't have anything, go to admin/config/development/logging ("Logging and errors" under the "Development" section in "Configuration") and setting the error reporting to "All messages, with backtrace information", and then trying the export again.
Either that will give you more info, or it might add extra info to the log messages.

amool’s picture

@tim.plunkett: I've checked "Recent log messages", the logged error is:

LogicException: Unable to guess the mime type as no guessers are available (Did you enable the php_fileinfo extension?) in Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser->guess() (line 123 of H:\xampp\htdocs\d8.x-dev_test\core\vendor\symfony\http-foundation\Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser.php).

So in php.ini I've uncommented the line:
extension=php_fileinfo.dll

and then tried to export the configuration. Hooray!! Error is fixed! thank you :)

tim.plunkett’s picture

Title: Full configuration export crashing » Full configuration export crashing without the php_fileinfo extension
Status: Active » Fixed

Thanks for working through this @amool! I'm glad it works now.

larowlan’s picture

Status: Fixed » Needs work

Wait shouldn't we add a hook_requirements for php_fileinfo then?

tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
FileSize
360 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed. View

Can we automatically test this?
For manual testing, @amool, could you try installing again but with the extension=php_fileinfo.dll commented out again?

Status: Needs review » Needs work

The last submitted patch, 8: php_fileinfo-2290261-8.patch, failed testing.

larowlan’s picture

maybe just fileinfo?

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
463 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch fileinfo-2290261-11.patch. Unable to apply patch. See the log in the details link for more information. View

Status: Needs review » Needs work

The last submitted patch, 11: fileinfo-2290261-11.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
368 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,915 pass(es). View

/me facepalms

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

unless bot disagrees.

tim.plunkett’s picture

Issue tags: -Needs tests

@larowlan points out we can't test this with the current testbot infrastructure

amool’s picture

@tim.plunkett: I've tested your patch. Its not letting me install drupal unless I enable that `fileinfo` extension. After enabling the extension & restarting the server. It is proceeding further:

Check out the same here (50 secs)
http://screencast.com/t/bEOfwAdIkD

tim.plunkett’s picture

Committers, please include @amool in the commit message.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6a8faf5 and pushed to 8.x. Thanks!

  • alexpott committed 6a8faf5 on 8.x
    Issue #2290261 by tim.plunkett, amool: Fixed Full configuration export...
StuartJNCC’s picture

Status: Fixed » Active

I came across this yesterday when doing a fresh install. Right at the start of the installation it came up with a a requirements error with a message to enable the fileinfo extension and wouldn't let me go further. I enabled it and install worked. But, as far as I can see this is not mentioned in the requirements documentation:

Somewhere, a note about fileinfo needs adding, probably in the "Drupal specific version notes on PHP requirements - Drupal 8" section.

webchick’s picture

Also having this on DreamHost. Seems like we should do some research to figure out how common this extension is before we make it a requirement?

devad’s picture

I got reply from my shared hosting provider:

> Sorry, we had never supported "fileinfo" also, cPanel doesn't recommend installing it prior PHP 5.5
> There are few vulnerabilities which were patched in 5.4.33 (your version) but still...

I suppose many others will have problems with fileinfo extension on their shared hostings as well.

If cPanel do not recommend it prior PHP 5.5 - which is not so widely used yet - is it a good way to make it a requirement?

webchick’s picture

Priority: Normal » Critical

I'm not sure if this is actually a release blocker (guess it depends on who the other hosts are), but escalating this to get some eyes on it.

catch’s picture

Title: Full configuration export crashing without the php_fileinfo extension » Decide whether php_fileinfo extension is a reasonable base requirement

Re-titling since I keep looking at this wondering why we don't just add the dependency.

mbrett5062’s picture

According PHP installation manual the fileinfo extension is enabled by default from 5.3.0 onwards.

Quote from manual.

This extension is enabled by default as of PHP 5.3.0. Before this time, fileinfo was a PECL extension but is no longer maintained there. However, versions prior to 5.3+ may use the » discontinued PECL extension.

Windows users must include the bundled php_fileinfo.dll DLL file in php.ini to enable this extension.

This information and link were added to the PHP documentation page by myself.

cilefen’s picture

Issue summary: View changes

Would it be possible to swap out the MimeTypeGuesser if fileinfo is not detected?

catch’s picture

@webchick it looks like fileinfo is supported on Dreamhost if you add the extension to php.ini/phprc yourself, at least according to http://wiki.dreamhost.com/Fileinfo

alexpott’s picture

re #26 yes it would be but then we'd have to maintain a replacement in core.
re #27 the disclaimers on http://wiki.dreamhost.com/Fileinfo are hilarious.

cilefen’s picture

Issue summary: View changes
dawehner’s picture

While discussing the criticality of this issue we saw the following code:

    private function __construct()
    {
        if (FileBinaryMimeTypeGuesser::isSupported()) {
            $this->register(new FileBinaryMimeTypeGuesser());
        }

        if (FileinfoMimeTypeGuesser::isSupported()) {
            $this->register(new FileinfoMimeTypeGuesser());
        }
    }

inside Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser::__construct

Given that we host on mostly unix systems, the question is why the following condition of FileBinaryMimeTypeGuesser::isSupported() fails.

        return !defined('PHP_WINDOWS_VERSION_BUILD') && function_exists('passthru') && function_exists('escapeshellarg');

These functions are built into php, it is not even an extension ... dreamhost seems to seriously do something crazy
like patching PHP itself and remove stuff from it.

We need some more detailed research.

dawehner’s picture

Component: configuration system » base system

.

webchick’s picture

If it was just Dreamhost that'd be one thing, but see also #22. CPanel is installed nearly everywhere. We need more data, probably starting with the hosts mentioned here, particularly GoDaddy: http://trends.builtwith.com/hosting/shared-hosting

And yes, the disclaimers on that page, while hilarious, effectively mean Dreamhost users won't be able to run Drupal 8, because no one in their right mind is going to do that in the face of those warnings. :\ Since DreamHost is apparently running 14% of all the top 100K (3% across all) shared hosting sites on the internet, that seems a pretty big deal.

webchick’s picture

webchick’s picture

Also, adding the issue where this was introduced. Looks like the problem's existed since alpha13.

joshtaylor’s picture

By default fileinfo isn't enabled on stock cPanel, but will install on stock cPanel with PHP 5.4 once fileinfo is enabled.

pwolanin’s picture

Can someone update the summary to give more info about where that symfony class is used or how?

The name alone makes me cringe and think of file-naming security exploits

catch’s picture

@pwolanin see the issue in #34. We kept the same mapping as 7.x so the security considerations should be the same I think.

cilefen’s picture

Issue summary: View changes
Kazanir’s picture

Wrt #30, I had a friend with Dreamhost access check and the 3 conditions for FileBinaryMimeTypeGuesser::isSupported() all pass. So, hopefully this sort of failure isn't totally widespread.

However, patching PHP isn't necessary at all to achieve this effect: PHP allows you to add functions to a disable_functions list in the php.ini file. Those functions presumably fail a "function exists" check, and escapeshellarg is a frequent candidate for this disabling because it is part of code (i.e. shell calls) that shared hosting administrators consider insecure.

It is worth noting that all of those disclaimers seem to be from a legacy era when fileinfo was a PECL extension. If fileinfo has been on-by-default since PHP 5.3, we could probably prevail on Dreamhost (and cPanel) to enable fileinfo by default or provide better guidance about how common it is and get rid of the hilarious red warnings and so forth.

chx’s picture

> Drupal\Core\File\MimeType\MimeTypeGuesser uses the Symfony MimeTypeGuesser class when handling files

As far as I can see, the only service tagged by mime_type_guesser is file.mime_type.guesser.extension. I can't see how Symfony MimeTypeGuesser is used.

Nonetheless, it would be a great security boon to use it but I am not sure whether this is critical; we have released a truckload of Drupal versions based on this weak mime guesser.

chx’s picture

Issue summary: View changes
chx’s picture

Status: Active » Closed (duplicate)
webchick’s picture

Status: Closed (duplicate) » Needs review
FileSize
368 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,053 pass(es). View

Ok, so after a big round of discussion/detective work in IRC (thanks dawehner/chx), turns out #2387443: BinaryFileResponse can fail because the core MIME guessing is not added to the MimeType singleton is the root cause of amool's problems.

Therefore, I'd like to solve this issue by reverting the patch in #13 that added fileinfo as a requirement. It turns out this isn't necessary, since Drupal's mime type guesser doesn't require it, and the reason it's resulting in errors about missing dependencies is because Symfony's got some code that's hard-coding a call to theirs.

This would address both the additional requirements issue leading to install failures on some shared hosts, and also leave a clean slate for #2387443: BinaryFileResponse can fail because the core MIME guessing is not added to the MimeType singleton to handle only the parts required to fix the bug.

Here's a patch.

webchick’s picture

Title: Decide whether php_fileinfo extension is a reasonable base requirement » Revert php_fileinfo requirement
Issue summary: View changes

Updating title and issue summary.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Commit it!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 502c78a and pushed to 8.0.x. Thanks!

  • alexpott committed 502c78a on 8.0.x
    Issue #2290261 by webchick: Revert php_fileinfo requirement
    
chx’s picture

Summarizing what happened in the issue and on IRC: I need to yell at people to get them to listen to me instead of believing whatever Symfony says. I don't want to yell at people. That hurts people and hurts me. I am, again, out. I thought I can work together with people to get criticals fixed, turns out I can't. Too bad. You'll need someone else to do critical hours.

chx’s picture

There are more thoughts swirling in my head , this issue mostly just sparkles writing them down.

  1. One of the fundamentals of Drupal were hooks. Instead of doing a requirements-pros-cons analysis based on what Drupal, Symfony and other frameworks did we have not replaced but added Symfony's event system which in every possible way is the exact opposite of hooks: explicit vs implicit, functions vs method, priorities vs weights. It also turned out that the system didn't scale to Drupal's needs so we have overwritten/extended Symfony's relevant parts. More would be needed.
  2. One of the fundamentals of Drupal is handling incoming and outgoing links/paths/routing. Instead of doing a requirements-pros-cons analysis based on what Drupal, Symfony and other frameworks are doing we have added Symfony CMF's routing system. When it turned out it (immediately) doesn't scale to Drupal's needs we have overwritten/extended Symfony's relevant parts. However, this was not enough and instead of doing more of this we have decided to solve the performance issues by storing and make routes our central API despite neither incoming nor outgoing links are routes.

Now, let's talk of release. There are two routes ahead: PHP 5 and PHP 6. PHP 6 was poised to be a revolution, had books printed then stalled and never was released. PHP 5.0 was a horrible release, PHP 5.0.5 broke things so badly several people didn't touch PHP 5 until GoPHP years and years later made them. PHP 5.2 was a pleasant release.

I do not think the community can fix the mess created in any reasonable time frame. klausi had the right idea: pick a date, I recommend two weeks after DrupalCon Los Angeles and release Drupal 8.0.0. Deadline makes things happen. We know that. We can rally people much better around a big countdown than a critical countdown marker that can not reach zero.

Edit: cross-posted to the release issue.

webchick’s picture

Taking a break sounds like a very good idea if that's how you interpreted this afternoon's events.

I'm happy to run with core critical hours until we can find someone more suitable. I still think it's a great idea for creating momentum around critical issues, especially from people not in the list of top 10 D8 contributors.

devad’s picture

8.0.x installation works on my shared hosting now. Nice to see d8 installed for the first time. :)

Thnx!

chx’s picture

I am not contesting the quality of Symfony / Symfony CMF. In the past perhaps I did, that was stupid. However, that was often a knee jerk reaction to the prevailing and equally stupid "Symfony good, Drupal bad" default viewpoint of some? many? developers.

No, Symfony is certainly good at certain things and Drupal is certainly good at certain things and where Drupal 8 has problems is these two things don't quite match. This bug is a textbook case:

Symfony has a MIME guesser that relies on tried and true tools which give you a very good MIME guess based on the contents of the binary itself. Drupal has a MIME guesser which gives you a poor guess based on the extension of the file. The latter, however is available everywhere. Also, Drupal does not rely on the guess from the MIME guesser for security and as such a weak guesser that is correct in most cases and available everywhere was fine until now. And the best solution is to run Symfony's strong MIME guesser where possible and runs Drupal's when not. Do you see finally what I am saying? Symfony and Drupal both make compromises. Symfony tends to be more correct in theory and works better for people building neat web applications on top. Drupal, on the other hand, tends to be more practical to be workable for an extremely wide range of users. The integration can be great but the road leading there is rocky.

What makes me mad is when the fine tuned, battle tested practical solutions of Drupal are changed to fix bugs in this new amalgamation of Drupal and Symfony. Instead, the default bugfixing method should be to strengthen the integration, to override/extend Symfony with Drupal's solutions. If you don't do this then you move sideways and not forward towards a release as this issue showed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.