Skip to content

Commit f3b0fd8

Browse files
VincentLangletVincentLanglet
committed
Report invalid printf placeholder
1 parent acf7f97 commit f3b0fd8

File tree

9 files changed

+88
-14
lines changed

9 files changed

+88
-14
lines changed

‎src/Rules/Functions/PrintfArrayParametersRule.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,17 @@ public function processNode(Node $node, Scope $scope): array
6666
foreach ($formatArgType->getConstantStrings() as $formatString) {
6767
$format = $formatString->getValue();
6868

69-
$placeHoldersCounts[] = $this->printfHelper->getPrintfPlaceholdersCount($format);
69+
$count = $this->printfHelper->getPrintfPlaceholdersCount($format);
70+
if ($count === null) {
71+
return [
72+
RuleErrorBuilder::message(sprintf(
73+
'Call to %s contains an invalid placeholder.',
74+
$name,
75+
))->identifier(sprintf('argument.%s', $name))->build(),
76+
];
77+
}
78+
79+
$placeHoldersCounts[] = $count;
7080
}
7181

7282
if ($placeHoldersCounts === []) {

‎src/Rules/Functions/PrintfHelper.php‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,26 @@ public function __construct(private PhpVersion $phpVersion)
2424
{
2525
}
2626

27-
public function getPrintfPlaceholdersCount(string $format): int
27+
public function getPrintfPlaceholdersCount(string $format): ?int
2828
{
2929
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format);
3030
}
3131

3232
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
33-
public function getPrintfPlaceholders(string $format): array
33+
public function getPrintfPlaceholders(string $format): ?array
3434
{
3535
return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
3636
}
3737

38-
public function getScanfPlaceholdersCount(string $format): int
38+
public function getScanfPlaceholdersCount(string $format): ?int
3939
{
4040
return $this->getPlaceholdersCount('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format);
4141
}
4242

43-
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
44-
private function parsePlaceholders(string $specifiersPattern, string $format): array
43+
/**
44+
* @phpstan-return array<int, non-empty-list<PrintfPlaceholder>>|null parameter index => placeholders
45+
*/
46+
private function parsePlaceholders(string $specifiersPattern, string $format): ?array
4547
{
4648
$addSpecifier = '';
4749
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
@@ -50,7 +52,7 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a
5052

5153
$specifiers = sprintf($specifiersPattern, $addSpecifier);
5254

53-
$pattern = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $specifiers . '~';
55+
$pattern = '~(?<before>%*)%(?:(?<position>\d+)\$)?[-+]?(?:[ 0]|(?:\'[^%]))?(?<width>\*)?-?\d*(?:\.(?:\d+|(?<precision>\*))?)?' . $specifiers . '?~';
5456

5557
$matches = Strings::matchAll($format, $pattern, PREG_SET_ORDER);
5658

@@ -89,13 +91,19 @@ private function parsePlaceholders(string $specifiersPattern, string $format): a
8991
$showValueSuffix = true;
9092
}
9193

94+
$specifier = $placeholder['specifier'] ?? '';
95+
if ($specifier === '') {
96+
// A placeholder is invalid.
97+
return null;
98+
}
99+
92100
$parsedPlaceholders[] = new PrintfPlaceholder(
93101
sprintf('"%s"', $placeholder[0]) . ($showValueSuffix ? ' (value)' : ''),
94102
isset($placeholder['position']) && $placeholder['position'] !== ''
95103
? $placeholder['position'] - 1
96104
: $parameterIdx++,
97105
$placeholderNumber,
98-
$this->getAcceptingTypeBySpecifier($placeholder['specifier'] ?? ''),
106+
$this->getAcceptingTypeBySpecifier($specifier),
99107
);
100108
}
101109

@@ -124,9 +132,14 @@ private function getAcceptingTypeBySpecifier(string $specifier): string
124132
return 'mixed';
125133
}
126134

127-
private function getPlaceholdersCount(string $specifiersPattern, string $format): int
135+
private function getPlaceholdersCount(string $specifiersPattern, string $format): ?int
128136
{
129-
$paramIndices = array_keys($this->parsePlaceholders($specifiersPattern, $format));
137+
$placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format);
138+
if ($placeholdersMap === null) {
139+
return null;
140+
}
141+
142+
$paramIndices = array_keys($placeholdersMap);
130143

131144
return $paramIndices === []
132145
? 0

‎src/Rules/Functions/PrintfParameterTypeRule.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ public function processNode(Node $node, Scope $scope): array
9292
$formatString = $formatArgTypeStrings[0];
9393
$format = $formatString->getValue();
9494
$placeholderMap = $this->printfHelper->getPrintfPlaceholders($format);
95+
if ($placeholderMap === null) {
96+
// Already reported by PrintfParametersRule.
97+
return [];
98+
}
99+
95100
$errors = [];
96101
$typeAllowedByCallToFunctionParametersRule = TypeCombinator::union(
97102
new StringAlwaysAcceptingObjectWithToStringType(),

‎src/Rules/Functions/PrintfParametersRule.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,15 @@ public function processNode(Node $node, Scope $scope): array
8686
$tempPlaceHoldersCount = $this->printfHelper->getScanfPlaceholdersCount($format);
8787
}
8888

