BinaryFileResponse can fail because the core MIME guessing is not added to the MimeType singleton

Let's disagree with the solution.

Let's get a bit more confident about the solution. ;)

Also if it's literally a matter of adding one line to a method somewhere, let's call this a Novice task and see what happens. :)

adding the tasks information, so people can see what needs to be done, and instructions on how to do it, which is important for helping people get involved. The dreditor insert tasks button is useful for this.

I think we need some steps to reproduce also. adding needs issue summary update for that.

was this discovered while working on another issue?

--

Can we add tests for this?
I'm going to guess yes.
If not, please update the issue summary to say why not.

Why not just throw sane exception if fileinfo is unavailable for an export exactly?

PS: Suppose each component/service can depends on php internals but how to properly expose this deps...

All other binary responses will fail. So that's anything downloaded through the FileDownloadController::download method (eg private files), first time image style derivatives and locale gettext downloads.

Also we could need this if we use Symfony's file or image validators.

@andypost this issue is not about only the fileinfo extension...

    /**
     * Registers all natively provided mime type guessers
     */
    private function __construct()
    {
        if (FileBinaryMimeTypeGuesser::isSupported()) {
            $this->register(new FileBinaryMimeTypeGuesser());
        }

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

From \Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser - if this code does not register a guesser we have problems.

1. +++ b/core/lib/Drupal/Core/DrupalKernel.php
   @@ -458,6 +459,9 @@ public function preHandle(Request $request) {
   +    // Override of Symfony's mime type guesser singleton.
   +    MimeTypeGuesser::overrideSymfonyGuessers();
   

   Is there a reason why we don't pass along the container? We should have it available here

2. +++ b/core/lib/Drupal/Core/File/MimeType/MimeTypeGuesser.php
   @@ -88,4 +89,18 @@ protected function sortGuessers() {
   +   * A helper function to override Symfony's default guessers.
   

   Let's describe it a bit better, we register an additional one.

3. +++ b/core/lib/Drupal/Core/DrupalKernel.php
   @@ -458,6 +459,9 @@ public function preHandle(Request $request) {
   +    MimeTypeGuesser::overrideSymfonyGuessers();
   
   +++ b/core/lib/Drupal/Core/File/MimeType/MimeTypeGuesser.php
   @@ -88,4 +89,18 @@ protected function sortGuessers() {
   +    $singleton->register(\Drupal::service('file.mime_type.guesser'));
   

   Afaik we have the container available and so can just pass it along

4. +++ b/core/tests/Drupal/Tests/Core/DrupalKernel/MimeTypeGuesserOverrideTest.php
   @@ -0,0 +1,55 @@
   + * Contains \Drupal\Tests\Core\DrupalKernel\MimeTypeGuesserOverrideTest.
   

   I would vote for renaming the test file, to make life easier.

5. +++ b/core/tests/Drupal/Tests/Core/DrupalKernel/MimeTypeGuesserOverrideTest.php
   @@ -0,0 +1,55 @@
   +  public function testSymfonyGuesserOverrider() {
   +    // Make the guessers property accessible on Symfony's MimeTypeGuesser.
   +    $symfony_guesser = SymfonyMimeTypeGuesser::getInstance();
   +    $reflection = new \ReflectionClass($symfony_guesser);
   +    $property = $reflection->getProperty('guessers');
   ...
   +    MimeTypeGuesser::overrideSymfonyGuessers();
   +    $guessers = $property->getValue($symfony_guesser);
   +    $this->assertInstanceOf('Drupal\Core\File\MimeType\MimeTypeGuesser', $guessers[0]);
   

   Learn about $this->readAttribute , luke :)

re #12 all sounds sensible. Move the test to Drupal\Tests\Core\File as well.

Alright, thank you!

I think we need some steps to reproduce also. adding needs issue summary update for that.

I think this is kinda pointless, given that this would be incredible hard to setup. (find a bad hoster ...)

@chx I think we could attempt that in a separate issue. We registering Drupal\Core\File\MimeType\MimeTypeGuesser which can have additional mime type guessers added during a compiler pass. Created #2388749: Register symfony's mime guessers if they are supported to do this.

Thanks for opening the follow-up. That looks worth looking at, but happy with this for the straightforward fix.

Committed/pushed to 8.0.x, thanks!