Register Drupal's mime type guesser the Symfony MimeTypes service

Retesting against 9.3.x. The "new" Symfony 5 mime type guesser was added in #3055193: [Symfony 5] The "Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser" class is deprecated since Symfony 4.3, use "Symfony\Component\Mime\MimeTypes" instead.. This causes difficult-to-diagnose issues such as #3216534: BinaryFileResponse can fail to provide the appropriate content type which I just marked as a duplicate of this issue.

Being bold and marking this major as it results in empty responses for binary file responses.

I came here basically with the same patch, and this is rather straightforward. Nice that it still passes tests against 9.3.x.

Only nit would be, does this need some sort of @todo to remove when Drupal 10 drops Symfony 4?

Marking RTBC to get some maintainer eyes.

Is it worth also taking the test from the closed duplicate issue as insurance against breaking this again? Testing that binary files have the correct content type seems a reasonable thing to do, and implies we don't have any other end to end coverage that uses this feature.

AFAICT this is still a bug today … I just wrote failing test coverage for the CDN module at #3300982-4: Drupal core regression: CDN's "far future" responses have wrong Content-Type response header.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

A lot of the singleton logic has gone from Drupal 10. There's new test coverage here though, so maybe we want to commit (some of?) that anyway? Sent the test-only patch for a re-test against 10.1.x

I just discovered this after some hard waiting for reproduction scenarios.

Downloaded a specific CSS file and it seems that the fallback to file_info picks it up as assembler source text - delivering text/x-asm! Of course, this only happens for an uncached hit to an aggregation - subsequent will pick it up. So it just is annoying. I'll raise as a separate issue on AssetControllerBase but this is related I believe.

css_po4KdqlRZWF8CfE0QBHXRKXwhj22MhbNT8WiGLDtx9w.css: assembler source text, ASCII text, with very long lines (24366)

There's already an issue for that bug here: #3371358: When AssetControllerBase delivers existing file should add content-type.

In #3197482: Update Drupal 10 to depend on Symfony 5.4 (as a stepping stone to Symfony 6, for deprecation checking support) we removed MimeTypeGuesser::registerWithSymfonyGuesser() entirely, was this a mistake?

If it is still needed this needs rerolling considering the registration method was removed entirely. I guess the additional test coverage should tell us if this is necessary.

I believe this is outdated. Was trying to do some archaeology on my side to determine if this would still be necessary but I think the answer is no. Comports with my note in #9 that this would no longer be necessary in D10.

Given #22 and #26 I rerolled the tests to see what the testbot has to say about this old issue.

Given the tests results
https://git.drupalcode.org/issue/drupal-3172550/-/jobs/1533523#L2239

It seems that this issue is not that outdated!

Ah, thank you! I couldn't for the life of me remember _why_ I was here in the first place but this helped jog my memory.

I think per #26 then, we can consider that #3197482: Update Drupal 10 to depend on Symfony 5.4 (as a stepping stone to Symfony 6, for deprecation checking support) resulted in a regression and this should be high priority for 11.0...

OK, this should be good for review, now. I do believe this is a pretty significant regression on binary file handling.

Needed to update a config validation test that was depending on the default Symfony mimetype guesser's fileinfo implementation to pass.

Can the issue summary be filled in. I put a TBD for sections that may apply but if they didn't can change to N/A

Thanks for updating! Looking again

In Drupal 9 we should be registering with both and then in Drupal 10 we should remove the legacy registration.

Believe this no longer applies?

IS updated.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Updated.

Meant to post in my previous comment but test coverage exists and can be seen here https://git.drupalcode.org/issue/drupal-3172550/-/jobs/1631442

Believe all feedback has been addressed and change does appear to make sense with the register.

There's a @todo linking to this issue in AssetControllerBase - I've removed the one line of code plus the comment, and confirmed the explicit test coverage we added for it still pass for me locally. Will go ahead and commit once gitlab comes back green. Nice to get rid of that.

Committed/pushed to 11.x and cherry-picked to 11.0.x, 10.4.x, 10.3.x, thanks!