diff --git a/app/Uploads/ImageService.php b/app/Uploads/ImageService.php index b8477eb40..7243feeb0 100644 --- 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 4012004fc..ea58e586b 100644 --- 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 296e4d187..32f79e9e0 100644 --- 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 789c967c6..b55572248 100644 --- 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 index 000000000..1b35084a3 Binary files /dev/null and b/tests/test-data/animated.png differ