[Bug] uploading a font file with 'upload' field throws an Exception at getExtensionFromFile()

Saad5400 commented 1 year ago

Bug report

What I did

Create a crud controller

Add the following field to the create operation

    CRUD::addField([
        'label' => 'رفع الخط',
        'name' => 'font_url',
        'type' => 'upload',
        'wrapper' => [
            'class' => 'upload-wrapper',
        ],
        'withFiles' => [
            'disk' => 'public',
            'path' => 'uploads/fonts',
        ],
    ]);

Upload a font file of type ttf and click save

What I expected to happen

The file to be uploaded

What happened

Error message: Backpack\CRUD\app\Library\Uploaders\Support\FileNameGenerator::getExtensionFromFile(): Return value must be of type string, null returned

What I've already tried to fix it

Nothing, but it works with images and other types of files

Backpack, Laravel, PHP, DB version

php artisan backpack:version

backpack/basset: 1.2.0
backpack/crud: 6.2.0
backpack/generators: v4.0.2
backpack/pro: 2.0.11
backpack/theme-tabler: 1.1.0

php artisan about

Environment ............................................  
Application Name ............................... Laravel
Laravel Version ................................ 10.23.1
PHP Version ...................................... 8.2.6
Composer Version ................................. 2.5.7
Environment ...................................... local
Debug Mode ..................................... ENABLED
URL ..................................... 127.0.0.1:8000
Maintenance Mode ................................... OFF

Cache ..................................................
Config ...................................... NOT CACHED
Events ...................................... NOT CACHED
Routes ...................................... NOT CACHED
Views ........................................... CACHED

Drivers ................................................
Broadcasting ....................................... log
Cache ............................................. file  
Database ......................................... mysql
Logs .................................... stack / single
Mail .............................................. smtp
Queue ............................................. sync
Session ........................................... file

Operating System and Server Setup

Personal Windows 10, using php artisan serve

karandatwani92 commented 1 year ago

Hey @Saad5400 Thanks for reporting the issue.

I tested it to confirm, and it's strange to see this happening only with font files.

pxpm commented 1 year ago

Hey @Saad5400 thanks for the report.

Sorry it took me a while to get back here.

I've spent some time (waaaaaay more than I would expect) researching this issue, and I still don't have a final solution for it.

Unfortunately is not something we can fix in Backpack. I have a solution for you at the end of my research results, keep reading.

The underlying problem here is that the file.ttf may have different file types depending on their content. In one of the files I was testing it returned font/sfnt as the mime type. https://en.wikipedia.org/wiki/SFNT#cite_note-iana-1

That by itself is not enough to determine the the file extension, as you can see from the linked wikipedia page the font/sfnt can have multiple formats too.

Laravel under the hood uses Symfony mime component to determine the mime types, and they don't provide any mapping to font/sfnt format. https://github.com/symfony/symfony/blob/751deddbdb89b16f694f98cbc59deef1caa4c822/src/Symfony/Component/Mime/MimeTypes.php#L142

I don't have knowledge to tell for sure if they didn't add it because the final extension is somewhat difficult/impossible to determine or was just an oversight and we can submit a PR to add it.

Of course using getClientOriginalExtension() is completely out of question here as it would be a major security implication.

As a workaround for your particular scenario, Backpack allow you to take control of the file generator used by uploaders. Please follow the guide here: https://backpackforlaravel.com/docs/6.x/crud-uploaders#naming-files-when-using-uploaders-1

I don't think we are going to push this issue forward, we have a solution for developers to take control of the name generation, and we support what our base frameworks (Laravel -> Symfony) support, I think for now we are good here.

Let me know if you think differently or I missed the point here.

Cheers

Saad5400 commented 10 months ago

Hey thank you for your reply @pxpm , I just used woff after I opened this issue, but now I am forced to ttf. I followed the link and made my own File Name Generator class, but I'm facing an issue as I'm unable to validate the uploaded file type. Using ValidUpload::field('required')->file('file|mimes:ttf') doesn't work. Any suggestion to how I might validate the file type without creating a custom Uploader?

Saad5400 commented 10 months ago

Regarding my last comment, I found https://laracasts.com/discuss/channels/laravel/mimesttf-does-not-working-in-validation and fixed the issue, thanks once again.

Laravel-Backpack / community-forum