Added detection and thumbnail bypass for apng images

Adds apng sniffing when generating thumbnails with retained ratios to
serve the original image files, as we do for GIF images, to prevent
the image being resized to a static version.

Is more tricky than GIF since apng file mimes and extensions
are the same as png, we have to detect part of the file header
to sniff the type. Means we have to sniff at a later stage
than GIF since we have to load the image file data.

Made some changes to the image thubmnail caching while doing
this work to fit in with this handling.

Added test to cover.
For #3136.

diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php
index b8477eb4034df5ef3f2d1eef1a10b571bb613ccb..7243feeb0ec0fa1c23cad6fced69015e90d6fc4d 100644 (file)
--- a/app/Uploads/ImageService.php
+++ b/app/Uploads/ImageService.php
@@ -3,6 +3,7 @@
 namespace BookStack\Uploads;
 
 use BookStack\Exceptions\ImageUploadException;
+use BookStack\Util\WebSafeMimeSniffer;
 use ErrorException;
 use Exception;
 use Illuminate\Contracts\Cache\Repository as Cache;
@@ -228,6 +229,20 @@ class ImageService
         return strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'gif';
     }
 
+    /**
+     * Check if the given image and image data is apng.
+     */
+    protected function isApngData(Image $image, string &$imageData): bool
+    {
+        $isPng = strtolower(pathinfo($image->path, PATHINFO_EXTENSION)) === 'png';
+        if (!$isPng) {
+            return false;
+        }
+
+        $initialHeader = substr($imageData, 0, strpos($imageData, 'IDAT'));
+        return strpos($initialHeader, 'acTL') !== false;
+    }
+
     /**
      * Get the thumbnail for an image.
      * If $keepRatio is true only the width will be used.
@@ -238,6 +253,7 @@ class ImageService
      */
     public function getThumbnail(Image $image, ?int $width, ?int $height, bool $keepRatio = false): string
     {
+        // Do not resize GIF images where we're not cropping
         if ($keepRatio && $this->isGif($image)) {
             return $this->getPublicUrl($image->path);
         }
@@ -246,19 +262,33 @@ class ImageService
         $imagePath = $image->path;
         $thumbFilePath = dirname($imagePath) . $thumbDirName . basename($imagePath);
 
-        if ($this->cache->has('images-' . $image->id . '-' . $thumbFilePath) && $this->cache->get('images-' . $thumbFilePath)) {
-            return $this->getPublicUrl($thumbFilePath);
+        $thumbCacheKey = 'images::' . $image->id . '::' . $thumbFilePath;
+
+        // Return path if in cache
+        $cachedThumbPath = $this->cache->get($thumbCacheKey);
+        if ($cachedThumbPath) {
+            return $this->getPublicUrl($cachedThumbPath);
         }
 
+        // If thumbnail has already been generated, serve that and cache path
         $storage = $this->getStorageDisk($image->type);
         if ($storage->exists($this->adjustPathForStorageDisk($thumbFilePath, $image->type))) {
+            $this->cache->put($thumbCacheKey, $thumbFilePath, 60 * 60 * 72);
             return $this->getPublicUrl($thumbFilePath);
         }
 
-        $thumbData = $this->resizeImage($storage->get($this->adjustPathForStorageDisk($imagePath, $image->type)), $width, $height, $keepRatio);
+        $imageData = $storage->get($this->adjustPathForStorageDisk($imagePath, $image->type));
+
+        // Do not resize apng images where we're not cropping
+        if ($keepRatio && $this->isApngData($image, $imageData)) {
+            $this->cache->put($thumbCacheKey, $image->path, 60 * 60 * 72);
+            return $this->getPublicUrl($image->path);
+        }
 
+        // If not in cache and thumbnail does not exist, generate thumb and cache path
+        $thumbData = $this->resizeImage($imageData, $width, $height, $keepRatio);
         $this->saveImageDataInPublicSpace($storage, $this->adjustPathForStorageDisk($thumbFilePath, $image->type), $thumbData);
-        $this->cache->put('images-' . $image->id . '-' . $thumbFilePath, $thumbFilePath, 60 * 60 * 72);
+        $this->cache->put($thumbCacheKey, $thumbFilePath, 60 * 60 * 72);
 
         return $this->getPublicUrl($thumbFilePath);
     }

diff --git a/app/Util/WebSafeMimeSniffer.php b/app/Util/WebSafeMimeSniffer.php
index 4012004fc51f0670566811c782a870dfcd81b0d3..ea58e586bbbeab595f804f8a0e40ce3a5ec37cf6 100644 (file)
--- a/app/Util/WebSafeMimeSniffer.php
+++ b/app/Util/WebSafeMimeSniffer.php
@@ -17,6 +17,7 @@ class WebSafeMimeSniffer
         'application/json',
         'application/octet-stream',
         'application/pdf',
+        'image/apng',
         'image/bmp',
         'image/jpeg',
         'image/png',

diff --git a/tests/Uploads/ImageTest.php b/tests/Uploads/ImageTest.php
index 296e4d1878ae85680e8553e5f0152ea214273dea..32f79e9e06334b74325d9c50471085de40aed256 100644 (file)
--- a/tests/Uploads/ImageTest.php
+++ b/tests/Uploads/ImageTest.php
@@ -61,6 +61,19 @@ class ImageTest extends TestCase
         $this->assertEquals($originalFileSize, $displayFileSize, 'Display thumbnail generation should not increase image size');
     }
 
+    public function test_image_display_thumbnail_generation_for_apng_images_uses_original_file()
+    {
+        $page = Page::query()->first();
+        $admin = $this->getAdmin();
+        $this->actingAs($admin);
+
+        $imgDetails = $this->uploadGalleryImage($page, 'animated.png');
+        $this->deleteImage($imgDetails['path']);
+
+        $this->assertStringContainsString('thumbs-', $imgDetails['response']->thumbs->gallery);
+        $this->assertStringNotContainsString('thumbs-', $imgDetails['response']->thumbs->display);
+    }
+
     public function test_image_edit()
     {
         $editor = $this->getEditor();

diff --git a/tests/Uploads/UsesImages.php b/tests/Uploads/UsesImages.php
index 789c967c6771c6938bf0f51491c9b6ee5677b243..b55572248a8bc1668da8b34cce264b696c5fa9b7 100644 (file)
--- a/tests/Uploads/UsesImages.php
+++ b/tests/Uploads/UsesImages.php
@@ -4,6 +4,7 @@ namespace Tests\Uploads;
 
 use BookStack\Entities\Models\Page;
 use Illuminate\Http\UploadedFile;
+use stdClass;
 
 trait UsesImages
 {
@@ -85,7 +86,7 @@ trait UsesImages
      *
      * @param Page|null $page
      *
-     * @return array
+     * @return array{name: string, path: string, page: Page, response: stdClass}
      */
     protected function uploadGalleryImage(Page $page = null, ?string $testDataFileName = null)
     {

diff --git a/tests/test-data/animated.png b/tests/test-data/animated.png
new file mode 100644 (file)
index 0000000..1b35084
Binary files /dev/null and b/tests/test-data/animated.png differ