Return false for hash_hmac/hash_hmac_file on invalid algorithm
#328
Conversation
|
|
||
| public function isFunctionSupported(FunctionReflection $functionReflection): bool | ||
| { | ||
| return in_array($functionReflection->getName(), $this->functions, true); |
There was a problem hiding this comment.
This array can be here statically, it's not referenced anywhere else in the extension.
There was a problem hiding this comment.
Have inlined the value, hope this is correct?
I had orientated on other instances like ArrayPointerFunctionsDynamicReturnTypeExtension which also uses the private array $functions = [] notation.
| public function getTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope): Type | ||
| { | ||
| $algo = $functionCall->args[0]->value; | ||
| if (!in_array($algo, hash_hmac_algos(), true)) { |
There was a problem hiding this comment.
There needs to be a fallback for PHP 7.1 using https://www.php.net/manual/en/function.hash-algos.php
There was a problem hiding this comment.
I have added function_exists('hash_hmac_algos') ? hash_hmac_algos() : hash_algos() check but sadly PHPStan doesn't care. Any suggestion?
There was a problem hiding this comment.
It's fine, just make the tests pass and I'll fix the PHPStan issue just before merging :) Thanks.
pascalheidmann
force-pushed
the
hash-hmac
branch
3 times, most recently
from
111870c
to
c72db7f
Compare
pascalheidmann
force-pushed
the
hash-hmac
branch
from
c72db7f
to
4a24397
Compare
|
Just a 2 cents: Maybe it might make more sense to specify the |
|
@ondrejmirtes I have replaced the conditional declaration of those supported algorithms with a static one like @dktapps suggested. As the behaviour seems to change from return |
|
Thank you! |
|
I'll handle that case when I'll be adding support for PHP 8. |
This PR adds a dynamic function return type extension for both
hash_hmacandhash_hmac_fileas both functions can returnfalsein certain circumstances. For both functions this means providing an invalid algorithm as first parameter will statically return false, otherwisestringforhash_hmacandstring|falseforhash_hmac_file(we can't check for file readability in static analysis, and you should always check for file errors anyway).There has been a PR more than 1.5 years ago that tried to fix this within static functions map but that would lead to far more false positives.