Skip to content

Commit baaf520

Browse files
committed
Restrict CallableStringifier to closures to prevent data leakage
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.
1 parent 6d38b05 commit baaf520

File tree

6 files changed

+144
-85
lines changed

6 files changed

+144
-85
lines changed

README.md

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,8 @@ echo stringify(new class { public int $property = 42; }) . PHP_EOL;
103103
echo stringify(new class extends WithProperties { }) . PHP_EOL;
104104
// `WithProperties@anonymous { +$publicProperty=true #$protectedProperty=42 }`
105105

106-
echo stringify('chr') . PHP_EOL;
107-
// `chr(int $codepoint): string`
108-
109-
echo stringify([new WithMethods(), 'publicMethod']) . PHP_EOL;
110-
// `WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
111-
112-
echo stringify('WithMethods::publicStaticMethod') . PHP_EOL;
113-
// `WithMethods::publicStaticMethod(int|float $parameter): void`
114-
115-
echo stringify(['WithMethods', 'publicStaticMethod']) . PHP_EOL;
116-
// `WithMethods::publicStaticMethod(int|float $parameter): void`
117-
118-
echo stringify(new WithInvoke()) . PHP_EOL;
119-
// `WithInvoke->__invoke(int $parameter = 0): never`
120-
121106
echo stringify(static fn(int $foo): string => '') . PHP_EOL;
122-
// `function (int $foo): string`
107+
// `Closure { static fn (int $foo): string }`
123108

124109
echo stringify(new DateTime()) . PHP_EOL;
125110
// `DateTime { 2023-04-21T11:29:03+00:00 }`

phpcs.xml.dist

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,8 @@
3232
<rule ref="Generic.PHP.CharacterBeforePHPOpeningTag.Found">
3333
<exclude-pattern>tests/integration/</exclude-pattern>
3434
</rule>
35+
<rule ref="SlevomatCodingStandard.Functions.StaticClosure.ClosureNotStatic">
36+
<exclude-pattern>tests/integration</exclude-pattern>
37+
<exclude-pattern>tests/unit/Stringifiers/CallableStringifierTest.php</exclude-pattern>
38+
</rule>
3539
</ruleset>

src/Stringifiers/CallableStringifier.php

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
use function array_keys;
2727
use function array_map;
28+
use function assert;
2829
use function count;
2930
use function implode;
3031
use function is_array;
@@ -44,43 +45,47 @@ final class CallableStringifier implements Stringifier
4445
public function __construct(
4546
private readonly Stringifier $stringifier,
4647
private readonly Quoter $quoter,
48+
private readonly bool $closureOnly = true,
4749
) {
4850
}
4951

