Drupal\file\IconMimeTypes doesn't handle NULL mimetypes

I created the Patch for it.

sahana _n → made their first commit to this issue’s fork.

Hi,
I can reproduce the issue in Drupal 10.3.x, when I attempt to view the page with a file reference to a file with NULL as the mimetype I got this error TypeError: Drupal\file\IconMimeTypes::getIconClass(): Argument #1 ($mimeType) must be of type string, null given, called in /var/www/html/core/modules/file/file.module on line 774 in Drupal\file\IconMimeTypes::getIconClass() (line 21 of core/modules/file/src/IconMimeTypes.php).

I tested the same scenario by applying the patch provided in comment #3 (update this comment number). The patch is applied cleanly and the error was cleared. So, I created MR with the same code changes.

I am attaching before & after screenshots for reference.

This issue is also reproducible in 11.x and was able to fix the issue with the same code changes.

I'll be happy to create MR for 11.x also if required. Please suggest how we can proceed further.

Fixes should be against 11.x

Also we should probably do some debugging vs putting a null check. Could be masking a larger issue

Hi,
Created an MR for 11. x.

Despite the db column allowing NULL. our API assumes there will always be a mime type. Why do you have NULL mimetypes?

@kim.pepper: Are you asking me why we had NULL mimetypes in the database? I don't know. Could be from a less-than-perfect migration, from when we moved from D7. They were generally pretty old records. Did the the API always assume there was a mimetype? If not, why would the db allow it to be NULL? At any rate, the fact that it does allow NULLs should mean that the system can handle them.

Or the DB setting is wrong.

\Drupal\Core\File\MimeType\MimeTypeGuesser::guessMimeType() defaults to 'application/octet-stream' if it can't determine a mime type. Would it make sense to default to this rather than empty string?

Well, it's better than getting an error, right?

Thanks for the patch.
In our case it happens if we send a webform graphql mutation with a pdf attached.

kim.pepper → changed the visibility of the branch 3469280-File-mime-type-null to hidden.

kim.pepper → changed the visibility of the branch 3469280-drupalfileiconmimetypes-doesnt-handle to hidden.

Added a default mime type constant of \Drupal\Core\File\MimeType\MimeTypeGuesser::DEFAULT_MIME_TYPE with value 'application/octet-stream' and use that instead of empty string.

Hiding some older patches and MRs.

Updated IS to reflect latest proposed resolution.

Updated title.

Seems pretty straight forward, not sure test coverage is needed here.

We have other code that assumes getMimeType() returns a string...

Consider

    $mimetype = $file->getMimeType();
    $mimetype = explode('/', $mimetype);

in

\Drupal\media\Plugin\media\Source\File::getThumbnail

This will also cause a deprection.

I think we should change \Drupal\file\Entity\File::getMimeType() to return the default value - or it could even do something like File::preCreate()... and guess the mimetype if we have a URI

  /**
   * {@inheritdoc}
   */
  public function getMimeType() {
    $mimetype = $this->get('filemime')->value;
    if ($mimetype === NULL) {
      $uri = $this->getFileUri();
      $mimetype = $uri !== NULL ? \Drupal::service('file.mime_type.guesser')->guess($uri) : MimeTypeGuesser::DEFAULT_MIME_TYPE;
    }
    return $mimetype;
  }

We should also update the interface to tell people not to return a NULL

I think this would be a good change because then all code can rely on getMimeType() returning something sensible rather than having to deal with the NULL situation (which must be very rare given the preCreate().

Re: #23 I updated \Drupal\file\Entity\File::getMimeType() to return the default mime type if NULL, and also the docs on \Drupal\file\FileInterface::getMimeType() to match this behaviour.

Feedback appears to be addressed.

One minor change, fine to self RTBC

Self-RTBTC

We can clean up a bit more here \Drupal\file\Plugin\Field\FieldFormatter\FileMediaFormatterBase::getSourceFiles has code that checks for NULL, which is no longer possible.

In searching for callers of this method, the number of places we're treating the return as a string is an excellent case to make this change.