InteractsWithMedia::addMediaFromUrl still creates media and attaches it to respective model upon NotReadableException

[Anticom]

Problem statement:
When trying to add media from url and it fails with a Intervention\Image\Exception\NotReadableException, the media record is still created and attached to the respective model.

Desired behavior:
IMHO the correct behavior would be to not create the media record at all when the item can not be processed due to invalid mime type etc. I thought about wrapping it in a try-catch block and delete the media in the catch block again. However in our case this would lead to drastic increase of the auto-increment that are totally unnecessary and hence may lead to issues down the line.

Additional information:
Specific call:

$model
    ->addMediaFromUrl('url.that/has/application/x-empty/image/type')
    ->setFileName('file-name.jpg')
    ->setName('name')
    ->setOrder(1)
    ->withCustomProperties([ /* ... */])
    ->toMediaCollection('my-specific-collection');

Stack trace (excerpt):

Intervention\Image\Exception\NotReadableException: Unsupported image type application/x-empty. GD driver is only able to decode JPG, PNG, GIF, BMP or WebP files.
#81 /vendor/intervention/image/src/Intervention/Image/Gd/Decoder.php(75): Intervention\Image\Gd\Decoder::initFromPath
#80 /vendor/intervention/image/src/Intervention/Image/AbstractDecoder.php(344): Intervention\Image\AbstractDecoder::init
#79 /vendor/intervention/image/src/Intervention/Image/AbstractDriver.php(66): Intervention\Image\AbstractDriver::init
#78 /vendor/intervention/image/src/Intervention/Image/ImageManager.php(54): Intervention\Image\ImageManager::make
#77 /vendor/league/glide/src/Api/Api.php(97): League\Glide\Api\Api::run
#76 /vendor/league/glide/src/Server.php(652): League\Glide\Server::makeImage
#75 /vendor/spatie/image/src/GlideConversion.php(74): Spatie\Image\GlideConversion::performManipulations
#74 /vendor/spatie/image/src/Image.php(120): Spatie\Image\Image::save
#73 /vendor/spatie/laravel-medialibrary/src/Conversions/Actions/PerformManipulationsAction.php(33): Spatie\MediaLibrary\Conversions\Actions\PerformManipulationsAction::execute
#72 /vendor/spatie/laravel-medialibrary/src/Conversions/Actions/PerformConversionAction.php(30): Spatie\MediaLibrary\Conversions\Actions\PerformConversionAction::execute
#71 /vendor/spatie/laravel-medialibrary/src/Conversions/FileManipulator.php(71): Spatie\MediaLibrary\Conversions\FileManipulator::Spatie\MediaLibrary\Conversions\{closure}
#70 /vendor/laravel/framework/src/Illuminate/Collections/Traits/EnumeratesValues.php(236): Illuminate\Support\Collection::each
#69 /vendor/spatie/laravel-medialibrary/src/Conversions/FileManipulator.php(72): Spatie\MediaLibrary\Conversions\FileManipulator::performConversions
#68 /vendor/spatie/laravel-medialibrary/src/Conversions/FileManipulator.php(39): Spatie\MediaLibrary\Conversions\FileManipulator::createDerivedFiles
#67 /vendor/spatie/laravel-medialibrary/src/MediaCollections/Filesystem.php(37): Spatie\MediaLibrary\MediaCollections\Filesystem::add
#66 /vendor/spatie/laravel-medialibrary/src/MediaCollections/FileAdder.php(446): Spatie\MediaLibrary\MediaCollections\FileAdder::processMediaItem
#65 /vendor/spatie/laravel-medialibrary/src/MediaCollections/FileAdder.php(428): Spatie\MediaLibrary\MediaCollections\FileAdder::attachMedia
#64 /vendor/spatie/laravel-medialibrary/src/MediaCollections/FileAdder.php(344): Spatie\MediaLibrary\MediaCollections\FileAdder::toMediaCollection
#63 location at my code performing the call mentioned above
#...

Versions:

$ php --version
PHP 8.1.13 (cli) (built: Dec  9 2022 16:07:55) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.13, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.13, Copyright (c), by Zend Technologies

$ composer --version
Composer version 2.5.1 2022-12-22 15:33:54

$ artisan --version
Laravel Framework 9.42.2

