Skip to content

Commit 4a16dd6

Browse files
VincentLangletVincentLanglet
authored
Fix filter var on uncertainty flags
1 parent 230e9ee commit 4a16dd6

File tree

5 files changed

+102
-34
lines changed

5 files changed

+102
-34
lines changed

‎src/Type/Php/FilterFunctionReturnTypeHelper.php‎

Lines changed: 74 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use PHPStan\Type\StringType;
2727
use PHPStan\Type\Type;
2828
use PHPStan\Type\TypeCombinator;
29+
use PHPStan\Type\UnionType;
2930
use function array_key_exists;
3031
use function array_merge;
3132
use function hexdec;
@@ -57,9 +58,14 @@ public function __construct(private ReflectionProvider $reflectionProvider, priv
5758

5859
private function getOffsetValueType(Type $inputType, Type $offsetType, ?Type $filterType, ?Type $flagsType): Type
5960
{
60-
$inexistentOffsetType = $this->hasFlag('FILTER_NULL_ON_FAILURE', $flagsType)
61-
? new ConstantBooleanType(false)
62-
: new NullType();
61+
$hasNullOnFailure = $this->hasFlag('FILTER_NULL_ON_FAILURE', $flagsType);
62+
if ($hasNullOnFailure->yes()) {
63+
$inexistentOffsetType = new ConstantBooleanType(false);
64+
} elseif ($hasNullOnFailure->no()) {
65+
$inexistentOffsetType = new NullType();
66+
} else {
67+
$inexistentOffsetType = new UnionType([new ConstantBooleanType(false), new NullType()]);
68+
}
6369

6470
$hasOffsetValueType = $inputType->hasOffsetValueType($offsetType);
6571
if ($hasOffsetValueType->no()) {
@@ -123,22 +129,42 @@ public function getType(Type $inputType, ?Type $filterType, ?Type $flagsType): T
123129
$hasOptions = $this->hasOptions($flagsType);
124130
$options = $hasOptions->yes() ? $this->getOptions($flagsType, $filterValue) : [];
125131

126-
$defaultType = $options['default'] ?? ($this->hasFlag('FILTER_NULL_ON_FAILURE', $flagsType)
127-
? new NullType()
128-
: new ConstantBooleanType(false));
132+
if (isset($options['default'])) {
133+
$defaultType = $options['default'];
134+
} else {
135+
$hasNullOnFailure = $this->hasFlag('FILTER_NULL_ON_FAILURE', $flagsType);
136+
if ($hasNullOnFailure->yes()) {
137+
$defaultType = new NullType();
138+
} elseif ($hasNullOnFailure->no()) {
139+
$defaultType = new ConstantBooleanType(false);
140+
} else {
141+
$defaultType = new UnionType([new ConstantBooleanType(false), new NullType()]);
142+
}
143+
}
144+
145+
$hasRequireArrayFlag = $this->hasFlag('FILTER_REQUIRE_ARRAY', $flagsType);
146+
if ($hasRequireArrayFlag->maybe()) {
147+
// Too complicated
148+
return $mixedType;
149+
}
129150

130151
$inputIsArray = $inputType->isArray();
131152
$hasRequireArrayFlag = $this->hasFlag('FILTER_REQUIRE_ARRAY', $flagsType);
132-
if ($inputIsArray->no() && $hasRequireArrayFlag) {
133-
if ($this->hasFlag('FILTER_THROW_ON_FAILURE', $flagsType)) {
153+
if ($inputIsArray->no() && $hasRequireArrayFlag->yes()) {
154+
if ($this->hasFlag('FILTER_THROW_ON_FAILURE', $flagsType)->yes()) {
134155
return new ErrorType();
135156
}
136157

137158
return $defaultType;
138159
}
139160

140161
$hasForceArrayFlag = $this->hasFlag('FILTER_FORCE_ARRAY', $flagsType);
141-
if ($inputIsArray->yes() && ($hasRequireArrayFlag || $hasForceArrayFlag)) {
162+
if ($hasRequireArrayFlag->no() && $hasForceArrayFlag->maybe()) {
163+
// Too complicated
164+
return $mixedType;
165+
}
166+
167+
if ($inputIsArray->yes() && ($hasRequireArrayFlag->yes() || $hasForceArrayFlag->yes())) {
142168
$inputArrayKeyType = $inputType->getIterableKeyType();
143169
$inputType = $inputType->getIterableValueType();
144170
}
@@ -152,9 +178,11 @@ public function getType(Type $inputType, ?Type $filterType, ?Type $flagsType): T
152178
$type = $exactType ?? $this->getFilterTypeMap()[$filterValue] ?? $mixedType;
153179
$type = $this->applyRangeOptions($type, $options, $defaultType);
154180

155-
if ($inputType->isNonEmptyString()->yes()
181+
if (
182+
$inputType->isNonEmptyString()->yes()
156183
&& $type->isString()->yes()
157-
&& !$this->canStringBeSanitized($filterValue, $flagsType)) {
184+
&& $this->canStringBeSanitized($filterValue, $flagsType)->no()
185+
) {
158186
$accessory = new AccessoryNonEmptyStringType();
159187
if ($inputType->isNonFalsyString()->yes()) {
160188
$accessory = new AccessoryNonFalsyStringType();
@@ -168,18 +196,18 @@ public function getType(Type $inputType, ?Type $filterType, ?Type $flagsType): T
168196
}
169197
}
170198

171-
if ($hasRequireArrayFlag) {
199+
if ($hasRequireArrayFlag->yes()) {
172200
$type = new ArrayType($inputArrayKeyType ?? $mixedType, $type);
173201
if (!$inputIsArray->yes()) {
174202
$type = TypeCombinator::union($type, $defaultType);
175203
}
176204
}
177205

178-
if (!$hasRequireArrayFlag && $hasForceArrayFlag) {
206+
if ($hasRequireArrayFlag->no() && $hasForceArrayFlag->yes()) {
179207
return new ArrayType($inputArrayKeyType ?? $mixedType, $type);
180208
}
181209

182-
if ($this->hasFlag('FILTER_THROW_ON_FAILURE', $flagsType)) {
210+
if ($this->hasFlag('FILTER_THROW_ON_FAILURE', $flagsType)->yes()) {
183211
$type = TypeCombinator::remove($type, $defaultType);
184212
}
185213

@@ -338,16 +366,19 @@ private function determineExactType(Type $in, int $filterValue, Type $defaultTyp
338366
}
339367

340368
if ($in instanceof ConstantStringType) {
341-
$value = $in->getValue();
342369
$allowOctal = $this->hasFlag('FILTER_FLAG_ALLOW_OCTAL', $flagsType);
343370
$allowHex = $this->hasFlag('FILTER_FLAG_ALLOW_HEX', $flagsType);
371+
if ($allowOctal->maybe() || $allowHex->maybe()) {
372+
return null;
373+
}
344374

345-
if ($allowOctal && preg_match('/\A0[oO][0-7]+\z/', $value) === 1) {
375+
$value = $in->getValue();
376+
if ($allowOctal->yes() && preg_match('/\A0[oO][0-7]+\z/', $value) === 1) {
346377
$octalValue = octdec($value);
347378
return is_int($octalValue) ? new ConstantIntegerType($octalValue) : $defaultType;
348379
}
349380

350-
if ($allowHex && preg_match('/\A0[xX][0-9A-Fa-f]+\z/', $value) === 1) {
381+
if ($allowHex->yes() && preg_match('/\A0[xX][0-9A-Fa-f]+\z/', $value) === 1) {
351382
$hexValue = hexdec($value);
352383
return is_int($hexValue) ? new ConstantIntegerType($hexValue) : $defaultType;
353384
}
@@ -357,7 +388,7 @@ private function determineExactType(Type $in, int $filterValue, Type $defaultTyp
357388
}
358389

359390
if ($filterValue === $this->getConstant('FILTER_DEFAULT')) {
360-
if (!$this->canStringBeSanitized($filterValue, $flagsType) && $in->isString()->yes()) {
391+
if ($this->canStringBeSanitized($filterValue, $flagsType)->no() && $in->isString()->yes()) {
361392
return $in;
362393
}
363394

@@ -452,20 +483,23 @@ private function getOptions(Type $flagsType, int $filterValue): array
452483
/**
453484
* @param non-empty-string $flagName
454485
*/
455-
private function hasFlag(string $flagName, ?Type $flagsType): bool
486+
private function hasFlag(string $flagName, ?Type $flagsType): TrinaryLogic
456487
{
457488
$flag = $this->getConstant($flagName);
458489
if ($flag === null) {
459-
return false;
490+
return TrinaryLogic::createNo();
460491
}
461492

462-
if ($flagsType === null) {
463-
return false;
493+
if ($flagsType === null) { // Will default to 0
494+
return TrinaryLogic::createNo();
464495
}
465496

466497
$type = $this->getFlagsValue($flagsType);
498+
if (!$type instanceof ConstantIntegerType) {
499+
return TrinaryLogic::createMaybe();
500+
}
467501

468-
return $type instanceof ConstantIntegerType && ($type->getValue() & $flag) === $flag;
502+
return TrinaryLogic::createFromBoolean(($type->getValue() & $flag) === $flag);
469503
}
470504

471505
private function getFlagsValue(Type $exprType): Type
@@ -474,25 +508,36 @@ private function getFlagsValue(Type $exprType): Type
474508
return $exprType;
475509
}
476510

477-
return $exprType->getOffsetValueType($this->flagsString);
511+
$hasOffsetValue = $exprType->hasOffsetValueType($this->flagsString);
512+
if ($hasOffsetValue->no()) {
513+
return new ConstantIntegerType(0);
514+
}
515+
if ($hasOffsetValue->yes()) {
516+
return $exprType->getOffsetValueType($this->flagsString);
517+
}
518+
519+
return TypeCombinator::union(
520+
new ConstantIntegerType(0),
521+
$exprType->getOffsetValueType($this->flagsString),
522+
);
478523
}
479524

480-
private function canStringBeSanitized(int $filterValue, ?Type $flagsType): bool
525+
private function canStringBeSanitized(int $filterValue, ?Type $flagsType): TrinaryLogic
481526
{
482527
// If it is a validation filter, the string will not be changed
483528
if (($filterValue & self::VALIDATION_FILTER_BITMASK) !== 0) {
484-
return false;
529+
return TrinaryLogic::createNo();
485530
}
486531

487532
// FILTER_DEFAULT will not sanitize, unless it has FILTER_FLAG_STRIP_LOW,
488533
// FILTER_FLAG_STRIP_HIGH, or FILTER_FLAG_STRIP_BACKTICK
489534
if ($filterValue === $this->getConstant('FILTER_DEFAULT')) {
490535
return $this->hasFlag('FILTER_FLAG_STRIP_LOW', $flagsType)
491-
|| $this->hasFlag('FILTER_FLAG_STRIP_HIGH', $flagsType)
492-
|| $this->hasFlag('FILTER_FLAG_STRIP_BACKTICK', $flagsType);
536+
->or($this->hasFlag('FILTER_FLAG_STRIP_HIGH', $flagsType))
537+
->or($this->hasFlag('FILTER_FLAG_STRIP_BACKTICK', $flagsType));
493538
}
494539

495-
return true;
540+
return TrinaryLogic::createYes();
496541
}
497542

498543
}

‎tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7354,8 +7354,8 @@ public static function dataFilterVar(): Generator
73547354

73557355
$typeAndFlags = [
73567356
['%s|false', ''],
7357-
['%s|false', ', $mixed'],
7358-
['%s|false', ', ["flags" => $mixed]'],
7357+
['mixed', ', $mixed'],
7358+
['mixed', ', ["flags" => $mixed]'],
73597359
['%s|null', ', FILTER_NULL_ON_FAILURE'],
73607360
['%s|null', ', ["flags" => FILTER_NULL_ON_FAILURE]'],
73617361
['%s|null', ', ["flags" => FILTER_NULL_ON_FAILURE | FILTER_FLAG_IPV4]'],
@@ -7384,8 +7384,8 @@ public static function dataFilterVar(): Generator
73847384

73857385
$boolFlags = [
73867386
['bool', ''],
7387-
['bool', ', $mixed'],
7388-
['bool', ', ["flags" => $mixed]'],
7387+
['mixed', ', $mixed'],
7388+
['mixed', ', ["flags" => $mixed]'],
73897389
['bool|null', ', FILTER_NULL_ON_FAILURE'],
73907390
['bool|null', ', ["flags" => FILTER_NULL_ON_FAILURE]'],
73917391
['bool|null', ', ["flags" => FILTER_NULL_ON_FAILURE | FILTER_FLAG_IPV4]'],

‎tests/PHPStan/Analyser/nsrt/filter-var.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public function invalidInput(array $arr, object $object, $resource): void
9090
public function intToInt(int $int, array $options): void
9191
{
9292
assertType('int', filter_var($int, FILTER_VALIDATE_INT));
93-
assertType('int|false', filter_var($int, FILTER_VALIDATE_INT, $options));
93+
assertType('mixed', filter_var($int, FILTER_VALIDATE_INT, $options));
9494
assertType('int<0, max>|false', filter_var($int, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]]));
9595
}
9696

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,11 @@ public function testBug11019(): void
10091009
$this->analyse([__DIR__ . '/data/bug-11019.php'], []);
10101010
}
10111011

1012+
public function testBug11485(): void
1013+
{
1014+
$this->analyse([__DIR__ . '/data/bug-11485.php'], []);
1015+
}
1016+
10121017
public function testBug12946(): void
10131018
{
10141019
$this->analyse([__DIR__ . '/data/bug-12946.php'], []);

‎tests/PHPStan/Rules/Comparison/data/bug-11485.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11485;
4+
5+
class Foo {
6+
7+
public function bar(string $value, bool $strict = true): bool {
8+
9+
$flags = $strict ? FILTER_NULL_ON_FAILURE : 0;
10+
$result = filter_var($value, FILTER_VALIDATE_BOOLEAN, $flags);
11+
if ($result === null) throw new \Exception("not a boolean");
12+
13+
$result = filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
14+
if ($result === null) throw new \Exception("not a boolean");
15+
16+
return $result;
17+
}
18+
}

0 commit comments

Comments
 (0)