Add Opt-In Runtime Validation of Filter Chain Specifications

src/Exception/InvalidSpecificationArrayException.php

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Laminas\Filter\Exception;
+
+use InvalidArgumentException;
+
+use function get_debug_type;
+use function sprintf;
+
+final class InvalidSpecificationArrayException extends InvalidArgumentException implements ExceptionInterface
+{
+    public static function becauseTheFilterListMustBeAnArray(mixed $spec): self
+    {
+        return new self(sprintf(
+            'The `filters` key must be a list of arrays or filter instances. Received %s',
+            get_debug_type($spec),
+        ));
+    }
+
+    public static function becauseFilterSpecMustBeAnArray(mixed $spec): self
+    {
+        return new self(sprintf(
+            'Each member of the `filters` list must be array specification. Received %s',
+            get_debug_type($spec),
+        ));
+    }
+
+    public static function becauseFilterNamesMustBeAString(mixed $name): self
+    {
+        return new self(sprintf(
+            'Individual filter array specifications must have the key `name` that references a filter by its '
+            . 'fully qualified class name or an alias configured in the plugin manager. Received %s',
+            get_debug_type($name),
+        ));
+    }
+
+    public static function becauseOptionsShouldBeArrays(mixed $options): self
+    {
+        return new self(sprintf(
+            'Filter options must be an array when specified. Received %s',
+            get_debug_type($options),
+        ));
+    }
+
+    public static function becauseFilterPriorityMustBeAnInteger(mixed $priority): self
+    {
+        return new self(sprintf(
+            'Filter priorities must be integers when specified. Received %s',
+            get_debug_type($priority),
+        ));
+    }
+
+    public static function becauseCallbackListMustBeAList(mixed $spec): self
+    {
+        return new self(sprintf(
+            'The `callbacks` key must be a list of arrays. Received %s',
+            get_debug_type($spec),
+        ));
+    }
+
+    public static function becauseCallbackMustBeArray(mixed $spec): self
+    {
+        return new self(sprintf(
+            'All items listed under the `callbacks` key must be arrays. Received %s',
+            get_debug_type($spec),
+        ));
+    }
+
+    public static function becauseCallbackMustBePresent(mixed $callback): self
+    {
+        return new self(sprintf(
+            'Each callback filter listed under `callbacks` must contain a callable under the key '
+            . '`callback`. Received %s',
+            get_debug_type($callback),
+        ));
+    }
+}

src/FilterChain.php

@@ -6,25 +6,30 @@
 
 use Countable;
 use IteratorAggregate;
+use Laminas\Filter\Exception\InvalidSpecificationArrayException;
 use Laminas\Stdlib\PriorityQueue;
 use Psr\Container\ContainerExceptionInterface;
 use Traversable;
 
 use function count;
+use function is_array;
 use function is_callable;
