From e3350543af11666973651d0eb70176ddcfb8283d Mon Sep 17 00:00:00 2001 From: IanM <16573496+imorland@users.noreply.github.com> Date: Fri, 19 Jan 2024 10:49:00 +0000 Subject: [PATCH] feat: upgrade `intervention/image` to 3.2 (#3947) * chore: create standalone imageprovider * chore: upgrade intervention to v3 * Apply fixes from StyleCI * use new static instatiation * Revert "Apply fixes from StyleCI" This reverts commit 096b4d9a79fa41c948a7572cf65316ebc6b07d36. * get avatar from remote * Apply fixes from StyleCI * fix: incorrect gid exception namespace * fix test * remove debug code --------- Co-authored-by: StyleCI Bot --- composer.json | 2 +- framework/core/composer.json | 2 +- .../Controller/UploadFaviconController.php | 11 ++-- .../Api/Controller/UploadImageController.php | 4 +- .../Api/Controller/UploadLogoController.php | 10 ++-- .../Filesystem/FilesystemServiceProvider.php | 27 ---------- .../core/src/Foundation/InstalledSite.php | 2 + .../core/src/Image/ImageServiceProvider.php | 53 +++++++++++++++++++ framework/core/src/User/AvatarUploader.php | 19 ++++--- framework/core/src/User/AvatarValidator.php | 7 +-- .../src/User/Command/RegisterUserHandler.php | 26 ++++++++- .../src/User/Command/UploadAvatarHandler.php | 2 +- .../core/src/User/Event/AvatarSaving.php | 4 +- .../tests/unit/User/AvatarUploaderTest.php | 4 +- 14 files changed, 113 insertions(+), 60 deletions(-) create mode 100644 framework/core/src/Image/ImageServiceProvider.php diff --git a/composer.json b/composer.json index 287d746d7..cf0f02d48 100644 --- a/composer.json +++ b/composer.json @@ -130,7 +130,7 @@ "illuminate/support": "^10.0", "illuminate/validation": "^10.0", "illuminate/view": "^10.0", - "intervention/image": "^2.7.2", + "intervention/image": "^3.2", "jenssegers/agent": "^2.6", "laminas/laminas-diactoros": "^3.0", "laminas/laminas-httphandlerrunner": "^2.6", diff --git a/framework/core/composer.json b/framework/core/composer.json index 3ffaf15b9..0bfd6b3b4 100644 --- a/framework/core/composer.json +++ b/framework/core/composer.json @@ -59,7 +59,7 @@ "illuminate/support": "^10.0", "illuminate/validation": "^10.0", "illuminate/view": "^10.0", - "intervention/image": "^2.7.2", + "intervention/image": "^3.2", "jenssegers/agent": "^2.6.4", "laminas/laminas-diactoros": "^3.0", "laminas/laminas-httphandlerrunner": "^2.6.1", diff --git a/framework/core/src/Api/Controller/UploadFaviconController.php b/framework/core/src/Api/Controller/UploadFaviconController.php index 8b2e71210..c251a64c0 100644 --- a/framework/core/src/Api/Controller/UploadFaviconController.php +++ b/framework/core/src/Api/Controller/UploadFaviconController.php @@ -13,8 +13,8 @@ use Flarum\Foundation\ValidationException; use Flarum\Locale\TranslatorInterface; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Contracts\Filesystem\Factory; -use Intervention\Image\Image; use Intervention\Image\ImageManager; +use Intervention\Image\Interfaces\EncodedImageInterface; use Psr\Http\Message\UploadedFileInterface; class UploadFaviconController extends UploadImageController @@ -31,7 +31,7 @@ class UploadFaviconController extends UploadImageController parent::__construct($settings, $filesystemFactory); } - protected function makeImage(UploadedFileInterface $file): Image + protected function makeImage(UploadedFileInterface $file): EncodedImageInterface { $this->fileExtension = pathinfo($file->getClientFilename(), PATHINFO_EXTENSION); @@ -45,10 +45,9 @@ class UploadFaviconController extends UploadImageController ]); } - $encodedImage = $this->imageManager->make($file->getStream()->getMetadata('uri'))->resize(64, 64, function ($constraint) { - $constraint->aspectRatio(); - $constraint->upsize(); - })->encode('png'); + $encodedImage = $this->imageManager->read($file->getStream()->getMetadata('uri')) + ->scale(64, 64) + ->toPng(); $this->fileExtension = 'png'; diff --git a/framework/core/src/Api/Controller/UploadImageController.php b/framework/core/src/Api/Controller/UploadImageController.php index 3e9e2e7d4..cb1a82268 100644 --- a/framework/core/src/Api/Controller/UploadImageController.php +++ b/framework/core/src/Api/Controller/UploadImageController.php @@ -15,7 +15,7 @@ use Illuminate\Contracts\Filesystem\Factory; use Illuminate\Contracts\Filesystem\Filesystem; use Illuminate\Support\Arr; use Illuminate\Support\Str; -use Intervention\Image\Image; +use Intervention\Image\Interfaces\EncodedImageInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UploadedFileInterface; use Tobscure\JsonApi\Document; @@ -55,5 +55,5 @@ abstract class UploadImageController extends ShowForumController return parent::data($request, $document); } - abstract protected function makeImage(UploadedFileInterface $file): Image; + abstract protected function makeImage(UploadedFileInterface $file): EncodedImageInterface; } diff --git a/framework/core/src/Api/Controller/UploadLogoController.php b/framework/core/src/Api/Controller/UploadLogoController.php index c461ec460..1ca056bc6 100644 --- a/framework/core/src/Api/Controller/UploadLogoController.php +++ b/framework/core/src/Api/Controller/UploadLogoController.php @@ -11,8 +11,8 @@ namespace Flarum\Api\Controller; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Contracts\Filesystem\Factory; -use Intervention\Image\Image; use Intervention\Image\ImageManager; +use Intervention\Image\Interfaces\EncodedImageInterface; use Psr\Http\Message\UploadedFileInterface; class UploadLogoController extends UploadImageController @@ -28,11 +28,11 @@ class UploadLogoController extends UploadImageController parent::__construct($settings, $filesystemFactory); } - protected function makeImage(UploadedFileInterface $file): Image + protected function makeImage(UploadedFileInterface $file): EncodedImageInterface { - $encodedImage = $this->imageManager->make($file->getStream()->getMetadata('uri'))->heighten(60, function ($constraint) { - $constraint->upsize(); - })->encode('png'); + $encodedImage = $this->imageManager->read($file->getStream()->getMetadata('uri')) + ->scale(height: 60) + ->toPng(); return $encodedImage; } diff --git a/framework/core/src/Filesystem/FilesystemServiceProvider.php b/framework/core/src/Filesystem/FilesystemServiceProvider.php index dd7bd9116..9fe6db3d6 100644 --- a/framework/core/src/Filesystem/FilesystemServiceProvider.php +++ b/framework/core/src/Filesystem/FilesystemServiceProvider.php @@ -10,20 +10,14 @@ namespace Flarum\Filesystem; use Flarum\Foundation\AbstractServiceProvider; -use Flarum\Foundation\Config; use Flarum\Foundation\Paths; use Flarum\Http\UrlGenerator; use Illuminate\Contracts\Container\Container; use Illuminate\Filesystem\Filesystem; -use Illuminate\Support\Arr; -use Intervention\Image\ImageManager; use League\Flysystem\Visibility; -use RuntimeException; class FilesystemServiceProvider extends AbstractServiceProvider { - protected const INTERVENTION_DRIVERS = ['gd' => 'gd', 'imagick' => 'imagick']; - public function register(): void { $this->container->singleton('files', function () { @@ -65,26 +59,5 @@ class FilesystemServiceProvider extends AbstractServiceProvider $container->make('flarum.filesystem.resolved_drivers') ); }); - - $this->container->singleton(ImageManager::class, function (Container $container) { - /** @var Config $config */ - $config = $this->container->make(Config::class); - - $intervention = $config->offsetGet('intervention'); - $driver = Arr::get($intervention, 'driver', self::INTERVENTION_DRIVERS['gd']); - - // Check that the imagick library is actually available, else default back to gd. - if ($driver === self::INTERVENTION_DRIVERS['imagick'] && ! extension_loaded(self::INTERVENTION_DRIVERS['imagick'])) { - $driver = self::INTERVENTION_DRIVERS['gd']; - } - - if (! Arr::has(self::INTERVENTION_DRIVERS, $driver)) { - throw new RuntimeException("intervention/image: $driver is not valid"); - } - - return new ImageManager([ - 'driver' => $driver - ]); - }); } } diff --git a/framework/core/src/Foundation/InstalledSite.php b/framework/core/src/Foundation/InstalledSite.php index b4b41e823..a22a09fcd 100644 --- a/framework/core/src/Foundation/InstalledSite.php +++ b/framework/core/src/Foundation/InstalledSite.php @@ -23,6 +23,7 @@ use Flarum\Forum\ForumServiceProvider; use Flarum\Frontend\FrontendServiceProvider; use Flarum\Group\GroupServiceProvider; use Flarum\Http\HttpServiceProvider; +use Flarum\Image\ImageServiceProvider; use Flarum\Locale\LocaleServiceProvider; use Flarum\Mail\MailServiceProvider; use Flarum\Notification\NotificationServiceProvider; @@ -115,6 +116,7 @@ class InstalledSite implements SiteInterface $app->register(GroupServiceProvider::class); $app->register(HashServiceProvider::class); $app->register(HttpServiceProvider::class); + $app->register(ImageServiceProvider::class); $app->register(LocaleServiceProvider::class); $app->register(MailServiceProvider::class); $app->register(NotificationServiceProvider::class); diff --git a/framework/core/src/Image/ImageServiceProvider.php b/framework/core/src/Image/ImageServiceProvider.php new file mode 100644 index 000000000..64f52d908 --- /dev/null +++ b/framework/core/src/Image/ImageServiceProvider.php @@ -0,0 +1,53 @@ +container->bind('image.drivers', function (): array { + return [ + 'gd' => Drivers\Gd\Driver::class, + 'imagick' => Drivers\Imagick\Driver::class + ]; + }); + + $this->container->singleton('image', function (Container $container): ImageManager { + $interventionDrivers = $container->make('image.drivers'); + + $configDriver = $container->make(Config::class)->offsetGet('intervention.driver'); + + // Default to 'gd' if not present in the config + $driver = $configDriver ?? 'gd'; + + // Check that the imagick library is actually available, else default back to gd. + if ($driver === 'imagick' && ! extension_loaded('imagick')) { + $driver = 'gd'; + } + + if (! Arr::has($interventionDrivers, $driver)) { + throw new RuntimeException("intervention/image: $driver is not valid"); + } + + return new ImageManager($interventionDrivers[$driver]); + }); + + $this->container->alias('image', ImageManager::class); + } +} diff --git a/framework/core/src/User/AvatarUploader.php b/framework/core/src/User/AvatarUploader.php index 443b9e69c..95c0a6c11 100644 --- a/framework/core/src/User/AvatarUploader.php +++ b/framework/core/src/User/AvatarUploader.php @@ -12,7 +12,7 @@ namespace Flarum\User; use Illuminate\Contracts\Filesystem\Factory; use Illuminate\Contracts\Filesystem\Filesystem; use Illuminate\Support\Str; -use Intervention\Image\Image; +use Intervention\Image\Interfaces\ImageInterface; class AvatarUploader { @@ -23,16 +23,19 @@ class AvatarUploader $this->uploadDir = $filesystemFactory->disk('flarum-avatars'); } - public function upload(User $user, Image $image): void + public function upload(User $user, ImageInterface $image): void { - if (extension_loaded('exif')) { - $image->orientate(); + $image = $image->cover(100, 100); + $avatarPath = Str::random(); + + if ($image->isAnimated()) { + $encodedImage = $image->toGif(); + $avatarPath .= '.gif'; + } else { + $encodedImage = $image->toPng(); + $avatarPath .= '.png'; } - $encodedImage = $image->fit(100, 100)->encode('png'); - - $avatarPath = Str::random().'.png'; - $this->removeFileAfterSave($user); $user->changeAvatarPath($avatarPath); diff --git a/framework/core/src/User/AvatarValidator.php b/framework/core/src/User/AvatarValidator.php index 0e10586e8..fbf6a6f26 100644 --- a/framework/core/src/User/AvatarValidator.php +++ b/framework/core/src/User/AvatarValidator.php @@ -14,7 +14,8 @@ use Flarum\Foundation\ValidationException; use Flarum\Locale\TranslatorInterface; use Illuminate\Validation\Factory; use Illuminate\Validation\Validator; -use Intervention\Image\Exception\NotReadableException; +use Intervention\Gif\Exceptions\DecoderException as GifDecoderException; +use Intervention\Image\Exceptions\DecoderException; use Intervention\Image\ImageManager; use Psr\Http\Message\UploadedFileInterface; use Symfony\Component\Mime\MimeTypes; @@ -76,8 +77,8 @@ class AvatarValidator extends AbstractValidator } try { - $this->imageManager->make($file->getStream()->getMetadata('uri')); - } catch (NotReadableException) { + $this->imageManager->read($file->getStream()->getMetadata('uri')); + } catch (DecoderException|GifDecoderException) { $this->raise('image'); } } diff --git a/framework/core/src/User/Command/RegisterUserHandler.php b/framework/core/src/User/Command/RegisterUserHandler.php index 88f848382..87f5c0fcd 100644 --- a/framework/core/src/User/Command/RegisterUserHandler.php +++ b/framework/core/src/User/Command/RegisterUserHandler.php @@ -18,6 +18,7 @@ use Flarum\User\Exception\PermissionDeniedException; use Flarum\User\RegistrationToken; use Flarum\User\User; use Flarum\User\UserValidator; +use GuzzleHttp\Client; use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Support\Arr; use Illuminate\Support\Str; @@ -135,9 +136,30 @@ class RegisterUserHandler throw new InvalidArgumentException("Provided avatar URL must have scheme http or https. Scheme provided was $scheme.", 503); } - $image = $this->imageManager->make($url); + $urlContents = $this->retrieveAvatarFromUrl($url); - $this->avatarUploader->upload($user, $image); + if ($urlContents !== null) { + $image = $this->imageManager->read($urlContents); + + $this->avatarUploader->upload($user, $image); + } + } + + private function retrieveAvatarFromUrl(string $url): ?string + { + $client = new Client(); + + try { + $response = $client->get($url); + } catch (\Exception $e) { + return null; + } + + if ($response->getStatusCode() !== 200) { + return null; + } + + return $response->getBody()->getContents(); } private function fulfillToken(User $user, RegistrationToken $token): void diff --git a/framework/core/src/User/Command/UploadAvatarHandler.php b/framework/core/src/User/Command/UploadAvatarHandler.php index b100a3d66..ec211e4e6 100644 --- a/framework/core/src/User/Command/UploadAvatarHandler.php +++ b/framework/core/src/User/Command/UploadAvatarHandler.php @@ -43,7 +43,7 @@ class UploadAvatarHandler $this->validator->assertValid(['avatar' => $command->file]); - $image = $this->imageManager->make($command->file->getStream()->getMetadata('uri')); + $image = $this->imageManager->read($command->file->getStream()->getMetadata('uri')); $this->events->dispatch( new AvatarSaving($user, $actor, $image) diff --git a/framework/core/src/User/Event/AvatarSaving.php b/framework/core/src/User/Event/AvatarSaving.php index 5d20c8612..bf242cccf 100644 --- a/framework/core/src/User/Event/AvatarSaving.php +++ b/framework/core/src/User/Event/AvatarSaving.php @@ -10,14 +10,14 @@ namespace Flarum\User\Event; use Flarum\User\User; -use Intervention\Image\Image; +use Intervention\Image\Interfaces\ImageInterface; class AvatarSaving { public function __construct( public User $user, public User $actor, - public Image $image + public ImageInterface $image ) { } } diff --git a/framework/core/tests/unit/User/AvatarUploaderTest.php b/framework/core/tests/unit/User/AvatarUploaderTest.php index 0252f9f4c..1323bb9fc 100644 --- a/framework/core/tests/unit/User/AvatarUploaderTest.php +++ b/framework/core/tests/unit/User/AvatarUploaderTest.php @@ -16,7 +16,7 @@ use Illuminate\Contracts\Events\Dispatcher; use Illuminate\Contracts\Filesystem\Factory; use Illuminate\Contracts\Filesystem\Filesystem; use Illuminate\Database\Eloquent\Model; -use Intervention\Image\ImageManagerStatic; +use Intervention\Image\ImageManager; use Mockery as m; class AvatarUploaderTest extends TestCase @@ -93,7 +93,7 @@ class AvatarUploaderTest extends TestCase $user->changeAvatarPath('ABCDEFGHabcdefgh.png'); $user->syncOriginal(); - $this->uploader->upload($user, ImageManagerStatic::canvas(50, 50)); + $this->uploader->upload($user, ImageManager::gd()->create(50, 50)); // Simulate saving foreach ($user->releaseAfterSaveCallbacks() as $callback) {