Restrict CallableStringifier to closures to prevent data leakage

henriquemoody · henriquemoody · commit 676f823920b4 · 2026-01-19T01:55:49.000+01:00
The `CallableStringifier` is undeniably useful, but its convenience comes at a
security risk. By allowing strings and arrays to be interpreted as callables
by default, we risk exposing sensitive data—like passwords or hostnames—hidden
within those structures. Furthermore, it is often frustrating to have arbitrary
strings automatically turned into callable representations.

To fix this, I am moving to a "secure-by-default" stance. The stringifier now
defaults to closure-only mode. Closures are significantly less likely to expose
sensitive data in their contracts, making this a much safer baseline.

I have made an exception for Fibers. Since a Fiber explicitly contains a
callable, we know the usage is intentional, and being explicit about the
underlying callable is important for debugging clarity.

If someone still needs the original behavior, they can have it, but they must be
intentional. They can restore the previous functionality by manually defining
the `$closureOnly` parameter as `false` in the constructor.
diff --git a/README.md b/README.md
@@ -103,23 +103,8 @@ echo stringify(new class { public int $property = 42; }) . PHP_EOL;
 echo stringify(new class extends WithProperties { }) . PHP_EOL;
 // `WithProperties@anonymous { +$publicProperty=true #$protectedProperty=42 }`
 
-echo stringify('chr') . PHP_EOL;
-// `chr(int $codepoint): string`
-
-echo stringify([new WithMethods(), 'publicMethod']) . PHP_EOL;
-// `WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
-
-echo stringify('WithMethods::publicStaticMethod') . PHP_EOL;
-// `WithMethods::publicStaticMethod(int|float $parameter): void`
-
-echo stringify(['WithMethods', 'publicStaticMethod']) . PHP_EOL;
-// `WithMethods::publicStaticMethod(int|float $parameter): void`
-
-echo stringify(new WithInvoke()) . PHP_EOL;
-// `WithInvoke->__invoke(int $parameter = 0): never`
-
-echo stringify(static fn(int $foo): string => '') . PHP_EOL;
-// `function (int $foo): string`
+echo stringify(fn(int $foo): string => '') . PHP_EOL;
+// `Closure { fn(int $foo): string }`
 
 echo stringify(new DateTime()) . PHP_EOL;
 // `DateTime { 2023-04-21T11:29:03+00:00 }`
diff --git a/phpcs.xml.dist b/phpcs.xml.dist
@@ -32,4 +32,8 @@
     <rule ref="Generic.PHP.CharacterBeforePHPOpeningTag.Found">
         <exclude-pattern>tests/integration/</exclude-pattern>
     </rule>
+    <rule ref="SlevomatCodingStandard.Functions.StaticClosure.ClosureNotStatic">
+        <exclude-pattern>tests/integration</exclude-pattern>
+        <exclude-pattern>tests/unit/Stringifiers/CallableStringifierTest.php</exclude-pattern>
+    </rule>
 </ruleset>