5052
public function stringify(mixed $raw, int $depth): string|null
5153
{
52-
if (!is_callable($raw)) {
53-
return null;
54-
}
55-
5654
if ($raw instanceof Closure) {
5755
return $this->buildFunction(new ReflectionFunction($raw), $depth);
5856
}
5957

58+
if ($this->closureOnly || !is_callable($raw)) {
59+
return null;
60+
}
61+
6062
if (is_object($raw)) {
6163
return $this->buildMethod(new ReflectionMethod($raw, '__invoke'), $raw, $depth);
6264
}
6365

64-
if (is_array($raw) && is_object($raw[0]) && is_string($raw[1])) {
65-
return $this->buildMethod(new ReflectionMethod($raw[0], $raw[1]), $raw[0], $depth);
66+
if (is_array($raw) && is_object($raw[0])) {
67+
$method = $raw[1];
68+
assert(is_string($method));
69+
70+
return $this->buildMethod(new ReflectionMethod($raw[0], $method), $raw[0], $depth);
6671
}
6772

68-
if (is_array($raw) && is_string($raw[0]) && is_string($raw[1])) {
69-
return $this->buildStaticMethod(new ReflectionMethod($raw[0], $raw[1]), $depth);
73+
if (is_array($raw) && is_string($raw[0])) {
74+
$method = $raw[1];
75+
assert(is_string($method));
76+
77+
return $this->buildStaticMethod(new ReflectionMethod($raw[0], $method), $depth);
7078
}
7179

72-
if (is_string($raw) && str_contains($raw, ':')) {
80+
/** @var callable-string $raw */
81+
if (str_contains($raw, ':')) {
7382
/** @var class-string $class */
7483
$class = (string) strstr($raw, ':', true);
7584
$method = substr((string) strrchr($raw, ':'), 1);
7685

7786
return $this->buildStaticMethod(new ReflectionMethod($class, $method), $depth);
7887
}
7988

80-
if (!is_string($raw)) {
81-
return null;
82-
}
83-
8489
return $this->buildFunction(new ReflectionFunction($raw), $depth);
8590
}
8691

@@ -107,8 +112,7 @@ private function buildStaticMethod(ReflectionMethod $reflection, int $depth): st
107112

108113
private function buildSignature(ReflectionFunctionAbstract $function, int $depth): string
109114
{
110-
$signature = $function->isClosure() ? 'function ' : $function->getName();
111-
$signature .= sprintf(
115+
$signature = sprintf(
112116
'(%s)',
113117
implode(
114118
', ',
@@ -138,7 +142,11 @@ private function buildSignature(ReflectionFunctionAbstract $function, int $depth
138142
$signature .= ': ' . $this->buildType($returnType, $depth);
139143
}
140144

141-
return $signature;
145+
if ($function->isClosure()) {
146+
return sprintf('Closure { %sfn%s }', $function->isStatic() ? 'static ' : '', $signature);
147+
}
148+
149+
return $function->getName() . $signature;
142150
}
143151

144152
private function buildParameter(ReflectionParameter $reflectionParameter, int $depth): string
@@ -192,15 +200,8 @@ private function buildType(ReflectionType $raw, int $depth): string
192200
);
193201
}
194202

195-
if ($raw instanceof ReflectionNamedType) {
196-
$type = $raw->getName();
197-
if ($raw->allowsNull()) {
198-
$type = sprintf('?%s', $type);
199-
}
200-
201-
return $type;
202-
}
203+
/** @var ReflectionNamedType $raw */
203204

204-
return '';
205+
return ($raw->allowsNull() ? '?' : '') . $raw->getName();
205206
}
206207
}

src/Stringifiers/CompositeStringifier.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ public static function createDefault(): self
6060
self::MAXIMUM_NUMBER_OF_PROPERTIES,
6161
),
6262
);
63-
$stringifier->prependStringifier($callableStringifier = new CallableStringifier($stringifier, $quoter));
64-
$stringifier->prependStringifier(new FiberObjectStringifier($callableStringifier, $quoter));
63+
$stringifier->prependStringifier(new CallableStringifier($stringifier, $quoter));
64+
$stringifier->prependStringifier(
65+
new FiberObjectStringifier(new CallableStringifier($stringifier, $quoter, closureOnly: false), $quoter),
66+
);
6567
$stringifier->prependStringifier(new EnumerationStringifier($quoter));
6668
$stringifier->prependStringifier(new ObjectWithDebugInfoStringifier($arrayStringifier, $quoter));
6769
$stringifier->prependStringifier(new ArrayObjectStringifier($arrayStringifier, $quoter));

tests/integration/stringify-callable.phpt

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,15 @@ declare(strict_types=1);
55

66
require 'vendor/autoload.php';
77

8-
$variable = new WithInvoke();
8+
$variable = true;
99

1010
outputMultiple(
11-
'chr',
12-
$variable,
13-
[new WithMethods(), 'publicMethod'],
14-
'WithMethods::publicStaticMethod',
15-
['WithMethods', 'publicStaticMethod'],
16-
static fn(int $foo): bool => (bool) $foo,
11+
fn(int $foo): bool => (bool) $foo,
1712
static function (int $foo) use ($variable): string {
1813
return $variable::class;
1914
},
2015
);
2116
?>
2217
--EXPECT--
23-
`chr(int $codepoint): string`
24-
`WithInvoke->__invoke(int $parameter = 0): never`
25-
`WithMethods->publicMethod(Iterator&Countable $parameter): ?static`
26-
`WithMethods::publicStaticMethod(int|float $parameter): void`
27-
`WithMethods::publicStaticMethod(int|float $parameter): void`
28-
`function (int $foo): bool`
29-
`function (int $foo) use ($variable): string`
18+
`Closure { fn(int $foo): bool }`
19+
`Closure { static fn(int $foo) use ($variable): string }`

0 commit comments

Comments
 (0)