From e703009d7fa6d1f3e448c23611ac907277412c42 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 30 Sep 2023 19:12:22 +0100 Subject: [PATCH] Images: Added thin wrapper around image filesystem instances Extracts duplicated required handling (Like path adjustment) out to simpler storage disk instance which can be passed around. --- app/Uploads/ImageService.php | 75 +++-------------- app/Uploads/ImageStorage.php | 63 ++------------ app/Uploads/ImageStorageDisk.php | 139 +++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 118 deletions(-) create mode 100644 app/Uploads/ImageStorageDisk.php diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index 81d6add92..f8567c3e5 100644 --- a/app/Uploads/ImageService.php +++ b/app/Uploads/ImageService.php @@ -9,9 +9,6 @@ use BookStack\Exceptions\ImageUploadException; use ErrorException; use Exception; use Illuminate\Contracts\Cache\Repository as Cache; -use Illuminate\Contracts\Filesystem\FileNotFoundException; -use Illuminate\Contracts\Filesystem\Filesystem as StorageDisk; -use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Filesystem\FilesystemManager; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; @@ -85,7 +82,7 @@ class ImageService $imagePath = '/uploads/images/' . $type . '/' . date('Y-m') . '/'; - while ($disk->exists($this->storage->adjustPathForDisk($imagePath . $fileName, $type))) { + while ($disk->exists($imagePath . $fileName)) { $fileName = Str::random(3) . $fileName; } @@ -95,7 +92,7 @@ class ImageService } try { - $this->storage->storeInPublicSpace($disk, $this->storage->adjustPathForDisk($fullPath, $type), $imageData); + $disk->put($fullPath, $imageData, true); } catch (Exception $e) { Log::error('Error when attempting image upload:' . $e->getMessage()); @@ -129,11 +126,9 @@ class ImageService { $imageData = file_get_contents($file->getRealPath()); $disk = $this->storage->getDisk($type); - $adjustedPath = $this->storage->adjustPathForDisk($path, $type); - $disk->put($adjustedPath, $imageData); + $disk->put($path, $imageData); } - /** * Checks if the image is a gif. Returns true if it is, else false. */ @@ -191,13 +186,13 @@ class ImageService // If thumbnail has already been generated, serve that and cache path $disk = $this->storage->getDisk($image->type); - if (!$shouldCreate && $disk->exists($this->storage->adjustPathForDisk($thumbFilePath, $image->type))) { + if (!$shouldCreate && $disk->exists($thumbFilePath)) { $this->cache->put($thumbCacheKey, $thumbFilePath, 60 * 60 * 72); return $this->storage->getPublicUrl($thumbFilePath); } - $imageData = $disk->get($this->storage->adjustPathForDisk($imagePath, $image->type)); + $imageData = $disk->get($imagePath); // Do not resize apng images where we're not cropping if ($keepRatio && $this->isApngData($image, $imageData)) { @@ -212,7 +207,7 @@ class ImageService // If not in cache and thumbnail does not exist, generate thumb and cache path $thumbData = $this->resizeImage($imageData, $width, $height, $keepRatio); - $this->storage->storeInPublicSpace($disk, $this->storage->adjustPathForDisk($thumbFilePath, $image->type), $thumbData); + $disk->put($thumbFilePath, $thumbData, true); $this->cache->put($thumbCacheKey, $thumbFilePath, 60 * 60 * 72); return $this->storage->getPublicUrl($thumbFilePath); @@ -253,7 +248,6 @@ class ImageService return $thumbData; } - /** * Get the raw data content from an image. * @@ -263,7 +257,7 @@ class ImageService { $disk = $this->storage->getDisk(); - return $disk->get($this->storage->adjustPathForDisk($image->path, $image->type)); + return $disk->get($image->path); } /** @@ -271,53 +265,13 @@ class ImageService * * @throws Exception */ - public function destroy(Image $image) + public function destroy(Image $image): void { - $this->destroyImagesFromPath($image->path, $image->type); + $disk = $this->storage->getDisk($image->type); + $disk->destroyAllMatchingNameFromPath($image->path); $image->delete(); } - /** - * Destroys an image at the given path. - * Searches for image thumbnails in addition to main provided path. - */ - protected function destroyImagesFromPath(string $path, string $imageType): bool - { - $path = $this->storage->adjustPathForDisk($path, $imageType); - $disk = $this->storage->getDisk($imageType); - - $imageFolder = dirname($path); - $imageFileName = basename($path); - $allImages = collect($disk->allFiles($imageFolder)); - - // Delete image files - $imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) { - return basename($imagePath) === $imageFileName; - }); - $disk->delete($imagesToDelete->all()); - - // Cleanup of empty folders - $foldersInvolved = array_merge([$imageFolder], $disk->directories($imageFolder)); - foreach ($foldersInvolved as $directory) { - if ($this->isFolderEmpty($disk, $directory)) { - $disk->deleteDirectory($directory); - } - } - - return true; - } - - /** - * Check whether a folder is empty. - */ - protected function isFolderEmpty(StorageDisk $storage, string $path): bool - { - $files = $storage->files($path); - $folders = $storage->directories($path); - - return count($files) === 0 && count($folders) === 0; - } - /** * Delete gallery and drawings that are not within HTML content of pages or page revisions. * Checks based off of only the image name. @@ -325,7 +279,7 @@ class ImageService * * Returns the path of the images that would be/have been deleted. */ - public function deleteUnusedImages(bool $checkRevisions = true, bool $dryRun = true) + public function deleteUnusedImages(bool $checkRevisions = true, bool $dryRun = true): array { $types = ['gallery', 'drawio']; $deletedPaths = []; @@ -361,8 +315,6 @@ class ImageService * Attempts to convert the URL to a system storage url then * fetch the data from the disk or storage location. * Returns null if the image data cannot be fetched from storage. - * - * @throws FileNotFoundException */ public function imageUrlToBase64(string $url): ?string { @@ -371,8 +323,6 @@ class ImageService return null; } - $storagePath = $this->storage->adjustPathForDisk($storagePath); - // Apply access control when local_secure_restricted images are active if ($this->storage->usingSecureRestrictedImages()) { if (!$this->checkUserHasAccessToRelationOfImageAtPath($storagePath)) { @@ -412,8 +362,7 @@ class ImageService } // Check local_secure is active - return $this->storage->usingSecureImages() - && $disk instanceof FilesystemAdapter + return $disk->usingSecureImages() // Check the image file exists && $disk->exists($imagePath) // Check the file is likely an image file diff --git a/app/Uploads/ImageStorage.php b/app/Uploads/ImageStorage.php index c51450052..dc4abc0f2 100644 --- a/app/Uploads/ImageStorage.php +++ b/app/Uploads/ImageStorage.php @@ -2,10 +2,8 @@ namespace BookStack\Uploads; -use Illuminate\Contracts\Filesystem\Filesystem as StorageDisk; use Illuminate\Filesystem\FilesystemManager; use Illuminate\Support\Str; -use League\Flysystem\WhitespacePathNormalizer; class ImageStorage { @@ -17,44 +15,25 @@ class ImageStorage /** * Get the storage disk for the given image type. */ - public function getDisk(string $imageType = ''): StorageDisk + public function getDisk(string $imageType = ''): ImageStorageDisk { - return $this->fileSystem->disk($this->getDiskName($imageType)); - } + $diskName = $this->getDiskName($imageType); - /** - * Check if local secure image storage (Fetched behind authentication) - * is currently active in the instance. - */ - public function usingSecureImages(string $imageType = 'gallery'): bool - { - return $this->getDiskName($imageType) === 'local_secure_images'; + return new ImageStorageDisk( + $diskName, + $this->fileSystem->disk($diskName), + ); } /** * Check if "local secure restricted" (Fetched behind auth, with permissions enforced) * is currently active in the instance. */ - public function usingSecureRestrictedImages() + public function usingSecureRestrictedImages(): bool { return config('filesystems.images') === 'local_secure_restricted'; } - /** - * Change the originally provided path to fit any disk-specific requirements. - * This also ensures the path is kept to the expected root folders. - */ - public function adjustPathForDisk(string $path, string $imageType = ''): string - { - $path = (new WhitespacePathNormalizer())->normalizePath(str_replace('uploads/images/', '', $path)); - - if ($this->usingSecureImages($imageType)) { - return $path; - } - - return 'uploads/images/' . $path; - } - /** * Clean up an image file name to be both URL and storage safe. */ @@ -78,7 +57,7 @@ class ImageStorage */ protected function getDiskName(string $imageType): string { - $storageType = config('filesystems.images'); + $storageType = strtolower(config('filesystems.images')); $localSecureInUse = ($storageType === 'local_secure' || $storageType === 'local_secure_restricted'); // Ensure system images (App logo) are uploaded to a public space @@ -154,30 +133,4 @@ class ImageStorage return rtrim($basePath, '/') . $filePath; } - - /** - * Save image data for the given path in the public space, if possible, - * for the provided storage mechanism. - */ - public function storeInPublicSpace(StorageDisk $storage, string $path, string $data): void - { - $storage->put($path, $data); - - // Set visibility when a non-AWS-s3, s3-like storage option is in use. - // Done since this call can break s3-like services but desired for other image stores. - // Attempting to set ACL during above put request requires different permissions - // hence would technically be a breaking change for actual s3 usage. - if (!$this->isS3Like()) { - $storage->setVisibility($path, 'public'); - } - } - - /** - * Check if the image storage in use is an S3-like (but not likely S3) external system. - */ - protected function isS3Like(): bool - { - $usingS3 = strtolower(config('filesystems.images')) === 's3'; - return $usingS3 && !is_null(config('filesystems.disks.s3.endpoint')); - } } diff --git a/app/Uploads/ImageStorageDisk.php b/app/Uploads/ImageStorageDisk.php new file mode 100644 index 000000000..3a95661ca --- /dev/null +++ b/app/Uploads/ImageStorageDisk.php @@ -0,0 +1,139 @@ +diskName === 'local_secure_images'; + } + + /** + * Change the originally provided path to fit any disk-specific requirements. + * This also ensures the path is kept to the expected root folders. + */ + protected function adjustPathForDisk(string $path): string + { + $path = (new WhitespacePathNormalizer())->normalizePath(str_replace('uploads/images/', '', $path)); + + if ($this->usingSecureImages()) { + return $path; + } + + return 'uploads/images/' . $path; + } + + /** + * Check if a file at the given path exists. + */ + public function exists(string $path): bool + { + return $this->filesystem->exists($this->adjustPathForDisk($path)); + } + + /** + * Get the file at the given path. + */ + public function get(string $path): bool + { + return $this->filesystem->get($this->adjustPathForDisk($path)); + } + + /** + * Save the given image data at the given path. Can choose to set + * the image as public which will update its visibility after saving. + */ + public function put(string $path, string $data, bool $makePublic = false): void + { + $path = $this->adjustPathForDisk($path); + $this->filesystem->put($path, $data); + + // Set visibility when a non-AWS-s3, s3-like storage option is in use. + // Done since this call can break s3-like services but desired for other image stores. + // Attempting to set ACL during above put request requires different permissions + // hence would technically be a breaking change for actual s3 usage. + if ($makePublic && !$this->isS3Like()) { + $this->filesystem->setVisibility($path, 'public'); + } + } + + /** + * Destroys an image at the given path. + * Searches for image thumbnails in addition to main provided path. + */ + public function destroyAllMatchingNameFromPath(string $path): void + { + $path = $this->adjustPathForDisk($path); + + $imageFolder = dirname($path); + $imageFileName = basename($path); + $allImages = collect($this->filesystem->allFiles($imageFolder)); + + // Delete image files + $imagesToDelete = $allImages->filter(function ($imagePath) use ($imageFileName) { + return basename($imagePath) === $imageFileName; + }); + $this->filesystem->delete($imagesToDelete->all()); + + // Cleanup of empty folders + $foldersInvolved = array_merge([$imageFolder], $this->filesystem->directories($imageFolder)); + foreach ($foldersInvolved as $directory) { + if ($this->isFolderEmpty($directory)) { + $this->filesystem->deleteDirectory($directory); + } + } + } + + /** + * Get the mime type of the file at the given path. + * Only works for local filesystem adapters. + */ + public function mimeType(string $path): string + { + return $this->filesystem instanceof FilesystemAdapter ? $this->filesystem->mimeType($path) : ''; + } + + /** + * Get a stream response for the image at the given path. + */ + public function response(string $path): StreamedResponse + { + return $this->filesystem->response($path); + } + + /** + * Check if the image storage in use is an S3-like (but not likely S3) external system. + */ + protected function isS3Like(): bool + { + $usingS3 = $this->diskName === 's3'; + return $usingS3 && !is_null(config('filesystems.disks.s3.endpoint')); + } + + /** + * Check whether a folder is empty. + */ + protected function isFolderEmpty(string $path): bool + { + $files = $this->filesystem->files($path); + $folders = $this->filesystem->directories($path); + + return count($files) === 0 && count($folders) === 0; + } +}