Enforce actual Imagick sequence frame limits

docs/safety.md

@@ -51,6 +51,11 @@ The default `Limits` policy is:
 | Truncated input | reject |
 | Projected output checks | enabled |
 
+Imagick checks the actual sequence length with a metadata probe before decoding
+pixels, and again before coalescing frames. Where available, its native list-length
+limit also bounds decoding and is restored after each request. Host decoder policies
+still govern the cost of probing compressed formats and external delegates.
+
 Apply stricter limits to a request when needed:
 
 ```php

src/Driver/Imagick/ImagickDriver.php

@@ -21,6 +21,7 @@
 use Alto\Image\Effort;
 use Alto\Image\Exception\CorruptImageException;
 use Alto\Image\Exception\DriverException;
+use Alto\Image\Exception\LimitExceededException;
 use Alto\Image\Format;
 use Alto\Image\Internal\QualitySearch;
 use Alto\Image\Limits;
@@ -151,6 +152,25 @@ public function canEncode(Encoding $encoding): Support
 
     public function process(Plan $plan): array
     {
+        return ResourcePolicy::withFrameLimit($plan->limits, fn(): array => $this->processWithinFrameLimit($plan));
+    }
+
+    /**
+     * @return list<Result>
+     */
+    private function processWithinFrameLimit(Plan $plan): array
+    {
+        $probe = new \Imagick();
+
+        try {
+            $probe->pingImageBlob($plan->source->contents());
+            $this->checkFrames($probe, $plan);
+        } catch (\ImagickException $error) {
+            $this->throwReadError($plan, $error);
+        } finally {
+            $probe->clear();
+        }
+
         $started = microtime(true);
         $master = null;
         $results = [];
@@ -263,6 +283,15 @@ private function passThrough(Plan $plan, int $index, float $duration): Result
         );
     }
 
+    private function checkFrames(\Imagick $image, Plan $plan): void
+    {
+        $count = $image->getNumberImages();
+
+        if ($count > $plan->limits->maxFrames) {
+            throw LimitExceededException::frames($count, $plan->limits->maxFrames, $plan->source->origin());
+        }
+    }
+
     private function decode(Plan $plan): \Imagick
     {
         ResourcePolicy::apply($plan->limits);
@@ -286,6 +315,8 @@ private function decode(Plan $plan): \Imagick
             // @codeCoverageIgnoreEnd
         }
 
+        $this->checkFrames($image, $plan);
+
         // Coalescing turns frame-local rectangles into full canvases before the
         // same transform is applied to every frame.
         if ($image->getNumberImages() > 1) {
@@ -398,25 +429,34 @@ private function read(Plan $plan, ?string $hint): \Imagick
         } catch (\ImagickException $error) {
             $image->clear();
 
-            // canDecode() answered optimistically to keep queryFormats() off the
-            // hot path, so this is where a build that really cannot read the
-            // format has to say so, and say where the true list is.
-            if (!$this->writesFormat($plan->input->format) && !$this->knowsFormat($plan->input->format)) {
-                throw new DriverException(\sprintf(
-                    "This ImageMagick has no coder for %s.\n"
-                    . "  Run vendor/bin/image doctor for the available formats.\n"
-                    . '  It said: %s',
-                    $plan->input->format->value,
-                    $error->getMessage(),
-                ), 0, $error);
-            }
-
-            throw CorruptImageException::unreadableHeader($plan->source->origin() . ': ' . $error->getMessage());
+            $this->throwReadError($plan, $error);
         }
 
         return $image;
     }
 
+    private function throwReadError(Plan $plan, \ImagickException $error): never
+    {
+        if (str_contains(strtolower($error->getMessage()), 'list length exceeds limit')) {
+            throw new LimitExceededException('ImageMagick rejected the sequence at its native frame limit: ' . $error->getMessage(), 0, $error);
+        }
+
+        // canDecode() answered optimistically to keep queryFormats() off the
+        // hot path, so this is where a build that really cannot read the
+        // format has to say so, and say where the true list is.
+        if (!$this->writesFormat($plan->input->format) && !$this->knowsFormat($plan->input->format)) {
+            throw new DriverException(\sprintf(
+                "This ImageMagick has no coder for %s.\n"
+                . "  Run vendor/bin/image doctor for the available formats.\n"
+                . '  It said: %s',
+                $plan->input->format->value,
+                $error->getMessage(),
+            ), 0, $error);
+        }
+
+        throw CorruptImageException::unreadableHeader($plan->source->origin() . ': ' . $error->getMessage());
+    }
+
     /**
      * @param list<Placement> $placements
      */

src/Driver/Imagick/ResourcePolicy.php

@@ -74,6 +74,39 @@ public static function apply(Limits $limits): array
         return $applied;
     }
 
+    /**
+     * @template T
+     *
+     * @param \Closure(): T $work
+     *
+     * @return T
+     */
+    public static function withFrameLimit(Limits $limits, \Closure $work): mixed
+    {
+        $constant = self::constant('listlength');
+
+        if (null === $constant) {
+            // Unavailable on older ImageMagick builds.
+            // @codeCoverageIgnoreStart
+            return $work();
+            // @codeCoverageIgnoreEnd
+        }
+
+        // Imagick returns a float; its unlimited value rounds above PHP_INT_MAX.
+        $current = \Imagick::getResourceLimit($constant);
+        $previous = $current >= \PHP_INT_MAX ? \PHP_INT_MAX : (int) $current;
+        // The decoder needs a sentinel image after the last accepted frame.
+        $ceiling = $limits->maxFrames < \PHP_INT_MAX ? $limits->maxFrames + 1 : \PHP_INT_MAX;
+
+        try {
+            \Imagick::setResourceLimit($constant, min($previous, $ceiling));
+
+            return $work();
+        } finally {
+            \Imagick::setResourceLimit($constant, $previous);
+        }
+    }
+
     /**
      * The limits that did not end up where they were asked to, for doctor.
      *
@@ -133,6 +166,7 @@ private static function bytesPerPixel(): int
     private static function constant(string $name): ?int
     {
         $map = [
+            'listlength' => 'RESOURCETYPE_LISTLENGTH',
             'width' => 'RESOURCETYPE_WIDTH',
             'height' => 'RESOURCETYPE_HEIGHT',
             'area' => 'RESOURCETYPE_AREA',

tests/Driver/AnimationLimitsTest.php

@@ -0,0 +1,136 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the ALTO library.
+ *
+ * © 2026-present Simon André
+ *
+ * For full copyright and license information, please see
+ * the LICENSE file distributed with this source code.
+ */
+
+namespace Alto\Image\Tests\Driver;
+
+use Alto\Image\Driver\Imagick\ImagickDriver;
+use Alto\Image\Driver\Imagick\ResourcePolicy;
+use Alto\Image\Exception\CorruptImageException;
+use Alto\Image\Exception\LimitExceededException;
+use Alto\Image\Image;
+use Alto\Image\Limits;
+use Alto\Image\Source;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+#[CoversClass(ImagickDriver::class)]
+#[CoversClass(ResourcePolicy::class)]
+final class AnimationLimitsTest extends TestCase
+{
+    public function testCopyWithUnlimitedPolicyDoesNotChangePixelResourceLimits(): void
+    {
+        $bytes = $this->sequence('gif');
+        $before = \Imagick::getResourceLimit(\Imagick::RESOURCETYPE_MEMORY);
+        $result = Image::open(Source::bytes($bytes))->using(new ImagickDriver())->within(Limits::none())->keepMetadata()->render();
+        self::assertSame($bytes, $result->bytes);
+        self::assertSame($before, \Imagick::getResourceLimit(\Imagick::RESOURCETYPE_MEMORY));
+    }
+
+    public function testInvalidPixelPayloadKeepsTheCorruptImageError(): void
+    {
+        $png = $this->sequence('png');
+        $offset = strpos($png, 'IDAT');
+        self::assertNotFalse($offset);
+        $length = unpack('Nlength', substr($png, $offset - 4, 4));
+        self::assertIsArray($length);
+        self::assertIsInt($length['length']);
+        $bad = str_repeat('x', $length['length']);
+        $png = substr_replace($png, $bad . pack('N', crc32('IDAT' . $bad)), $offset + 4, $length['length'] + 4);
+        $this->expectException(CorruptImageException::class);
+        Image::open(Source::bytes($png))->using(new ImagickDriver())->grayscale()->render();
+    }
+
+    #[DataProvider('formats')]
+    public function testActualFrameCountCannotExceedTheLimit(string $format): void
+    {
+        $bytes = $this->sequence($format);
+        $request = Image::open(Source::bytes($bytes))->using(new ImagickDriver())->within(new Limits(maxFrames: 2));
+        $this->expectException(LimitExceededException::class);
+        $request->grayscale()->png()->render();
+    }
+
+    public function testAllowedAnimationKeepsItsFramesAndTiming(): void
+    {
+        $bytes = $this->sequence('gif');
+        $output = Image::open(Source::bytes($bytes))->using(new ImagickDriver())->within(new Limits(maxFrames: 3))->grayscale()->render();
+        $decoded = new \Imagick();
+        $decoded->readImageBlob($output->bytes);
+        self::assertSame(3, $decoded->getNumberImages());
+        foreach ($decoded as $frame) {
+            self::assertSame(10, $frame->getImageDelay());
+        }
+        $decoded->clear();
+    }
+
+    #[DataProvider('frameLimits')]
+    public function testFailedAndSuccessfulRequestsRestoreTheNativeFrameLimit(int $maximum): void
+    {
+        $bytes = $this->sequence('gif');
+        if (!defined(\Imagick::class . '::RESOURCETYPE_LISTLENGTH')) {
+            self::markTestSkipped('Native list-length control is not available.');
+        }
+        $previous = \Imagick::getResourceLimit(\Imagick::RESOURCETYPE_LISTLENGTH);
+        $image = Image::open(Source::bytes($bytes))->using(new ImagickDriver());
+        try {
+            $image->within(new Limits(maxFrames: $maximum))->render();
+            self::assertSame(3, $maximum);
+        } catch (LimitExceededException) {
+            self::assertSame(2, $maximum);
+        }
+        self::assertSame($previous, \Imagick::getResourceLimit(\Imagick::RESOURCETYPE_LISTLENGTH));
+        $direct = new \Imagick();
+        $direct->readImageBlob($bytes);
+        self::assertSame(3, $direct->getNumberImages());
+        $direct->clear();
+    }
+
+    /**
+     * @return iterable<array{int}>
+     */
+    public static function frameLimits(): iterable
+    {
+        yield [2];
+        yield [3];
+    }
+
+    /**
+     * @return iterable<array{string}>
+     */
+    public static function formats(): iterable
+    {
+        yield ['gif'];
+        yield ['webp'];
+        yield ['tiff'];
+    }
+
+    private function sequence(string $format): string
+    {
+        if (!extension_loaded('imagick') || [] === \Imagick::queryFormats(strtoupper($format))) {
+            self::markTestSkipped('Imagick format not available.');
+        }
+        $sequence = new \Imagick();
+        foreach (['red', 'green', 'blue'] as $colour) {
+            $frame = new \Imagick();
+            $frame->newImage(2, 2, new \ImagickPixel($colour));
+            $frame->setImageFormat($format);
+            $frame->setImageDelay(10);
+            $sequence->addImage($frame);
+            $frame->clear();
+        }
+        $sequence->setImageFormat($format);
+        $bytes = $sequence->getImagesBlob();
+        $sequence->clear();
+        return $bytes;
+    }
+}