+use function is_int;
+use function is_string;
 
 /**
  * @psalm-type InstanceType = FilterInterface|(callable(mixed): mixed)
+ * @psalm-type FilterSpecification = array{
+ *     name: string|class-string<FilterInterface>,
+ *     options?: array<string, mixed>,
+ *     priority?: int,
+ * }|InstanceType
  * @psalm-type FilterChainConfiguration = array{
- *    filters?: list<array{
- *        name: string|class-string<FilterInterface>,
- *        options?: array<string, mixed>,
- *        priority?: int,
- *    }|InstanceType>,
- *    callbacks?: list<array{
- *        callback: FilterInterface|(callable(mixed): mixed),
- *        priority?: int,
- *    }>
+ *     filters?: list<FilterSpecification>,
+ *     callbacks?: list<array{
+ *         callback: FilterInterface|(callable(mixed): mixed),
+ *         priority?: int,
+ *     }>
  * }
  * @implements IteratorAggregate<array-key, InstanceType>
  * @implements FilterChainInterface<mixed>
@@ -137,4 +142,97 @@ public function getIterator(): Traversable
     {
         return clone $this->filters;
     }
+
+    /**
+     * @psalm-assert FilterChainConfiguration $spec
+     * @throws InvalidSpecificationArrayException If the specification is invalid.
+     */
+    public static function validateSpecification(array $spec): void
+    {
+        /** @psalm-var mixed $filters */
+        $filters = $spec['filters'] ?? null;
+        /** @psalm-var mixed $callbacks */
+        $callbacks = $spec['callbacks'] ?? null;
+
+        if ($filters === null && $callbacks === null) {
+            return; // An effectively empty specification is OK
+        }
+
+        if ($filters !== null) {
+            self::validateFilterList($filters);
+        }
+
+        if ($callbacks !== null) {
+            self::validateCallbackList($callbacks);
+        }
+    }
+
+    private static function validateFilterList(mixed $spec): void
+    {
+        if (! is_array($spec)) {
+            throw InvalidSpecificationArrayException::becauseTheFilterListMustBeAnArray($spec);
+        }
+
+        /** @psalm-var mixed $item */
+        foreach ($spec as $item) {
+            self::validateFilterSpecification($item);
+        }
+    }
+
+    private static function validateFilterSpecification(mixed $spec): void
+    {
+        if (is_callable($spec) || $spec instanceof FilterInterface) {
+            return;
+        }
+
+        if (! is_array($spec)) {
+            throw InvalidSpecificationArrayException::becauseFilterSpecMustBeAnArray($spec);
+        }
+
+        $name     = $spec['name'] ?? null;
+        $options  = $spec['options'] ?? null;
+        $priority = $spec['priority'] ?? null;
+
+        if (! is_string($name) || $name === '') {
+            throw InvalidSpecificationArrayException::becauseFilterNamesMustBeAString($name ?? null);
+        }
+
+        if ($options !== null && ! is_array($options)) {
+            throw InvalidSpecificationArrayException::becauseOptionsShouldBeArrays($options);
+        }
+
+        if ($priority !== null && ! is_int($priority)) {
+            throw InvalidSpecificationArrayException::becauseFilterPriorityMustBeAnInteger($priority);
+        }
+    }
+
+    private static function validateCallbackList(mixed $spec): void
+    {
+        if (! is_array($spec)) {
+            throw InvalidSpecificationArrayException::becauseCallbackListMustBeAList($spec);
+        }
+
+        /** @psalm-var mixed $item */
+        foreach ($spec as $item) {
+            self::validateCallback($item);
+        }
+    }
+
+    private static function validateCallback(mixed $spec): void
+    {
+        if (! is_array($spec)) {
+            throw InvalidSpecificationArrayException::becauseCallbackMustBeArray($spec);
+        }
+
+        $callback = $spec['callback'] ?? null;
+        $priority = $spec['priority'] ?? null;
+
+        if (! is_callable($callback)) {
+            throw InvalidSpecificationArrayException::becauseCallbackMustBePresent($callback);
+        }
+
+        if ($priority !== null && ! is_int($priority)) {
+            throw InvalidSpecificationArrayException::becauseFilterPriorityMustBeAnInteger($priority);
+        }
+    }
 }

src/FilterChainFactory.php

@@ -15,7 +15,10 @@ final class FilterChainFactory implements FactoryInterface
     public function __invoke(ContainerInterface $container, string $requestedName, ?array $options = null): FilterChain
     {
         /**
-         * It's not worth attempting runtime validation of the specification shape
+         * Runtime validation of the chain spec can be done but is not because it would introduce a BC break
+         *
+         * @see FilterChain::validateSpecification()
+         *
          * @psalm-var FilterChainConfiguration $options
          */
         $options       = $options ?? [];

test/FilterChainTest.php

@@ -4,14 +4,18 @@
 
 namespace LaminasTest\Filter;
 
+use Laminas\Filter\Exception\InvalidSpecificationArrayException;
 use Laminas\Filter\FilterChain;
 use Laminas\Filter\FilterPluginManager;
 use Laminas\Filter\PregReplace;
+use Laminas\Filter\StringPrefix;
 use Laminas\Filter\StringToLower;
 use Laminas\Filter\StringTrim;
 use Laminas\Filter\StripTags;
+use Laminas\Filter\ToInt;
 use Laminas\ServiceManager\ServiceManager;
 use LaminasTest\Filter\TestAsset\StrRepeatFilterInterface;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\TestCase;
 
@@ -265,4 +269,109 @@ public function testServiceManagerServicesCanBeUsedInChains(): void
         self::assertCount(2, $chain);
         self::assertSame($closure, $chain[1]);
     }
