Skip to content

Commit

Permalink
Fix filter_var() narrowing with unknown options
Browse files Browse the repository at this point in the history
  • Loading branch information
@herndlm
herndlm committed Jan 19, 2023
1 parent 39c12da commit 4dd92cd
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 24 deletions.
74 changes: 53 additions & 21 deletions src/Type/Php/FilterVarDynamicReturnTypeExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,21 @@ public function getTypeFromFunctionCall(
$filterValue = $filterType->getValue();
}

$flagsType = isset($functionCall->getArgs()[2]) ? $scope->getType($functionCall->getArgs()[2]->value) : null;
$flagsType = isset($functionCall->getArgs()[2]) ? $scope->getType($functionCall->getArgs()[2]->value) : new ConstantIntegerType(0);
$inputType = $scope->getType($functionCall->getArgs()[0]->value);
$defaultType = $this->hasFlag($this->getConstant('FILTER_NULL_ON_FAILURE'), $flagsType)
? new NullType()
: new ConstantBooleanType(false);

if ($inputType->isScalar()->no() && $inputType->isNull()->no()) {
return $defaultType;
}

$exactType = $this->determineExactType($inputType, $filterValue, $defaultType, $flagsType);
$type = $exactType ?? $this->getFilterTypeMap()[$filterValue] ?? $mixedType;

$options = $flagsType !== null && $this->hasOptions($flagsType)->yes() ? $this->getOptions($flagsType, $filterValue) : [];
$hasOptions = $this->hasOptions($flagsType);
$options = $hasOptions->yes() ? $this->getOptions($flagsType, $filterValue) : [];
$otherTypes = $this->getOtherTypes($flagsType, $options, $defaultType);

if ($inputType->isNonEmptyString()->yes()
Expand All @@ -185,7 +191,7 @@ public function getTypeFromFunctionCall(
}
}

