From 41a8d76ba45c6b81864baebd7ed82157f0464273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Heidk=C3=A4mper?= Date: Tue, 3 Jan 2023 16:25:12 +0100 Subject: [PATCH 1/5] Account for broken images when generating placeholders --- src/Breakpoint.php | 12 ++++++++++-- tests/Feature/BreakpointTest.php | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Breakpoint.php b/src/Breakpoint.php index b5177e8..3d7616f 100644 --- a/src/Breakpoint.php +++ b/src/Breakpoint.php @@ -266,8 +266,16 @@ private function placeholderSvg(): string * @see \Statamic\Tags\Glide::generateGlideDataUrl */ $cache = GlideManager::cacheDisk(); - $assetContentEncoded = base64_encode($cache->read($manipulationPath)); - $base64Placeholder = 'data:'.$cache->mimeType($manipulationPath).';base64,'.$assetContentEncoded; + + try { + $assetContent = $cache->read($manipulationPath); + } catch (\Exception $e) { + logger()->error($e->getMessage()); + + $assetContent = ''; + } + + $base64Placeholder = 'data:'.$cache->mimeType($manipulationPath).';base64,'.base64_encode($assetContent); return view('responsive-images::placeholderSvg', [ 'width' => 32, diff --git a/tests/Feature/BreakpointTest.php b/tests/Feature/BreakpointTest.php index d3eef14..92b47d0 100644 --- a/tests/Feature/BreakpointTest.php +++ b/tests/Feature/BreakpointTest.php @@ -141,3 +141,20 @@ expect($secondPlaceholder)->toEqual($firstPlaceholder) ->and($cacheDiskPathAfter)->not->toEqual($cacheDiskPathBefore); }); + +it("doesn't crash when the placeholder image cannot be read", function () { + $responsive = new Breakpoint($this->asset, 'default', 0, []); + + // Generate placeholder to trigger caching + $responsive->placeholder(); + + // Forget cached files + $pathPrefix = \Statamic\Imaging\ImageGenerator::assetCachePathPrefix($this->asset); + + \Statamic\Facades\Glide::server()->deleteCache($pathPrefix.'/'.$this->asset->path()); + + Blink::store()->flush(); + + // Generate new placeholder + $responsive->placeholder(); +})->expectNotToPerformAssertions(); From 129a59c959607ef09f5faf72293c3ab2eb9d6cbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Heidk=C3=A4mper?= Date: Wed, 4 Jan 2023 08:59:35 +0100 Subject: [PATCH 2/5] Throw UnableToReadFile exception right away when in debug mode --- src/Breakpoint.php | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/Breakpoint.php b/src/Breakpoint.php index 3d7616f..f886cdd 100644 --- a/src/Breakpoint.php +++ b/src/Breakpoint.php @@ -261,21 +261,7 @@ private function placeholderSvg(): string 'cache' => Config::get('statamic.assets.image_manipulation.cache', false), ]); - /** - * Glide tag has undocumented method for generating data URL that we borrow from - * @see \Statamic\Tags\Glide::generateGlideDataUrl - */ - $cache = GlideManager::cacheDisk(); - - try { - $assetContent = $cache->read($manipulationPath); - } catch (\Exception $e) { - logger()->error($e->getMessage()); - - $assetContent = ''; - } - - $base64Placeholder = 'data:'.$cache->mimeType($manipulationPath).';base64,'.base64_encode($assetContent); + $base64Placeholder = $this->readImageToBase64($manipulationPath); return view('responsive-images::placeholderSvg', [ 'width' => 32, @@ -285,4 +271,28 @@ private function placeholderSvg(): string ])->render(); }); } + + private function readImageToBase64($assetPath): string + { + /** + * Glide tag has undocumented method for generating data URL that we borrow from + * @see \Statamic\Tags\Glide::generateGlideDataUrl + */ + $cache = GlideManager::cacheDisk(); + + try { + $assetContent = $cache->read($assetPath); + $assetMimeType = $cache->mimeType($assetPath); + } catch (\Exception $e) { + if (config('app.debug')) { + throw $e; + } + + logger()->error($e->getMessage()); + + return ''; + } + + return 'data:'.$cache->mimeType($assetPath).';base64,'.base64_encode($assetContent); + } } From 6521f9003011bdb0fb78aa7af3cfe5aa41d1828f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Heidk=C3=A4mper?= Date: Wed, 4 Jan 2023 16:09:14 +0100 Subject: [PATCH 3/5] Don't output empty placeholder when a UnableToReadFile exception occurs --- src/Breakpoint.php | 49 +++++++++++++++++++------------- tests/Feature/BreakpointTest.php | 2 +- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/Breakpoint.php b/src/Breakpoint.php index f886cdd..0e00285 100644 --- a/src/Breakpoint.php +++ b/src/Breakpoint.php @@ -59,7 +59,13 @@ public function getSrcSet(bool $includePlaceholder = true, string $format = null return "{$this->buildImageJob($width, $format, $this->ratio)->handle()} {$width}w"; }) ->when($includePlaceholder, function (Collection $widths) { - return $widths->prepend($this->placeholderSrc()); + $placeholderSrc = $this->placeholderSrc(); + + if (empty($placeholderSrc)) { + return $widths; + } + + return $widths->prepend($placeholderSrc); }) ->implode(', '); } @@ -210,13 +216,6 @@ private function getGlideParams(): array ->toArray(); } - public function placeholder(): string - { - $base64Svg = base64_encode($this->placeholderSvg()); - - return "data:image/svg+xml;base64,{$base64Svg}"; - } - public function toGql(array $args): array { $data = [ @@ -241,12 +240,7 @@ public function toGql(array $args): array return $data; } - private function placeholderSrc(): string - { - return $this->placeholder() . ' 32w'; - } - - private function placeholderSvg(): string + public function placeholder(): string { return Blink::once("placeholder-{$this->asset->id()}-{$this->ratio}", function () { $imageGenerator = app(ImageGenerator::class); @@ -261,18 +255,35 @@ private function placeholderSvg(): string 'cache' => Config::get('statamic.assets.image_manipulation.cache', false), ]); - $base64Placeholder = $this->readImageToBase64($manipulationPath); + $base64Image = $this->readImageToBase64($manipulationPath); - return view('responsive-images::placeholderSvg', [ + if (! $base64Image) { + return ''; + } + + $placeholderSvg = view('responsive-images::placeholderSvg', [ 'width' => 32, 'height' => round(32 / $this->ratio), - 'image' => $base64Placeholder, + 'image' => $base64Image, 'asset' => $this->asset->toAugmentedArray(), ])->render(); + + return 'data:image/svg+xml;base64,' . base64_encode($placeholderSvg); }); } - private function readImageToBase64($assetPath): string + private function placeholderSrc(): string + { + $placeholder = $this->placeholder(); + + if (empty($placeholder)) { + return ''; + } + + return $placeholder . ' 32w'; + } + + private function readImageToBase64($assetPath): string|null { /** * Glide tag has undocumented method for generating data URL that we borrow from @@ -290,7 +301,7 @@ private function readImageToBase64($assetPath): string logger()->error($e->getMessage()); - return ''; + return null; } return 'data:'.$cache->mimeType($assetPath).';base64,'.base64_encode($assetContent); diff --git a/tests/Feature/BreakpointTest.php b/tests/Feature/BreakpointTest.php index 92b47d0..70debfa 100644 --- a/tests/Feature/BreakpointTest.php +++ b/tests/Feature/BreakpointTest.php @@ -122,7 +122,7 @@ /** * We use Blink cache for placeholder generation that we need to clear just in case * @see https://statamic.dev/extending/blink-cache - * @see Breakpoint::placeholderSvg() + * @see Breakpoint::placeholder() */ Blink::store()->flush(); From 996b7bb89f464a38d9fbb166c16b691fce1b3b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Heidk=C3=A4mper?= Date: Wed, 4 Jan 2023 18:35:56 +0100 Subject: [PATCH 4/5] Use the FilesystemException interface from Flysystem --- src/Breakpoint.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Breakpoint.php b/src/Breakpoint.php index 0e00285..869fe4f 100644 --- a/src/Breakpoint.php +++ b/src/Breakpoint.php @@ -5,6 +5,7 @@ use Illuminate\Contracts\Support\Arrayable; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Config; +use League\Flysystem\FilesystemException; use Spatie\ResponsiveImages\Jobs\GenerateImageJob; use Statamic\Contracts\Assets\Asset; use Statamic\Facades\Blink; @@ -294,7 +295,7 @@ private function readImageToBase64($assetPath): string|null try { $assetContent = $cache->read($assetPath); $assetMimeType = $cache->mimeType($assetPath); - } catch (\Exception $e) { + } catch (FilesystemException $e) { if (config('app.debug')) { throw $e; } From a1624ac75961effa5d39e60839e41bedd828a104 Mon Sep 17 00:00:00 2001 From: ncla Date: Sun, 8 Jan 2023 21:41:07 +0200 Subject: [PATCH 5/5] Use $assetMimeType variable --- src/Breakpoint.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Breakpoint.php b/src/Breakpoint.php index 869fe4f..103d3e1 100644 --- a/src/Breakpoint.php +++ b/src/Breakpoint.php @@ -305,6 +305,6 @@ private function readImageToBase64($assetPath): string|null return null; } - return 'data:'.$cache->mimeType($assetPath).';base64,'.base64_encode($assetContent); + return 'data:' . $assetMimeType . ';base64,' . base64_encode($assetContent); } }