Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test to prove mime-type parsing of UploadFile #3735

Conversation

jaap
Copy link

@jaap jaap commented Dec 5, 2024

While writing a test using an Illuminate\Http\UploadedFile with a set MIME type, I encountered an issue where the MIME type does not propagate correctly through the Media Library package.

Although it's not directly within the scope of the package to handle Illuminate\Http\UploadedFile, I believe it could be useful. As @freekmurze mentioned in #3469, he is open to a PR to support this use case.

Current Behavior:

In my test (Feature/FileAdder/MediaConversions/MediaCollectionTest.php), I demonstrate the current behavior, which I believe is incorrect. However, I'm uncertain whether this should be addressed in Laravel Medialibrary. The issue arises because when using UploadedFile::fake() for testing in Laravel, the generated file is given a random temporary name (e.g., phpzcNPGn). Note that this name on disk does not contain the extension, even if you provide one.

$uploadFile = UploadedFile::fake()
  ->create('song.mp3', 1000)
  ->mimeType('audio/mpeg');

Results in something like /private/var/folders/yb/1b4wqmkj1wx_hzx_jwb38_hc0000gn/T/phpzcNPGn on disk.

The package responsible for determining the MIME type is Symfony’s MimeTypes.php, which might (I did not check that) rely on the file extension when it cannot determine the MIME type from the file content. At the point where the MIME type is assigned in MediaCollections/FileAdder.php, only the file path is available:

$media->mime_type = File::getMimeType($this->pathToFile);

Given this, it’s impossible to know whether the file is a fake upload, making it challenging to adjust the MIME type resolution.

Workaround:

The current workaround is to avoid using UploadedFile::fake() and instead use an actual stub file (similar to the approach used in other tests in this package).

Suggestion:

It may be best to document this behavior rather than modifying the package. However, if a PR is considered, it might involve additional checks to detect fake uploads and adjust MIME type resolution accordingly.

TL;DR

  • Added a test to demonstrate the issue with MIME type propagation when using Illuminate\Http\UploadedFile in tests.
  • Open for discussion on whether this is something the Media Library package should address directly.

Update

A workaround using a bit of both works...
By using an actual mp3 and providing that as the content for the UploadedFile::fake() works and propagates the correct MIME type. Will comment on the discussion as well.

$testAudioFilePath = base_path('tests/data/test.mp3');
$uploadFile = UploadedFile::fake()->createWithContent('test.mp3', file_get_contents($testAudioFilePath));

@jaap jaap closed this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant