Skip to content

More precise hash return type #3665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 16, 2025
Merged

More precise hash return type #3665

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86a908c
Improve hash return type
VincentLanglet Nov 24, 2024
8970ab6
Rework
VincentLanglet Nov 25, 2024
ee2968d
Try
VincentLanglet Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions resources/functionMap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3909,18 +3909,18 @@
'HaruPage::stroke' => ['bool', 'close_path='=>'bool'],
'HaruPage::textOut' => ['bool', 'x'=>'float', 'y'=>'float', 'text'=>'string'],
'HaruPage::textRect' => ['bool', 'left'=>'float', 'top'=>'float', 'right'=>'float', 'bottom'=>'float', 'text'=>'string', 'align='=>'int'],
'hash' => ['non-empty-string|false', 'algo'=>'string', 'data'=>'string', 'raw_output='=>'bool'],
'hash' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'data'=>'string', 'raw_output='=>'bool'],
Copy link
Contributor

@mvorisek mvorisek Nov 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.php.net/manual/en/function.hash.php - must be conditional return, when $binary flag is set, the string can be anything (contain UC easily)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! I updated the PR, please take a new look

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not sure if non-falsy-string in the functionMap.php is correct as short hash functions like crc32 can quite easily produce falsy string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvorisek can you show an example on 3v4l.org ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://3v4l.org/Bo2AT

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so your examples shows us a case where it is non-falsy-string - which is the opposite of your thesis?

crc32 can quite easily produce falsy string

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! '00..' is non-falsy-string per https://www.php.net/manual/en/language.types.boolean.php#language.types.boolean.casting but only '00..' == 0 is true.

Copy link
Contributor

@staabm staabm Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK every string with more then 1 char is non-falsy-string

'hash_algos' => ['non-empty-list<non-falsy-string>'],
'hash_copy' => ['HashContext', 'context'=>'HashContext'],
'hash_equals' => ['bool', 'known_string'=>'string', 'user_string'=>'string'],
'hash_file' => ['non-empty-string|false', 'algo'=>'string', 'filename'=>'string', 'raw_output='=>'bool'],
'hash_final' => ['non-empty-string', 'context'=>'HashContext', 'raw_output='=>'bool'],
'hash_hkdf' => ['non-empty-string|false', 'algo'=>'string', 'key'=>'string', 'length='=>'int', 'info='=>'string', 'salt='=>'string'],
'hash_hmac' => ['non-empty-string|false', 'algo'=>'string', 'data'=>'string', 'key'=>'string', 'raw_output='=>'bool'],
'hash_file' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'filename'=>'string', 'raw_output='=>'bool'],
'hash_final' => ['non-falsy-string&lowercase-string', 'context'=>'HashContext', 'raw_output='=>'bool'],
'hash_hkdf' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'key'=>'string', 'length='=>'int', 'info='=>'string', 'salt='=>'string'],
'hash_hmac' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'data'=>'string', 'key'=>'string', 'raw_output='=>'bool'],
'hash_hmac_algos' => ['non-empty-list<non-falsy-string>'],
'hash_hmac_file' => ['non-empty-string|false', 'algo'=>'string', 'filename'=>'string', 'key'=>'string', 'raw_output='=>'bool'],
'hash_hmac_file' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'filename'=>'string', 'key'=>'string', 'raw_output='=>'bool'],
'hash_init' => ['HashContext', 'algo'=>'string', 'options='=>'int', 'key='=>'string'],
'hash_pbkdf2' => ['non-empty-string|false', 'algo'=>'string', 'password'=>'string', 'salt'=>'string', 'iterations'=>'int', 'length='=>'int', 'raw_output='=>'bool'],
'hash_pbkdf2' => ['(non-falsy-string&lowercase-string)|false', 'algo'=>'string', 'password'=>'string', 'salt'=>'string', 'iterations'=>'int', 'length='=>'int', 'raw_output='=>'bool'],
'hash_update' => ['bool', 'context'=>'HashContext', 'data'=>'string'],
'hash_update_file' => ['bool', 'context'=>'HashContext', 'filename'=>'string', 'scontext='=>'?HashContext'],
'hash_update_stream' => ['int', 'context'=>'HashContext', 'handle'=>'resource', 'length='=>'int'],
Expand Down
12 changes: 7 additions & 5 deletions src/Type/Php/HashFunctionsReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
use PHPStan\Php\PhpVersion;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Accessory\AccessoryLowercaseStringType;
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
Expand Down Expand Up @@ -111,24 +112,25 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,

