From ee9819a8fde557b5c31fb55cb4213304dc9723a3 Mon Sep 17 00:00:00 2001 From: "Hans Krentel (hakre)" Date: Sun, 3 May 2026 20:02:50 +0200 Subject: [PATCH 1/2] Testdata *scanf(format) bad scan conversion cases Additional by-catch fix of a variable misnomer in 023fa08d5 ("Fix printf parameters rule", 2017-04-02) spotted during review. --- .../PHPStan/Rules/Functions/PrintfParametersRuleTest.php | 8 ++++++++ tests/PHPStan/Rules/Functions/data/printf.php | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index 1e3ab9ddd78..15e8c997dbd 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -84,6 +84,14 @@ public function testFile(): void 'Call to sprintf contains 2 placeholders, 1 value given.', 29, ], + [ + 'Call to sscanf contains an invalid placeholder.', + 38, + ], + [ + 'Call to fscanf contains an invalid placeholder.', + 39, + ], [ 'Call to sprintf contains 2 placeholders, 1 value given.', 45, diff --git a/tests/PHPStan/Rules/Functions/data/printf.php b/tests/PHPStan/Rules/Functions/data/printf.php index b4236303978..136687292b5 100644 --- a/tests/PHPStan/Rules/Functions/data/printf.php +++ b/tests/PHPStan/Rules/Functions/data/printf.php @@ -35,8 +35,8 @@ sscanf($str, "%20[^abcde]a%d", $string, $number); // ok printf("%.E", 3.14159); // ok sprintf("%.E", 3.14159); // ok -sscanf($str, '%.E', $number); // ok -fscanf($str, '%.E', $number); // ok +sscanf($str, '%.E', $number); // bad scan conversion character '.' +fscanf($resource, '%.E', $number); // bad scan conversion character '.' sscanf($str, '%[A-Z]%d', $char, $number); // ok sprintf('%s %s %s', ...[1]); // do not detect unpacked arguments sprintf('%s %s %s', ...[1, 2, 3]); // ok From ac55b3620d8176e81df3c372b63d74804bbf9475 Mon Sep 17 00:00:00 2001 From: hakre Date: Sun, 3 May 2026 17:59:41 +0200 Subject: [PATCH 2/2] Fix counting `*scanf()` format string placeholders Update PrintfHelper.php to make public function getScanfPlaceholdersCount(string $format): ?int` return the sscanf() vetted number of placeholders that return/assign conversions. Addresses long-standing regressions reported originally in [phpstan-10260]. References: - [phpstan-10260] - https://github.com/phpstan/phpstan/issues/10260#issuecomment-4320659650 - [phpstan-src-5591] - [phpstan-14567] - https://3v4l.org/WR85Q [phpstan-10260]: https://github.com/phpstan/phpstan/issues/10260 [phpstan-src-5591]: https://github.com/phpstan/phpstan-src/pull/5591 [phpstan-14567]: https://github.com/phpstan/phpstan/issues/14567 --- src/Rules/Functions/PrintfHelper.php | 30 +++++--- .../Rules/Functions/PrintfHelperTest.php | 72 +++++++++++++++++++ 2 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 tests/PHPStan/Rules/Functions/PrintfHelperTest.php diff --git a/src/Rules/Functions/PrintfHelper.php b/src/Rules/Functions/PrintfHelper.php index 411972885d3..b9443a451b9 100644 --- a/src/Rules/Functions/PrintfHelper.php +++ b/src/Rules/Functions/PrintfHelper.php @@ -5,12 +5,14 @@ use Nette\Utils\Strings; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Php\PhpVersion; +use ValueError; use function array_filter; use function array_keys; use function count; use function in_array; use function max; use function sprintf; +use function sscanf; use function strlen; use const PREG_SET_ORDER; @@ -26,24 +28,36 @@ public function __construct(private PhpVersion $phpVersion) public function getPrintfPlaceholdersCount(string $format): ?int { - return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format, false); + return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format); } /** @phpstan-return array> parameter index => placeholders */ public function getPrintfPlaceholders(string $format): ?array { - return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format, false); + return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format); } public function getScanfPlaceholdersCount(string $format): ?int { - return $this->getPlaceholdersCount('(?[cdDeEfinosuxX%s]|\[[^\]]+\])', $format, true); + if ($this->phpVersion->throwsValueErrorForInternalFunctions()) { + try { + $result = sscanf('', '%*n' . $format); + } catch (ValueError) { + return null; + } + } else { + $result = @sscanf('', '%*n' . $format); + } + if ($result === null) { + return null; + } + return count($result); } /** * @phpstan-return array>|null parameter index => placeholders */ - private function parsePlaceholders(string $specifiersPattern, string $format, bool $isScanf): ?array + private function parsePlaceholders(string $specifiersPattern, string $format): ?array { $addSpecifier = ''; if ($this->phpVersion->supportsHhPrintfSpecifier()) { @@ -72,10 +86,6 @@ private function parsePlaceholders(string $specifiersPattern, string $format, bo $showValueSuffix = false; if (isset($placeholder['width']) && $placeholder['width'] !== '') { - if ($isScanf) { - // In scanf, * means assignment suppression - skip this placeholder entirely - continue; - } $parsedPlaceholders[] = new PrintfPlaceholder( sprintf('"%s" (width)', $placeholder[0]), $parameterIdx++, @@ -136,9 +146,9 @@ private function getAcceptingTypeBySpecifier(string $specifier): string return 'mixed'; } - private function getPlaceholdersCount(string $specifiersPattern, string $format, bool $isScanf): ?int + private function getPlaceholdersCount(string $specifiersPattern, string $format): ?int { - $placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format, $isScanf); + $placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format); if ($placeholdersMap === null) { return null; } diff --git a/tests/PHPStan/Rules/Functions/PrintfHelperTest.php b/tests/PHPStan/Rules/Functions/PrintfHelperTest.php new file mode 100644 index 00000000000..8aa3498ad91 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/PrintfHelperTest.php @@ -0,0 +1,72 @@ +printf = $this->getPhpVersionIdAwareHelper(PHP_VERSION_ID); + } + + #[RequiresPhp('< 8.0.0')] + public function testReturnsNullForInvalidPatternOnLegacyPhpVersion(): void + { + $this->assertNull($this->printf->getScanfPlaceholdersCount('%a')); + } + + #[RequiresPhp('>= 8.0.0')] + public function testReturnsNullForInvalidPatternOnPhp8(): void + { + $this->assertNull($this->printf->getScanfPlaceholdersCount('%a')); + } + + #[RequiresPhp('>= 8.0.0')] + #[DataProvider('dataLegacyVersionIds')] + public function testLegacyVersionStillThrowsValueErrorOnPhp8(int $versionId): void + { + $helper = $this->getPhpVersionIdAwareHelper($versionId); + $this->expectException(ValueError::class); + $this->expectExceptionMessage('Bad scan conversion character "a"'); + $helper->getScanfPlaceholdersCount('%a'); + $this->fail('check your phpunit'); + } + + private function getPhpVersionIdAwareHelper(int $versionId): PrintfHelper + { + return new PrintfHelper($this->getPhpVersion($versionId)); + } + + private function getPhpVersion(int $versionId): PhpVersion + { + return new PhpVersion($versionId); + } + + public static function dataLegacyVersionIds(): array + { + return [ + 'PHP 4.3.14' => [40314], + 'PHP 5.0.20' => [50020], + 'PHP 5.3.23' => [50323], + 'PHP 5.5.36' => [50536], + 'PHP 7.0.16' => [70016], + 'PHP 7.1.0' => [70100], + 'PHP 7.2.22' => [70222], + 'PHP 7.3.0' => [70300], + 'PHP 7.4.33' => [70433], + ]; + } + +}