if ($exactType !== null) {
if ($exactType !== null && !$hasOptions->maybe() && ($inputType->equals($type) || !$inputType->isSuperTypeOf($type)->yes())) {
unset($otherTypes['default']);
}

Expand All @@ -202,32 +208,58 @@ public function getTypeFromFunctionCall(

private function determineExactType(Type $in, int $filterValue, Type $defaultType, ?Type $flagsType): ?Type
{
if (($filterValue === $this->getConstant('FILTER_VALIDATE_BOOLEAN') && $in->isBoolean()->yes())
|| ($filterValue === $this->getConstant('FILTER_VALIDATE_INT') && $in->isInteger()->yes())
|| ($filterValue === $this->getConstant('FILTER_VALIDATE_FLOAT') && $in->isFloat()->yes())) {
return $in;
if ($filterValue === $this->getConstant('FILTER_VALIDATE_BOOLEAN')) {
if ($in->isBoolean()->yes()) {
return $in;
}
}

if ($filterValue === $this->getConstant('FILTER_VALIDATE_INT') && $in instanceof ConstantStringType) {
$value = $in->getValue();
$allowOctal = $this->hasFlag($this->getConstant('FILTER_FLAG_ALLOW_OCTAL'), $flagsType);
$allowHex = $this->hasFlag($this->getConstant('FILTER_FLAG_ALLOW_HEX'), $flagsType);
if ($filterValue === $this->getConstant('FILTER_VALIDATE_FLOAT')) {
if ($in->isFloat()->yes()) {
return $in;
}

if ($allowOctal && preg_match('/\A0[oO][0-7]+\z/', $value) === 1) {
$octalValue = octdec($value);
return is_int($octalValue) ? new ConstantIntegerType($octalValue) : $defaultType;
if ($in->isInteger()->yes()) {
return $in->toFloat();
}
}

if ($filterValue === $this->getConstant('FILTER_VALIDATE_INT')) {
if ($in->isFloat()->yes()) {
return $in->toInteger();
}

if ($allowHex && preg_match('/\A0[xX][0-9A-Fa-f]+\z/', $value) === 1) {
$hexValue = hexdec($value);
return is_int($hexValue) ? new ConstantIntegerType($hexValue) : $defaultType;
if ($in->isInteger()->yes()) {
return $in;
}

return preg_match('/\A[+-]?(?:0|[1-9][0-9]*)\z/', $value) === 1 ? $in->toInteger() : $defaultType;
if ($in instanceof ConstantStringType) {
$value = $in->getValue();
$allowOctal = $this->hasFlag($this->getConstant('FILTER_FLAG_ALLOW_OCTAL'), $flagsType);
$allowHex = $this->hasFlag($this->getConstant('FILTER_FLAG_ALLOW_HEX'), $flagsType);

if ($allowOctal && preg_match('/\A0[oO][0-7]+\z/', $value) === 1) {
$octalValue = octdec($value);
return is_int($octalValue) ? new ConstantIntegerType($octalValue) : $defaultType;
}

if ($allowHex && preg_match('/\A0[xX][0-9A-Fa-f]+\z/', $value) === 1) {
$hexValue = hexdec($value);
return is_int($hexValue) ? new ConstantIntegerType($hexValue) : $defaultType;
}

return preg_match('/\A[+-]?(?:0|[1-9][0-9]*)\z/', $value) === 1 ? $in->toInteger() : $defaultType;
}
}

if ($filterValue === $this->getConstant('FILTER_VALIDATE_FLOAT') && $in->isInteger()->yes()) {
return $in->toFloat();
if ($filterValue === $this->getConstant('FILTER_DEFAULT')) {
if (!$this->canStringBeSanitized($filterValue, $flagsType) && $in->isString()->yes()) {
return $in;
}

if ($in->isBoolean()->yes() || $in->isFloat()->yes() || $in->isInteger()->yes() || $in->isNull()->yes()) {
return $in->toString();
}
}

return null;
Expand Down Expand Up @@ -273,7 +305,7 @@ private function getOtherTypes(?Type $flagsType, array $typeOptions, Type $defau

private function hasOptions(Type $flagsType): TrinaryLogic
{
return $flagsType->isConstantArray()
return $flagsType->isArray()
->and($flagsType->hasOffsetValueType(new ConstantStringType('options')));
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ public function dataFileAsserts(): iterable
}

yield from $this->gatherAssertTypes(__DIR__ . '/data/filesystem-functions.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/filter-var.php');

if (PHP_VERSION_ID >= 80100) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/enums.php');
Expand Down
9 changes: 6 additions & 3 deletions tests/PHPStan/Analyser/data/filter-var-returns-non-empty-string.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public function run(string $str, int $int, int $positive_int, int $negative_int)
assertType('non-empty-string', $str);

$return = filter_var($str, FILTER_DEFAULT);
assertType('non-empty-string|false', $return);
assertType('non-empty-string', $return);

$return = filter_var($str, FILTER_DEFAULT, FILTER_FLAG_STRIP_LOW);
assertType('string|false', $return);
Expand Down Expand Up @@ -82,7 +82,10 @@ public function run(string $str, int $int, int $positive_int, int $negative_int)
assertType('9', $return);

$return = filter_var(1.0, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1, 'max_range' => 9]]);
assertType('int<1, 9>|false', $return);
assertType('1', $return);

$return = filter_var(11.0, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1, 'max_range' => 9]]);
assertType('false', $return);

$return = filter_var($str, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1, 'max_range' => $positive_int]]);
assertType('int<1, max>|false', $return);
Expand All @@ -95,7 +98,7 @@ public function run(string $str, int $int, int $positive_int, int $negative_int)

$str2 = '';
$return = filter_var($str2, FILTER_DEFAULT);
assertType('string|false', $return);
assertType("''", $return);

$return = filter_var($str2, FILTER_VALIDATE_URL);
assertType('string|false', $return);
Expand Down
97 changes: 97 additions & 0 deletions tests/PHPStan/Analyser/data/filter-var.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php declare(strict_types=1);

namespace FilterVar;

use stdClass;
use function PHPStan\Testing\assertType;

class FilterVar
{

public function doFoo($mixed): void
{
assertType('int|false', filter_var($mixed, FILTER_VALIDATE_INT));
assertType('int|null', filter_var($mixed, FILTER_VALIDATE_INT, ['flags' => FILTER_NULL_ON_FAILURE]));
assertType('array<int|false>', filter_var($mixed, FILTER_VALIDATE_INT, ['flags' => FILTER_FORCE_ARRAY]));
assertType('array<int|null>', filter_var($mixed, FILTER_VALIDATE_INT, ['flags' => FILTER_FORCE_ARRAY|FILTER_NULL_ON_FAILURE]));
assertType('0|int<17, 19>', filter_var($mixed, FILTER_VALIDATE_INT, ['options' => ['default' => 0, 'min_range' => 17, 'max_range' => 19]]));

assertType('array<false>', filter_var(false, FILTER_VALIDATE_BOOLEAN, FILTER_FORCE_ARRAY | FILTER_NULL_ON_FAILURE));
}

/** @param resource $resource */
public function invalidInput(array $arr, object $object, $resource): void
{
assertType('false', filter_var($arr));
assertType('false', filter_var($object));
assertType('false', filter_var($resource));
assertType('null', filter_var(new stdClass(), FILTER_DEFAULT, FILTER_NULL_ON_FAILURE));
}

public function intToInt(int $int, array $options): void
{
assertType('int', filter_var($int, FILTER_VALIDATE_INT));
assertType('int|false', filter_var($int, FILTER_VALIDATE_INT, $options));
assertType('int<0, max>|false', filter_var($int, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]));
}

/**
* @param int<0, 9> $intRange
* @param non-empty-string $nonEmptyString
*/
public function scalars(bool $bool, float $float, int $int, string $string, int $intRange, string $nonEmptyString): void
{
assertType('bool', filter_var($bool, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('true', filter_var(true, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('false', filter_var(false, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var($float, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var(17.0, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)); // could be null
assertType('bool|null', filter_var($int, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var($intRange, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var(17, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)); // could be null
assertType('bool|null', filter_var($string, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var($nonEmptyString, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE));
assertType('bool|null', filter_var('17', FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)); // could be null
assertType('bool|null', filter_var(null, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE)); // could be null

assertType('float|false', filter_var($bool, FILTER_VALIDATE_FLOAT));
assertType('float|false', filter_var(true, FILTER_VALIDATE_FLOAT)); // could be 1
assertType('float|false', filter_var(false, FILTER_VALIDATE_FLOAT)); // could be false
assertType('float', filter_var($float, FILTER_VALIDATE_FLOAT));
assertType('17.0', filter_var(17.0, FILTER_VALIDATE_FLOAT));
assertType('float', filter_var($int, FILTER_VALIDATE_FLOAT));
assertType('float', filter_var($intRange, FILTER_VALIDATE_FLOAT));
assertType('17.0', filter_var(17, FILTER_VALIDATE_FLOAT));
assertType('float|false', filter_var($string, FILTER_VALIDATE_FLOAT));
assertType('float|false', filter_var($nonEmptyString, FILTER_VALIDATE_FLOAT));
assertType('float|false', filter_var('17', FILTER_VALIDATE_FLOAT)); // could be 17.0
assertType('float|false', filter_var(null, FILTER_VALIDATE_FLOAT)); // could be false

assertType('int|false', filter_var($bool, FILTER_VALIDATE_INT));
assertType('int|false', filter_var(true, FILTER_VALIDATE_INT)); // could be 1
assertType('int|false', filter_var(false, FILTER_VALIDATE_INT)); // could be false
assertType('int', filter_var($float, FILTER_VALIDATE_INT));
assertType('17', filter_var(17.0, FILTER_VALIDATE_INT));
assertType('int', filter_var($int, FILTER_VALIDATE_INT));
assertType('int<0, 9>', filter_var($intRange, FILTER_VALIDATE_INT));
assertType('17', filter_var(17, FILTER_VALIDATE_INT));
assertType('int|false', filter_var($string, FILTER_VALIDATE_INT));
assertType('int|false', filter_var($nonEmptyString, FILTER_VALIDATE_INT));
assertType('17', filter_var('17', FILTER_VALIDATE_INT));
assertType('int|false', filter_var(null, FILTER_VALIDATE_INT)); // could be false

assertType("''|'1'", filter_var($bool));
assertType("'1'", filter_var(true));
assertType("''", filter_var(false));
assertType('numeric-string', filter_var($float));
assertType("'17'", filter_var(17.0));
assertType('numeric-string', filter_var($int));
assertType('numeric-string', filter_var($intRange));
assertType("'17'", filter_var(17));
assertType('string', filter_var($string));
assertType('non-empty-string', filter_var($nonEmptyString));
assertType("'17'", filter_var('17'));
assertType("''", filter_var(null));
}

}
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,16 @@ public function testBug8158(): void
$this->analyse([__DIR__ . '/data/bug-8158.php'], []);
}

public function testBug8516(): void
{
if (PHP_VERSION_ID < 70400) {
$this->markTestSkipped('Test requires PHP 7.4.');
}

$this->checkAlwaysTrueStrictComparison = true;
$this->analyse([__DIR__ . '/data/bug-8516.php'], []);
}

public function testPhpUnitIntegration(): void
{
$this->checkAlwaysTrueStrictComparison = true;
Expand Down
18 changes: 18 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-8516.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php declare(strict_types = 1); // lint >= 7.4

namespace Bug8516;

function validate($value, array $options = null): bool
{
if (is_int($value)) {
$options ??= ['options' => ['min_range' => 0]];
if (filter_var($value, FILTER_VALIDATE_INT, $options) === false) {
return false;
}
// ...
}
if (is_string($value)) {
// ...
}
return true;
}

0 comments on commit 4dd92cd

Please sign in to comment.