diff --git a/src/Stringifiers/CallableStringifier.php b/src/Stringifiers/CallableStringifier.php
@@ -44,43 +44,44 @@ final class CallableStringifier implements Stringifier
     public function __construct(
         private readonly Stringifier $stringifier,
         private readonly Quoter $quoter,
+        private readonly bool $closureOnly = true,
     ) {
     }
 
     public function stringify(mixed $raw, int $depth): string|null
     {
-        if (!is_callable($raw)) {
-            return null;
-        }
-
         if ($raw instanceof Closure) {
             return $this->buildFunction(new ReflectionFunction($raw), $depth);
         }
 
+        if ($this->closureOnly || !is_callable($raw)) {
+            return null;
+        }
+
         if (is_object($raw)) {
             return $this->buildMethod(new ReflectionMethod($raw, '__invoke'), $raw, $depth);
         }
 
-        if (is_array($raw) && is_object($raw[0]) && is_string($raw[1])) {
+        if (is_array($raw) && isset($raw[0], $raw[1]) && is_object($raw[0]) && is_string($raw[1])) {
             return $this->buildMethod(new ReflectionMethod($raw[0], $raw[1]), $raw[0], $depth);
         }
 
-        if (is_array($raw) && is_string($raw[0]) && is_string($raw[1])) {
+        if (is_array($raw) && isset($raw[0], $raw[1]) && is_string($raw[0]) && is_string($raw[1])) {
             return $this->buildStaticMethod(new ReflectionMethod($raw[0], $raw[1]), $depth);
         }
 
-        if (is_string($raw) && str_contains($raw, ':')) {
+        if (!is_string($raw)) {
+            return null;
+        }
+
+        if (str_contains($raw, ':')) {
             /** @var class-string $class */
             $class = (string) strstr($raw, ':', true);
             $method = substr((string) strrchr($raw, ':'), 1);
 
             return $this->buildStaticMethod(new ReflectionMethod($class, $method), $depth);
         }
 
-        if (!is_string($raw)) {
-            return null;
-        }
-
         return $this->buildFunction(new ReflectionFunction($raw), $depth);
     }
 
@@ -107,8 +108,7 @@ private function buildStaticMethod(ReflectionMethod $reflection, int $depth): st
 
     private function buildSignature(ReflectionFunctionAbstract $function, int $depth): string
     {
-        $signature = $function->isClosure() ? 'function ' : $function->getName();
-        $signature .= sprintf(
+        $signature = sprintf(
             '(%s)',
             implode(
                 ', ',
@@ -138,7 +138,11 @@ private function buildSignature(ReflectionFunctionAbstract $function, int $depth
             $signature .= ': ' . $this->buildType($returnType, $depth);
         }
 
-        return $signature;
+        if ($function->isClosure()) {
+            return sprintf('Closure { %sfn%s }', $function->isStatic() ? 'static ' : '', $signature);
+        }
+
+        return $function->getName() . $signature;
     }
 
     private function buildParameter(ReflectionParameter $reflectionParameter, int $depth): string
diff --git a/src/Stringifiers/CompositeStringifier.php b/src/Stringifiers/CompositeStringifier.php
@@ -60,8 +60,10 @@ public static function createDefault(): self
                 self::MAXIMUM_NUMBER_OF_PROPERTIES,
             ),
         );
-        $stringifier->prependStringifier($callableStringifier = new CallableStringifier($stringifier, $quoter));
-        $stringifier->prependStringifier(new FiberObjectStringifier($callableStringifier, $quoter));
+        $stringifier->prependStringifier(new CallableStringifier($stringifier, $quoter));
+        $stringifier->prependStringifier(
+            new FiberObjectStringifier(new CallableStringifier($stringifier, $quoter, closureOnly: false), $quoter),
+        );
         $stringifier->prependStringifier(new EnumerationStringifier($quoter));
         $stringifier->prependStringifier(new ObjectWithDebugInfoStringifier($arrayStringifier, $quoter));
         $stringifier->prependStringifier(new ArrayObjectStringifier($arrayStringifier, $quoter));
diff --git a/tests/integration/stringify-callable.phpt b/tests/integration/stringify-callable.phpt
@@ -5,25 +5,15 @@ declare(strict_types=1);
 
 require 'vendor/autoload.php';
 
-$variable = new WithInvoke();
+$variable = true;
 
 outputMultiple(
-    'chr',
-    $variable,
-    [new WithMethods(), 'publicMethod'],
-    'WithMethods::publicStaticMethod',
-    ['WithMethods', 'publicStaticMethod'],
-    static fn(int $foo): bool => (bool) $foo,
+    fn(int $foo): bool => (bool) $foo,
     static function (int $foo) use ($variable): string {
         return $variable::class;
     },
 );
 ?>
 --EXPECT--
-`chr(int $codepoint): string`
-`WithInvoke->__invoke(int $parameter = 0): never`
-`WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
-`WithMethods::publicStaticMethod(int|float $parameter): void`
-`WithMethods::publicStaticMethod(int|float $parameter): void`
-`function (int $foo): bool`
-`function (int $foo) use ($variable): string`
+`Closure { fn(int $foo): bool }`
+`Closure { static fn(int $foo) use ($variable): string }`
diff --git a/tests/unit/Stringifiers/CallableStringifierTest.php b/tests/unit/Stringifiers/CallableStringifierTest.php
@@ -32,14 +32,14 @@ final class CallableStringifierTest extends TestCase
     #[Test]
     public function itShouldNotStringifyWhenRawValueIsNotCallable(): void
     {
-        $sut = new CallableStringifier(new FakeStringifier(), new FakeQuoter());
+        $sut = new CallableStringifier(new FakeStringifier(), new FakeQuoter(), closureOnly: false);
 
         self::assertNull($sut->stringify(1, self::DEPTH));
     }
 
     #[Test]
-    #[DataProvider('callableRawValuesProvider')]
-    public function itShouldStringifyWhenRawValueIsCallable(callable $raw, string $expectedWithoutQuotes): void
+    #[DataProvider('closureRawValuesProvider')]
+    public function itShouldStringifyWhenRawValueIsClosure(callable $raw, string $expectedWithoutQuotes): void
     {
         $quoter = new FakeQuoter();
 
@@ -51,6 +51,31 @@ public function itShouldStringifyWhenRawValueIsCallable(callable $raw, string $e
         self::assertEquals($expected, $actual);
     }
 
+    #[Test]
+    #[DataProvider('nonClosureCallableRawValuesProvider')]
+    public function itShouldNotStringifyNonClosureCallableByDefault(callable $raw, string $useless): void
+    {
+        $sut = new CallableStringifier(new FakeStringifier(), new FakeQuoter());
+
+        self::assertNull($sut->stringify($raw, self::DEPTH));
+    }
+
+    #[Test]
+    #[DataProvider('nonClosureCallableRawValuesProvider')]
+    public function itShouldStringifyNonClosureCallableWhenClosureOnlyIsFalse(
+        callable $raw,
+        string $expectedWithoutQuotes,
+    ): void {
+        $quoter = new FakeQuoter();
+
+        $sut = new CallableStringifier(new FakeStringifier(), $quoter, closureOnly: false);
+
+        $actual = $sut->stringify($raw, self::DEPTH);
+        $expected = $quoter->quote($expectedWithoutQuotes, self::DEPTH);
+
+        self::assertEquals($expected, $actual);
+    }
+
     #[Test]
     public function itShouldStringifyWhenRawValueIsCallableWithDefaultValues(): void
     {
@@ -63,7 +88,7 @@ public function itShouldStringifyWhenRawValueIsCallableWithDefaultValues(): void
 
         $actual = $sut->stringify($raw, self::DEPTH);
         $expected = $quoter->quote(
-            sprintf('function (int $value = %s): int', $stringifier->stringify(1, self::DEPTH + 1)),
+            sprintf('Closure { static fn(int $value = %s): int }', $stringifier->stringify(1, self::DEPTH + 1)),
             self::DEPTH,
         );
 
@@ -77,7 +102,7 @@ public function itShouldStringifyWhenRawValueIsCallableThatDoesNotHaveAnAccessib
 
         $quoter = new FakeQuoter();
 
-        $sut = new CallableStringifier(new FakeStringifier(), $quoter);
+        $sut = new CallableStringifier(new FakeStringifier(), $quoter, closureOnly: false);
 
         $actual = $sut->stringify($raw, self::DEPTH);
         $expected = $quoter->quote(
@@ -88,40 +113,87 @@ public function itShouldStringifyWhenRawValueIsCallableThatDoesNotHaveAnAccessib
         self::assertEquals($expected, $actual);
     }
 
-    /** @return array<int, array{0: callable, 1: string}> */
-    public static function callableRawValuesProvider(): array
+    /** @return array<string, array{0: callable, 1: string}> */
+    public static function closureRawValuesProvider(): array
     {
         $var1 = 1;
         $var2 = 2;
 
         return [
-            [static fn() => 1, 'function ()'],
-            [static fn(): int => 1, 'function (): int'],
-            [static fn(float $value): int => (int) $value, 'function (float $value): int'],
-            [static fn(float &$value): int => (int) $value, 'function (float &$value): int'],
-            // phpcs:ignore SlevomatCodingStandard.TypeHints.DNFTypeHintFormat
-            [static fn(?float $value): int => (int) $value, 'function (?float $value): int'],
-            [static fn(int $value = self::DEPTH): int => $value, 'function (int $value = self::DEPTH): int'],
-            [static fn(int|float $value): int => (int) $value, 'function (int|float $value): int'],
-            [static fn(Countable&Iterator $value): int => $value->count(), 'function (Countable&Iterator $value): int'],
-            [static fn(int ...$value): int => array_sum($value), 'function (int ...$value): int'],
-            [
+            'static closure without parameters' => [
+                static fn() => 1,
+                'Closure { static fn() }',
+            ],
+            'non-static closure without parameters' => [
+                fn() => 1,
+                'Closure { fn() }',
+            ],
+            'static closure with return type' => [
+                static fn(): int => 1,
+                'Closure { static fn(): int }',
+            ],
+            'non-static closure with return type' => [
+                fn(): int => 1,
+                'Closure { fn(): int }',
+            ],
+            'static closure with typed parameter' => [
+                static fn(float $value): int => (int) $value,
+                'Closure { static fn(float $value): int }',
+            ],
+            'static closure with reference parameter' => [
+                static fn(float &$value): int => (int) $value,
+                'Closure { static fn(float &$value): int }',
+            ],
+            'static closure with nullable parameter' => [
+                static fn(float|null $value): int => (int) $value,
+                'Closure { static fn(?float $value): int }',
+            ],
+            'static closure with constant default value' => [
+                static fn(int $value = self::DEPTH): int => $value,
+                'Closure { static fn(int $value = self::DEPTH): int }',
+            ],
+            'static closure with union type parameter' => [
+                static fn(int|float $value): int => (int) $value,
+                'Closure { static fn(int|float $value): int }',
+            ],
+            'static closure with intersection type parameter' => [
+                static fn(Countable&Iterator $value): int => $value->count(),
+                'Closure { static fn(Countable&Iterator $value): int }',
+            ],
+            'static closure with variadic parameter' => [
+                static fn(int ...$value): int => array_sum($value),
+                'Closure { static fn(int ...$value): int }',
+            ],
+            'static closure with multiple parameters' => [
                 static fn(float $value1, float $value2): float => $value1 + $value2,
-                'function (float $value1, float $value2): float',
+                'Closure { static fn(float $value1, float $value2): float }',
             ],
-            [
+            'static closure with single use variable' => [
                 static function (int $value) use ($var1): int {
                     return $value + $var1;
                 },
-                'function (int $value) use ($var1): int',
+                'Closure { static fn(int $value) use ($var1): int }',
             ],
-            [
+            'static closure with multiple use variables' => [
                 static function (int $value) use ($var1, $var2): int {
                     return $value + $var1 + $var2;
                 },
-                'function (int $value) use ($var1, $var2): int',
+                'Closure { static fn(int $value) use ($var1, $var2): int }',
             ],
-            [
+            'non-static closure with use variable' => [
+                function (int $value) use ($var1): int {
+                    return $value + $var1;
+                },
+                'Closure { fn(int $value) use ($var1): int }',
+            ],
+        ];
+    }
+
+    /** @return array<string, array{0: callable, 1: string}> */
+    public static function nonClosureCallableRawValuesProvider(): array
+    {
+        return [
+            'invokable object' => [
                 new class {
                     public function __invoke(int $parameter): never
                     {
@@ -130,19 +202,22 @@ public function __invoke(int $parameter): never
                 },
                 'class->__invoke(int $parameter): never',
             ],
-            [
+            'object method as array' => [
                 [new DateTime(), 'format'],
                 'DateTime->format(string $format)',
             ],
-            [
+            'static method as array' => [
                 ['DateTime', 'createFromImmutable'],
                 'DateTime::createFromImmutable(DateTimeImmutable $object)',
             ],
-            [
+            'static method as string' => [
                 'DateTimeImmutable::getLastErrors',
                 'DateTimeImmutable::getLastErrors()',
             ],
-            ['chr', 'chr(int $codepoint): string'],
+            'function name as string' => [
+                'chr',
+                'chr(int $codepoint): string',
+            ],
         ];
     }
 }