$ composer show spatie/laravel-medialibrary
name     : spatie/laravel-medialibrary
descrip. : Associate files with Eloquent models
keywords : cms, conversion, downloads, images, laravel, laravel-medialibrary, media, spatie
versions : * 10.7.3
type     : library
license  : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText
homepage : https://github.com/spatie/laravel-medialibrary
source   : [git] https://github.com/spatie/laravel-medialibrary.git d42ef44607527aa9196b3e9eaf691c22ea5e4ff0
dist     : [zip] https://api.github.com/repos/spatie/laravel-medialibrary/zipball/d42ef44607527aa9196b3e9eaf691c22ea5e4ff0 d42ef44607527aa9196b3e9eaf691c22ea5e4ff0
path     : /Users/anticom/src/cfm/ferien-touristik/rms/vendor/spatie/laravel-medialibrary
names    : spatie/laravel-medialibrary

support
issues : https://github.com/spatie/laravel-medialibrary/issues
source : https://github.com/spatie/laravel-medialibrary/tree/10.7.3

autoload
psr-4
Spatie\MediaLibrary\ => src

requires
ext-exif *
ext-fileinfo *
ext-json *
illuminate/bus ^9.18
illuminate/console ^9.18
illuminate/database ^9.18
illuminate/pipeline ^9.18
illuminate/support ^9.18
intervention/image ^2.7
maennchen/zipstream-php ^2.0
php ^8.0
spatie/image ^2.2.2
spatie/temporary-directory ^2.0
symfony/console ^6.0

requires (dev)
aws/aws-sdk-php ^3.133.11
doctrine/dbal ^2.13
ext-imagick *
ext-pdo_sqlite *
ext-zip *
guzzlehttp/guzzle ^7.4
league/flysystem-aws-s3-v3 ^3.0
mockery/mockery ^1.4
nunomaduro/larastan ^2.0
orchestra/testbench 7.*
pestphp/pest ^1.21
phpstan/extension-installer ^1.1
spatie/laravel-ray ^1.28
spatie/pdf-to-image ^2.1
spatie/phpunit-snapshot-assertions ^4.2

suggests
league/flysystem-aws-s3-v3 Required to use AWS S3 file storage
php-ffmpeg/php-ffmpeg Required for generating video thumbnails
spatie/pdf-to-image Required for generating thumbsnails of PDFs and SVGs

conflicts
php-ffmpeg/php-ffmpeg <0.6.1

[Anticom]

Looking into the issue a little more, the issue actually isn't the addMediaFromUrl but generating conversions.
I am generating that conversions non queued, however this can't be guaranteed for everyone and if it fails in an async process there's probably no sensible way to roll back.

[Anticom]

After contemplating over a potential solution I just discovered the $allowedMimeTypes param for addMediaFromUrl. I took the list of supported mime types from Imaginary's GD Decoder and used those as valid mime types and used those as allowed mime types:

'image/png',
'image/x-png',
'image/jpg',
'image/jpeg',
'image/pjpeg',
'image/gif',
'image/webp',
'image/x-webp',
'image/bmp',
'image/ms-bmp',
'image/x-bitmap',
'image/x-bmp',
'image/x-ms-bmp',
'image/x-win-bitmap',
'image/x-windows-bmp',
'image/x-xbitmap',

This seems to solve the issue for me, since the image isn't attached anymore when the provided image has application/x-empty mime type.

---

That being said I would vastly appreciate a couple of constants providing common media formats that this library is able to coup with. Common categories that come to mind are:

• Images
• Videos
• PDFs
• Documents

IMO it would be sufficient to have public consts somewhere in the lib, for example:

class ValidMimeTypes
{
    // list taken from https://github.com/Intervention/image/blob/54934ae8ea3661fd189437df90fb09ec3b679c74/src/Intervention/Image/Gd/Decoder.php#L30-L65
    public const IMAGE = [
        'image/png',
        'image/x-png',
        'image/jpg',
        'image/jpeg',
        'image/pjpeg',
        'image/gif',
        'image/webp',
        'image/x-webp',
        'image/bmp',
        'image/ms-bmp',
        'image/x-bitmap',
        'image/x-bmp',
        'image/x-ms-bmp',
        'image/x-win-bitmap',
        'image/x-windows-bmp',
        'image/x-xbitmap',
    ];

    // ...
}

That way, we could pass those into addMediaFromUrl easily. Also composition, modification, etc. would be easily achievable (especially using the spread operator).