89+
if ($tempPlaceHoldersCount === null) {
90+
return [
91+
RuleErrorBuilder::message(sprintf(
92+
'Call to %s contains an invalid placeholder.',
93+
$name,
94+
))->identifier(sprintf('argument.%s', $name))->build(),
95+
];
96+
}
97+
8998
if ($maxPlaceHoldersCount === null) {
9099
$maxPlaceHoldersCount = $tempPlaceHoldersCount;
91100
} elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) {

‎tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,4 +71,18 @@ public function testFile(): void
7171
]);
7272
}
7373

74+
public function testBug1889(): void
75+
{
76+
$this->analyse([__DIR__ . '/data/bug-1889.php'], [
77+
[
78+
'Call to vsprintf contains an invalid placeholder.',
79+
7,
80+
],
81+
[
82+
'Call to vsprintf contains an invalid placeholder.',
83+
9,
84+
],
85+
]);
86+
}
87+
7488
}

‎tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,18 @@ public function testBug2342(): void
123123
]);
124124
}
125125

126+
public function testBug1889(): void
127+
{
128+
$this->analyse([__DIR__ . '/data/bug-1889.php'], [
129+
[
130+
'Call to printf contains an invalid placeholder.',
131+
3,
132+
],
133+
[
134+
'Call to printf contains an invalid placeholder.',
135+
5,
136+
],
137+
]);
138+
}
139+
126140
}

‎tests/PHPStan/Rules/Functions/data/bug-1889.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
printf('%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%', 71, 73, 70, 56, 57, 97, 1, 0, 1, 0, 128, 255, 0, 192, 192, 192, 0, 0, 0, 33, 249, 4, 1, 0, 0, 0, 0, 44, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 68, 1, 0, 59);
4+
5+
printf('%c%c%c%', 71, 73, 70, 80);
6+
7+
vsprintf('%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%', [71, 73, 70, 56, 57, 97, 1, 0, 1, 0, 128, 255, 0, 192, 192, 192, 0, 0, 0, 33, 249, 4, 1, 0, 0, 0, 0, 44, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 68, 1, 0, 59]);
8+
9+
vsprintf('%c%c%c%', [71, 73, 70, 80]);

‎tests/PHPStan/Rules/Functions/data/printf.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
sprintf('%s %s', 'foo'); // one parameter missing
77
sprintf('foo', 'foo'); // one parameter over
88
sprintf('foo %s', 'foo', 'bar'); // one parameter over
9-
sprintf('%2$s %1$s %% %1$s %%%', 'one'); // one parameter missing
9+
sprintf('%2$s %1$s %% %1$s %%', 'one'); // one parameter missing
1010
sprintf('%2$s %%'); // two parameters required
1111
sprintf('%2$s %1$s %1$s %s %s %s %s'); // four parameters required
12-
sprintf('%2$s %1$s %% %s %s %s %s %%% %%%%', 'one', 'two', 'three', 'four'); // ok
12+
sprintf('%2$s %1$s %% %s %s %s %s %% %%%%', 'one', 'two', 'three', 'four'); // ok
1313
sprintf("%'.9d %1$'.9d %0.3f %d %d %d", 123, 456);
1414
sprintf('%-4s', 'foo'); // ok
1515
sprintf('%%s %s', 'foo', 'bar'); // one parameter over

‎tests/PHPStan/Rules/Functions/data/vprintf.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ function doFoo($message, array $arr) {
1010
vsprintf('%s %s', ['foo']); // one parameter missing
1111
vsprintf('foo', ['foo']); // one parameter over
1212
vsprintf('foo %s', ['foo', 'bar']); // one parameter over
13-
vsprintf('%2$s %1$s %% %1$s %%%', ['one']); // one parameter missing
13+
vsprintf('%2$s %1$s %% %1$s %%', ['one']); // one parameter missing
1414
vsprintf('%2$s %%'); // two parameters required
1515
vsprintf('%2$s %%', []); // two parameters required
1616
vsprintf('%2$s %1$s %1$s %s %s %s %s'); // four parameters required
17-
vsprintf('%2$s %1$s %% %s %s %s %s %%% %%%%', ['one', 'two', 'three', 'four']); // ok
17+
vsprintf('%2$s %1$s %% %s %s %s %s %% %%%%', ['one', 'two', 'three', 'four']); // ok
1818
vsprintf("%'.9d %1$'.9d %0.3f %d %d %d", [123, 456]); // five parameters required
1919

2020
vsprintf('%-4s', ['foo']); // ok

0 commit comments

Comments
 (0)