Handle corrupt JSON files in deferred image storage (#110)

Co-authored-by: Martin Auswöger <[email protected]>

CHANGELOG.md

@@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## [Unreleased]
 
+ * Handle corrupt JSON files in deferred image storage. [#110]
 
 ## [1.2.2] (2024-10-02)
 
@@ -175,6 +176,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 [0.2.0]: https://github.com/contao/image/compare/0.1.0...0.2.0
 [0.1.0]: https://github.com/contao/image/commits/0.1.0
 
+[#110]: https://github.com/contao/image/issues/110
 [#108]: https://github.com/contao/image/issues/108
 [#104]: https://github.com/contao/image/issues/104
 [#101]: https://github.com/contao/image/issues/101

src/DeferredImageStorageFilesystem.php

@@ -69,7 +69,7 @@ public function set(string $path, array $value): void
      */
     public function get(string $path): array
     {
-        return $this->decode(file_get_contents($this->getConfigPath($path)));
+        return $this->decode(file_get_contents($this->getConfigPath($path)), $path);
     }
 
     /**
@@ -111,7 +111,7 @@ public function getLocked(string $path, bool $blocking = true): ?array
 
         $this->locks[$path] = $handle;
 
-        return $this->decode(stream_get_contents($handle));
+        return $this->decode(stream_get_contents($handle), $path);
     }
 
     /**
@@ -202,15 +202,21 @@ private function getConfigPath(string $path): string
     /**
      * Decodes the contents of a stored configuration.
      */
-    private function decode(string $contents): array
+    private function decode(string $contents, string $path): array
     {
         $content = json_decode($contents, true);
 
         if (JSON_ERROR_NONE !== json_last_error()) {
+            // Delete the file if the contents are corrupt
+            $this->delete($path);
+
             throw new JsonException(json_last_error_msg());
         }
 
         if (!\is_array($content)) {
+            // Delete the file if the contents are invalid
+            $this->delete($path);
+
             throw new InvalidArgumentException(sprintf('Invalid JSON data: expected array, got "%s"', \gettype($content)));
         }
 

src/DeferredResizer.php

@@ -92,7 +92,12 @@ public function getDeferredImage(string $targetPath, ImagineInterface $imagine):
             return null;
         }
 
-        $config = $this->storage->get($targetPath);
+        try {
+            $config = $this->storage->get($targetPath);
+        } catch (\Throwable $exception) {
+            // Ignore storage failure
+            return null;
+        }
 
         return new DeferredImage(
             Path::join($this->cacheDir, $targetPath),

tests/DeferredImageStorageFilesystemTest.php

@@ -137,7 +137,11 @@ public function testInvalidJsonThrows(): void
 
         $this->expectException(JsonException::class);
 
-        $storage->get('test');
+        try {
+            $storage->get('test');
+        } finally {
+            $this->assertFileNotExists(Path::join($this->rootDir, 'deferred/test.json'));
+        }
     }
 
     public function testInvalidJsonDataThrows(): void
@@ -151,7 +155,11 @@ public function testInvalidJsonDataThrows(): void
 
         $this->expectException(InvalidArgumentException::class);
 
-        $storage->get('test');
+        try {
+            $storage->get('test');
+        } finally {
+            $this->assertFileNotExists(Path::join($this->rootDir, 'deferred/test.json'));
+        }
     }
 
     public function testInvalidUtf8Throws(): void
@@ -223,4 +231,13 @@ public static function assertMatchesRegularExpression(string $pattern, string $s
             parent::assertRegExp($pattern, $string, $message);
         }
     }
+
+    public static function assertFileNotExists(string $filename, string $message = ''): void
+    {
+        if (method_exists(parent::class, 'assertFileDoesNotExist')) {
+            parent::assertFileDoesNotExist($filename, $message);
+        } else {
+            parent::assertFileNotExists($filename, $message);
+        }
+    }
 }

tests/DeferredResizerTest.php

@@ -17,6 +17,7 @@
 use Contao\Image\DeferredResizer;
 use Contao\Image\Exception\FileNotExistsException;
 use Contao\Image\Exception\InvalidArgumentException;
+use Contao\Image\Exception\JsonException;
 use Contao\Image\Exception\RuntimeException;
 use Contao\Image\Image;
 use Contao\Image\ImageDimensions;
@@ -225,6 +226,28 @@ public function testGetMissingDeferredImage(): void
         $this->assertNull($resizer->getDeferredImage($imagePath, $imagine));
     }
 
+    public function testGetCorruptDeferredImage(): void
+    {
+        $storage = $this->createMock(DeferredImageStorageInterface::class);
+        $storage
+            ->method('has')
+            ->willReturn(true)
+        ;
+
+        $storage
+            ->method('get')
+            ->with('a/foo-5fc1c9f9.jpg')
+            ->willThrowException(new JsonException('JSON error'))
+        ;
+
+        $imagine = $this->createMock(ImagineInterface::class);
+        $resizer = $this->createResizer(null, null, null, $storage);
+        $imagePath = Path::join($this->rootDir, 'a/foo-5fc1c9f9.jpg');
+        $deferredImage = $resizer->getDeferredImage($imagePath, $imagine);
+
+        $this->assertNull($deferredImage);
+    }
+
     public function testResizeDeferredImageThrowsForOutsidePath(): void
     {
         $resizer = $this->createResizer();