From c7fea8fe08e8d26840da7a0d353c05f7a84d3ff1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 1 Nov 2021 00:24:42 +0000 Subject: [PATCH] Cleaned up logic within ImageRepo - Moved out extension check to ImageService as that seems more relevant. - Updated models to use static-style references instead of facade to align with common modern usage within the app. - Updated custom image_extension validation rule to use shared logic in image service. --- app/Entities/Tools/PageContent.php | 3 +- .../Controllers/Images/ImageController.php | 10 +----- .../CustomValidationServiceProvider.php | 6 ++-- app/Uploads/ImageRepo.php | 32 ++++--------------- app/Uploads/ImageService.php | 12 +++++++ 5 files changed, 25 insertions(+), 38 deletions(-) diff --git a/app/Entities/Tools/PageContent.php b/app/Entities/Tools/PageContent.php index 724230a3d..c5b17ddef 100644 --- a/app/Entities/Tools/PageContent.php +++ b/app/Entities/Tools/PageContent.php @@ -9,6 +9,7 @@ use BookStack\Exceptions\ImageUploadException; use BookStack\Facades\Theme; use BookStack\Theming\ThemeEvents; use BookStack\Uploads\ImageRepo; +use BookStack\Uploads\ImageService; use BookStack\Util\HtmlContentFilter; use DOMDocument; use DOMNodeList; @@ -130,7 +131,7 @@ class PageContent $imageInfo = $this->parseBase64ImageUri($uri); // Validate extension and content - if (empty($imageInfo['data']) || !$imageRepo->imageExtensionSupported($imageInfo['extension'])) { + if (empty($imageInfo['data']) || !ImageService::isExtensionSupported($imageInfo['extension'])) { return ''; } diff --git a/app/Http/Controllers/Images/ImageController.php b/app/Http/Controllers/Images/ImageController.php index 66ccadc5e..231712d52 100644 --- a/app/Http/Controllers/Images/ImageController.php +++ b/app/Http/Controllers/Images/ImageController.php @@ -9,27 +9,19 @@ use BookStack\Uploads\Image; use BookStack\Uploads\ImageRepo; use BookStack\Uploads\ImageService; use Exception; -use Illuminate\Filesystem\Filesystem as File; -use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Storage; use Illuminate\Validation\ValidationException; -use League\Flysystem\Util; class ImageController extends Controller { - protected $image; - protected $file; protected $imageRepo; protected $imageService; /** * ImageController constructor. */ - public function __construct(Image $image, File $file, ImageRepo $imageRepo, ImageService $imageService) + public function __construct(ImageRepo $imageRepo, ImageService $imageService) { - $this->image = $image; - $this->file = $file; $this->imageRepo = $imageRepo; $this->imageService = $imageService; } diff --git a/app/Providers/CustomValidationServiceProvider.php b/app/Providers/CustomValidationServiceProvider.php index c54f48ca3..c2c0197c7 100644 --- a/app/Providers/CustomValidationServiceProvider.php +++ b/app/Providers/CustomValidationServiceProvider.php @@ -2,6 +2,7 @@ namespace BookStack\Providers; +use BookStack\Uploads\ImageService; use Illuminate\Support\Facades\Validator; use Illuminate\Support\ServiceProvider; @@ -13,9 +14,8 @@ class CustomValidationServiceProvider extends ServiceProvider public function boot(): void { Validator::extend('image_extension', function ($attribute, $value, $parameters, $validator) { - $validImageExtensions = ['png', 'jpg', 'jpeg', 'gif', 'webp']; - - return in_array(strtolower($value->getClientOriginalExtension()), $validImageExtensions); + $extension = strtolower($value->getClientOriginalExtension()); + return ImageService::isExtensionSupported($extension); }); Validator::extend('safe_url', function ($attribute, $value, $parameters, $validator) { diff --git a/app/Uploads/ImageRepo.php b/app/Uploads/ImageRepo.php index 694560a14..4d46bb56d 100644 --- a/app/Uploads/ImageRepo.php +++ b/app/Uploads/ImageRepo.php @@ -11,36 +11,18 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; class ImageRepo { - protected $image; protected $imageService; protected $restrictionService; - protected $page; - - protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; /** * ImageRepo constructor. */ public function __construct( - Image $image, ImageService $imageService, - PermissionService $permissionService, - Page $page + PermissionService $permissionService ) { - $this->image = $image; $this->imageService = $imageService; $this->restrictionService = $permissionService; - $this->page = $page; - } - - /** - * Check if the given image extension is supported by BookStack. - * The extension must not be altered in this function. This check should provide a guarantee - * that the provided extension is safe to use for the image to be saved. - */ - public function imageExtensionSupported(string $extension): bool - { - return in_array($extension, static::$supportedExtensions); } /** @@ -48,7 +30,7 @@ class ImageRepo */ public function getById($id): Image { - return $this->image->findOrFail($id); + return Image::query()->findOrFail($id); } /** @@ -83,7 +65,7 @@ class ImageRepo string $search = null, callable $whereClause = null ): array { - $imageQuery = $this->image->newQuery()->where('type', '=', strtolower($type)); + $imageQuery = Image::query()->where('type', '=', strtolower($type)); if ($uploadedTo !== null) { $imageQuery = $imageQuery->where('uploaded_to', '=', $uploadedTo); @@ -114,7 +96,7 @@ class ImageRepo int $uploadedTo = null, string $search = null ): array { - $contextPage = $this->page->findOrFail($uploadedTo); + $contextPage = Page::visible()->findOrFail($uploadedTo); $parentFilter = null; if ($filterType === 'book' || $filterType === 'page') { @@ -205,7 +187,7 @@ class ImageRepo */ public function destroyByType(string $imageType) { - $images = $this->image->where('type', '=', $imageType)->get(); + $images = Image::query()->where('type', '=', $imageType)->get(); foreach ($images as $image) { $this->destroyImage($image); } @@ -218,10 +200,10 @@ class ImageRepo */ public function loadThumbs(Image $image) { - $image->thumbs = [ + $image->setAttribute('thumbs', [ 'gallery' => $this->getThumbnail($image, 150, 150, false), 'display' => $this->getThumbnail($image, 1680, null, true), - ]; + ]); } /** diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index dc18783c5..4cd818bcc 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -26,6 +26,8 @@ class ImageService protected $image; protected $fileSystem; + protected static $supportedExtensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; + /** * ImageService constructor. */ @@ -470,6 +472,16 @@ class ImageService return $disk->response($path); } + /** + * Check if the given image extension is supported by BookStack. + * The extension must not be altered in this function. This check should provide a guarantee + * that the provided extension is safe to use for the image to be saved. + */ + public static function isExtensionSupported(string $extension): bool + { + return in_array($extension, static::$supportedExtensions); + } + /** * Get a storage path for the given image URL. * Ensures the path will start with "uploads/images".