+
+    /** @return array<string, array{0: array, 1: string}> */
+    public static function invalidSpecProvider(): array
+    {
+        return [
+            'Filters string'            => [
+                ['filters' => 'foo'],
+                'The `filters` key must be a list of arrays or filter instances',
+            ],
+            'Filter not array'          => [
+                ['filters' => ['foo']],
+                'Each member of the `filters` list must be array specification',
+            ],
+            'Filter name missing'       => [
+                ['filters' => [['missing name']]],
+                'Individual filter array specifications must have the key `name`',
+            ],
+            'Filter name empty'         => [
+                ['filters' => [['name' => '']]],
+                'Individual filter array specifications must have the key `name`',
+            ],
+            'Filter name null'          => [
+                ['filters' => [['name' => null]]],
+                'Individual filter array specifications must have the key `name`',
+            ],
+            'Filter name non-string'    => [
+                ['filters' => [['name' => 1]]],
+                'Individual filter array specifications must have the key `name`',
+            ],
+            'Filter options non-array'  => [
+                ['filters' => [['name' => 'foo', 'options' => 1]]],
+                'Filter options must be an array when specified',
+            ],
+            'Filter priority non-int'   => [
+                ['filters' => [['name' => 'foo', 'priority' => 'banana']]],
+                'Filter priorities must be integers when specified',
+            ],
+            'Callbacks not array'       => [
+                ['callbacks' => 'foo'],
+                'The `callbacks` key must be a list of arrays',
+            ],
+            'Callback not array'        => [
+                ['callbacks' => ['foo']],
+                'All items listed under the `callbacks` key must be arrays',
+            ],
+            'Callback missing callback' => [
+                ['callbacks' => [['missing required key']]],
+                'must contain a callable under the key `callback`',
+            ],
+            'Callback not callable'     => [
+                ['callbacks' => [['callback' => 'foo']]],
+                'must contain a callable under the key `callback`',
+            ],
+            'Callback priority non-int' => [
+                ['callbacks' => [['callback' => static fn () => null, 'priority' => 'banana']]],
+                'Filter priorities must be integers when specified',
+            ],
+        ];
+    }
+
+    #[DataProvider('invalidSpecProvider')]
+    public function testSpecificationValidationWithInvalidSpecs(array $spec, string $expectMessage): void
+    {
+        $this->expectException(InvalidSpecificationArrayException::class);
+        $this->expectExceptionMessage($expectMessage);
+        FilterChain::validateSpecification($spec);
+    }
+
+    /** @return array<string, array{0: array}> */
+    public static function validSpecProvider(): array
+    {
+        return [
+            'Empty'              => [[]],
+            'Empty Filters'      => [['filters' => []]],
+            'Empty Callbacks'    => [['callbacks' => []]],
+            'Full Specification' => [
+                [
+                    'filters'   => [
+                        new StringToLower(),
+                        [
+                            'name' => ToInt::class,
+                        ],
+                        [
+                            'name'     => StringPrefix::class,
+                            'options'  => ['prefix' => 'Foo'],
+                            'priority' => 10,
+                        ],
+                    ],
+                    'callbacks' => [
+                        [
+                            'callback' => static fn () => null,
+                            'priority' => 9,
+                        ],
+                    ],
+                ],
+            ],
+        ];
+    }
+
+    #[DataProvider('validSpecProvider')]
+    public function testValidSpecifications(array $spec): void
+    {
+        $this->expectNotToPerformAssertions();
+        FilterChain::validateSpecification($spec);
+    }
 }