Created the patch file for 9114 PR instead of using PR in composer.json file

Issue fork drupal-3466462

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mfb created an issue. See original summary.

Chandansha made their first commit to this issue’s fork.

chandansha’s picture

Hello @mfb ,
i did not face same error like you can say. could please verify my step. if i am wrong please guide me to reproduced error.
1. setup project using 3466462-deprecated-function-explode this branch.
2. create file field in basic page and set file extension foobaz, mp3.
3. create node and upload mp3 file.
4. open node in front page. (i can't see error).
5. Again i try another way like create view for basic page and render file field and open that page still i can't see error.

Can you please provide proper steps to reproduced error

Thanks!!

mfb’s picture

Issue summary: View changes
mfb’s picture

@Chandansha Oops I forgot to mention configuring the field formatter in the steps to reproduce, that is probably what you were missing

chandansha’s picture

Status: Active » Needs review

I have fixed deprecated function error.
Please review.
Thanks!!

smustgrave’s picture

Status: Needs review » Needs work

Research needs to be done to figure out why it's null. Putting just a simple check could be masking a larger issue.

mfb’s picture

@smustgrave the premise is configuring an unknown file extension, so I think it's reasonable for the mime type to be null.

And silently ignoring/masking the null seems fine to me, unless folks think it's important enough to log a warning that an extension with unknown mime type was configured.

Left at needs work since I suggested changes on the merge request.

charlliequadros’s picture

Hi @smustgrave

The issue occurs because when an unknown file type is created, it doesn't find the MIME type, resulting in a null return from this function.

https://git.drupalcode.org/project/drupal/-/blob/11.0.0/core/lib/Drupal/Core/File/MimeType/ExtensionMimeTypeGuesser.php#L929

One solution would be to set 'application/octet-stream' as the MIME type for unknown file types. This would prevent the error from occurring. However, I'm not sure if this would be the best approach or if the current implementation in the MR is sufficient, as it already resolves the problem.

smustgrave’s picture

Issue tags: +Needs tests

Possibly will need to have test coverage to handle when unknown file type is sent.

mfb’s picture

Looks like MimeTypeGuesser::guessMimeType() returns 'application/octet-stream' if all of the registered guesser(s) have returned null. But ExtensionMimeTypeGuesser::guessMimeType(), being a specific guesser, returns null.

In this case, the specific guesser is being called directly, which is a bit odd, but doing it this way means a null mime type is possible.

So yes, just handling this possible null seems like the easiest solution.

charlliequadros’s picture

Hi @Chandansha,
I added the check that @mfb requested in the function where the error occurs. I hope you don't mind.

morvaim’s picture

Issue summary: View changes
morvaim’s picture

Issue summary: View changes
mxr576’s picture

Issue summary: View changes
mxr576’s picture

This issue was most likely caused by fixing this other one... rolling back the change introduced by that issue probably would not be a good fix, since it addressed and important mismatch between expected behavior vs. actual implementation.

mfb’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Added this scenario to the existing functional test

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Ran test-only feature https://git.drupalcode.org/issue/drupal-3466462/-/jobs/2398791 and shows the coverage and code change seems straight forward.

Will go ahead and mark.

quietone’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs title update

I have only noticed the title here.

The title should be a description of what is being fixed or improved. The title is used as the git commit message so it should be meaningful an concise. See List of issue fields.

mxr576’s picture

Title: Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\file\Plugin\Field\FieldFormatter\FileMediaFormatterBase::mimeTypeApplies() (line 171 of Plugin/Field/FieldFormatter/FileMediaFormatterBase.php) » Fix handling of unknown file extensions in FileMediaFormatterBase
Issue tags: -Needs title update
mxr576’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @mxr576 for taking care of that so quickly!

blb’s picture

I had this problem with the documents media type when was in the UI for Manager display it had this error and i was prevented from making changes. I mention this here in case anyone encounters the same.

'Deprecated function: explode(): Passing null to parameter #2 ($string) of type string is deprecated in Drupal\file\Plugin\Field\FieldFormatter\FileMediaFormatterBase::mimeTypeApplies() (line 171 of core/modules/file/src/Plugin/Field/FieldFormatter/FileMediaFormatterBase.php)' was repeated many times. (on D 10.3.2)

It seems by default the documents type has a long list of acceptable suffixes:
file_extensions: 'txt rtf doc docx ppt pptx xls xlsx pdf odf odg odp ods odt fodt fods fodp fodg key numbers pages'

replacing with the shorter list: txt, doc, docx, pdf -- solves the problem. maybe this should be looked at.

grimreaper’s picture

StatusFileSize
new5.72 KB

Hi,

Thanks for the work here!

Encountered the same issue and MR fixes the bug on Core 10.3.2

mfb’s picture

Hiding out of date patch

dalemoore’s picture

Just encountered the same error message on a new site when adding a new view mode to the Document media type; paired the allowed list in the field to "txt, rtf, doc, docx, ppt, pptx, xls, xlsx, pdf" and it bypasses for now. Just deleting "numbers" and "pages" from the list didn't fix so must be others not in there.

kim.pepper’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

We shouldn't be calling ExtensionMimeTypeGuesser (service id 'file.mime_type.guesser.extension') directly, but instead using 'file.mime_type.guesser' which is a service collector and returns a default of 'application/octet-stream' if none are found.

kim.pepper’s picture

Status: Needs work » Needs review

Created a MR that does #29

mfb’s picture

@kim.pepper But in this case, we have a "fake" file: 'fakedFile.' . $extension So, we only want to retrieve a MIME type based on the extension, not anything else that file.mime_type.guesser might use. Thus only file.mime_type.guesser.extension is used directly.

kim.pepper’s picture

Status: Needs review » Reviewed & tested by the community

Ah you are right. Back to RTBC for MR!9114

kim.pepper’s picture

peter_serfozo’s picture

StatusFileSize
new1.78 KB
kim.pepper’s picture

We don't need a re-roll @peter_serfozo as we are using the MR !9114

Edit: ah unless you are using composer patches.

quietone’s picture

I read the IS, comments and the MR. There are no unanswered questions and everything is in order here.

Leaving at RTBC

phily’s picture

MR !9114 works for me using Drupal 10.3.6

larowlan’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Bug Smash Initiative

I think we need to also handle a possible NULL value in a second place in the formatter.
We should also consider logging the invalid file extension rather than silently ignoring it.

mfb’s picture

Status: Needs work » Needs review

As I mentioned in #9, silently ignoring the null rather than logging a warning seems fine to me.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Resolved some of the threads, from what I can tell the feedback has been addressed.

quietone’s picture

One remaining question here is #40. I'll leave that to another committer to respond to.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed ec3fedad1d8 to 11.x and f96fff0dad9 to 11.1.x and 1f1970a9bf8 to 10.5.x and 51439626866 to 10.4.x. Thanks!

  • alexpott committed 51439626 on 10.4.x
    Issue #3466462 by mfb, chandansha, kim.pepper, charlliequadros,...

  • alexpott committed 1f1970a9 on 10.5.x
    Issue #3466462 by mfb, chandansha, kim.pepper, charlliequadros,...

  • alexpott committed f96fff0d on 11.1.x
    Issue #3466462 by mfb, chandansha, kim.pepper, charlliequadros,...

  • alexpott committed ec3fedad on 11.x
    Issue #3466462 by mfb, chandansha, kim.pepper, charlliequadros,...
alexpott’s picture

smustgrave’s picture

Status: Reviewed & tested by the community » Fixed

Weird I clearly see it was marked fixed but didn’t stick

Status: Fixed » Closed (fixed)

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

yevko’s picture

Indeed, this is not in 11.1.1

dhavalpanchal’s picture

Issue summary: View changes
StatusFileSize
new8.4 KB

Creating patch file for Drupal 10.4.1 and PHP 8.3.

parth_sinha made their first commit to this issue’s fork.