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

@@ -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($path, $imageData);
-        $disk->put($adjustedPath, $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()
+        return $disk->usingSecureImages()
-            && $disk instanceof FilesystemAdapter
             // Check the image file exists
             && $disk->exists($imagePath)
             // Check the file is likely an image file

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);
-    }
 
-    /**
+        return new ImageStorageDisk(
-     * Check if local secure image storage (Fetched behind authentication)
+            $diskName,
-     * is currently active in the instance.
+            $this->fileSystem->disk($diskName),
-     */
+        );
-    public function usingSecureImages(string $imageType = 'gallery'): bool
-    {
-        return $this->getDiskName($imageType) === 'local_secure_images';
     }
 
     /**
      * 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'));
-    }
 }

app/Uploads/ImageStorageDisk.php

@@ -0,0 +1,139 @@
+<?php
+
+namespace BookStack\Uploads;
+
+use Illuminate\Contracts\Filesystem\Filesystem;
+use Illuminate\Filesystem\FilesystemAdapter;
+use League\Flysystem\WhitespacePathNormalizer;
+use Symfony\Component\HttpFoundation\StreamedResponse;
+
+class ImageStorageDisk
+{
+    public function __construct(
+        protected string $diskName,
+        protected Filesystem $filesystem,
+    ) {
+    }
+
+    /**
+     * Check if local secure image storage (Fetched behind authentication)
+     * is currently active in the instance.
+     */
+    public function usingSecureImages(): bool
+    {
+        return $this->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;
+    }
+}