$neverType = new NeverType();
$falseType = new ConstantBooleanType(false);
$nonEmptyString = new IntersectionType([
$nonFalsyLowercaseString = new IntersectionType([
new StringType(),
new AccessoryNonEmptyStringType(),
new AccessoryNonFalsyStringType(),
new AccessoryLowercaseStringType(),
]);

$invalidAlgorithmType = $this->phpVersion->throwsValueErrorForInternalFunctions() ? $neverType : $falseType;
$functionData = self::SUPPORTED_FUNCTIONS[strtolower($functionReflection->getName())];

$returnTypes = array_map(
function (ConstantStringType $type) use ($functionData, $nonEmptyString, $invalidAlgorithmType) {
function (ConstantStringType $type) use ($functionData, $nonFalsyLowercaseString, $invalidAlgorithmType) {
$algorithm = strtolower($type->getValue());
if (!in_array($algorithm, $this->hashAlgorithms, true)) {
return $invalidAlgorithmType;
}
if ($functionData['cryptographic'] && in_array($algorithm, self::NON_CRYPTOGRAPHIC_ALGORITHMS, true)) {
return $invalidAlgorithmType;
}
return $nonEmptyString;
return $nonFalsyLowercaseString;
},
$constantAlgorithmTypes,
);
Expand Down
8 changes: 4 additions & 4 deletions tests/PHPStan/Analyser/nsrt/hash-functions-74.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public function hash_hmac(string $string): void
{
assertType('false', hash_hmac('crc32', 'data', 'key'));
assertType('false', hash_hmac('invalid', 'data', 'key'));
assertType('(non-empty-string|false)', hash_hmac($string, 'data', 'key'));
assertType('((lowercase-string&non-falsy-string)|false)', hash_hmac($string, 'data', 'key'));
}

public function hash_hmac_file(): void
Expand All @@ -23,7 +23,7 @@ public function hash_hmac_file(): void
public function hash(string $string): void
{
assertType('false', hash('invalid', 'data', false));
assertType('(non-empty-string|false)', hash($string, 'data'));
assertType('((lowercase-string&non-falsy-string)|false)', hash($string, 'data'));
}

public function hash_file(): void
Expand All @@ -35,14 +35,14 @@ public function hash_hkdf(string $string): void
{
assertType('false', hash_hkdf('crc32', 'key'));
assertType('false', hash_hkdf('invalid', 'key'));
assertType('(non-empty-string|false)', hash_hkdf($string, 'key'));
assertType('((lowercase-string&non-falsy-string)|false)', hash_hkdf($string, 'key'));
}

public function hash_pbkdf2(string $string): void
{
assertType('false', hash_pbkdf2('crc32', 'password', 'salt', 1000));
assertType('false', hash_pbkdf2('invalid', 'password', 'salt', 1000));
assertType('(non-empty-string|false)', hash_pbkdf2($string, 'password', 'salt', 1000));
assertType('((lowercase-string&non-falsy-string)|false)', hash_pbkdf2($string, 'password', 'salt', 1000));
}

public function caseSensitive()
Expand Down
30 changes: 15 additions & 15 deletions tests/PHPStan/Analyser/nsrt/hash-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,44 @@ class HashFunctionTests

public function hash_hmac(): void
{
assertType('non-empty-string', hash_hmac('md5', 'data', 'key'));
assertType('non-empty-string', hash_hmac('sha256', 'data', 'key'));
assertType('lowercase-string&non-falsy-string', hash_hmac('md5', 'data', 'key'));
assertType('lowercase-string&non-falsy-string', hash_hmac('sha256', 'data', 'key'));
}

public function hash_hmac_file(string $string): void
{
assertType('non-empty-string|false', hash_hmac_file('md5', 'filename', 'key'));
assertType('non-empty-string|false', hash_hmac_file('sha256', 'filename', 'key'));
assertType('(non-empty-string|false)', hash_hmac_file($string, 'filename', 'key'));
assertType('(lowercase-string&non-falsy-string)|false', hash_hmac_file('md5', 'filename', 'key'));
assertType('(lowercase-string&non-falsy-string)|false', hash_hmac_file('sha256', 'filename', 'key'));
assertType('((lowercase-string&non-falsy-string)|false)', hash_hmac_file($string, 'filename', 'key'));
}

public function hash($mixed): void
{
assertType('non-empty-string', hash('sha256', 'data', false));
assertType('non-empty-string', hash('sha256', 'data', true));
assertType('non-empty-string', hash('md5', $mixed, false));
assertType('lowercase-string&non-falsy-string', hash('sha256', 'data', false));
assertType('lowercase-string&non-falsy-string', hash('sha256', 'data', true));
assertType('lowercase-string&non-falsy-string', hash('md5', $mixed, false));
}

public function hash_file(): void
{
assertType('non-empty-string|false', hash_file('sha256', 'filename', false));
assertType('non-empty-string|false', hash_file('sha256', 'filename', true));
assertType('non-empty-string|false', hash_file('crc32', 'filename'));
assertType('(lowercase-string&non-falsy-string)|false', hash_file('sha256', 'filename', false));
assertType('(lowercase-string&non-falsy-string)|false', hash_file('sha256', 'filename', true));
assertType('(lowercase-string&non-falsy-string)|false', hash_file('crc32', 'filename'));
}

public function hash_hkdf(): void
{
assertType('non-empty-string', hash_hkdf('sha256', 'key'));
assertType('lowercase-string&non-falsy-string', hash_hkdf('sha256', 'key'));
}

public function hash_pbkdf2(): void
{
assertType('non-empty-string', hash_pbkdf2('sha256', 'password', 'salt', 1000));
assertType('lowercase-string&non-falsy-string', hash_pbkdf2('sha256', 'password', 'salt', 1000));
}

public function caseSensitive()
{
assertType('non-empty-string', hash('SHA256', 'data'));
assertType('lowercase-string&non-falsy-string', hash('SHA256', 'data'));
}

public function constantStrings(int $type)
Expand All @@ -69,7 +69,7 @@ public function constantStrings(int $type)
return;
}

assertType('non-empty-string', hash_pbkdf2($algorithm, 'password', 'salt', 1000));
assertType('lowercase-string&non-falsy-string', hash_pbkdf2($algorithm, 'password', 'salt', 1000));
}

}
Loading