Skip to content
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.

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

Types: be more specific about required scalar types #185

Merged
merged 2 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ indent_style = tab
[infection.txt]
indent_size = unset
trim_trailing_whitespace = unset

[phpstan-baseline.neon]
indent_style = unset
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@
/infection.json.dist export-ignore
/Makefile export-ignore
/phpstan.neon export-ignore
/phpstan-baseline.neon export-ignore
/phpunit.xml.dist export-ignore
/rector.php export-ignore
86 changes: 86 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
parameters:
ignoreErrors:
-
message: "#^Cannot cast mixed to int\\.$#"
count: 1
path: src/HOTP.php

-
message: "#^Comparison operation \"\\>\\=\" between int\\<0, max\\>\\|null and 0 is always true\\.$#"
count: 1
path: src/HOTP.php

-
message: "#^Method OTPHP\\\\OTP\\:\\:generateSecret\\(\\) should return non\\-empty\\-string but returns string\\.$#"
count: 1
path: src/OTP.php

-
message: "#^Variable property access on \\$this\\(OTPHP\\\\OTP\\)\\.$#"
count: 1
path: src/OTP.php

-
message: "#^Comparison operation \"\\>\\=\" between int\\<0, max\\> and 0 is always true\\.$#"
count: 1
path: src/TOTP.php

-
message: "#^Method OTPHP\\\\TOTP\\:\\:expiresIn\\(\\) should return int\\<0, max\\> but returns int\\.$#"
count: 1
path: src/TOTP.php

-
message: "#^Parameter \\#1 \\$epoch of method OTPHP\\\\TOTP\\:\\:setEpoch\\(\\) expects int\\<0, max\\>, int given\\.$#"
count: 1
path: src/TOTP.php

-
message: "#^Parameter \\#1 \\$input of method OTPHP\\\\TOTP\\:\\:at\\(\\) expects int\\<0, max\\>, int given\\.$#"
count: 1
path: src/TOTP.php

-
message: "#^Left side of \\|\\| is always true\\.$#"
count: 1
path: src/Url.php

-
message: "#^Parameter \\#2 \\$host of class OTPHP\\\\Url constructor expects non\\-empty\\-string, string given\\.$#"
count: 1
path: src/Url.php

-
message: "#^Parameter \\#3 \\$path of class OTPHP\\\\Url constructor expects non\\-empty\\-string, string given\\.$#"
count: 1
path: src/Url.php

-
message: "#^Parameter \\#4 \\$secret of class OTPHP\\\\Url constructor expects non\\-empty\\-string, array\\|string given\\.$#"
count: 1
path: src/Url.php

-
message: "#^Parameter \\#5 \\$query of class OTPHP\\\\Url constructor expects array\\<non\\-empty\\-string, mixed\\>, array\\<int\\|string, array\\|string\\> given\\.$#"
count: 1
path: src/Url.php

-
message: "#^Parameter \\#1 \\$counter of method OTPHP\\\\HOTP\\:\\:setCounter\\(\\) expects int\\<0, max\\>, \\-500 given\\.$#"
count: 1
path: tests/HOTPTest.php
Spomky marked this conversation as resolved.
Show resolved Hide resolved

-
message: "#^Parameter \\#1 \\$digits of method OTPHP\\\\OTP\\:\\:setDigits\\(\\) expects int\\<1, max\\>, 0 given\\.$#"
count: 1
path: tests/HOTPTest.php

-
message: "#^Parameter \\#1 \\$epoch of method OTPHP\\\\TOTP\\:\\:setEpoch\\(\\) expects int\\<0, max\\>, \\-1 given\\.$#"
count: 1
path: tests/TOTPTest.php

-
message: "#^Parameter \\#1 \\$period of method OTPHP\\\\TOTP\\:\\:setPeriod\\(\\) expects int\\<1, max\\>, \\-20 given\\.$#"
count: 1
path: tests/TOTPTest.php
18 changes: 1 addition & 17 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,6 @@ parameters:
paths:
- src
- tests
ignoreErrors:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By letting vendor/bin/phpstan analyse --generate-baseline generating the errors, they are much more specific and so eventual future inconsistencies emerge earlier

Copy link
Member

Choose a reason for hiding this comment

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

Excellent! I was not aware of this command.
I tested it on another project and you are right, it helps a lot.

-
message: '#Variable property access on \$this\(OTPHP\\OTP\)\.#'
path: src/ParameterTrait.php
count: 1
-
message: '#^Method OTPHP\\OTP::generateSecret\(\) should return non-empty-string but returns string\.$#'
path: src/OTP.php
count: 1
-
message: '#^Cannot cast mixed to int\.$#'
path: src/HOTP.php
count: 1
-
message: '#^Parameter \#\d .* of class OTPHP\\Url constructor expects .*\, .* given\.$#'
path: src/Url.php
count: 2

includes:
- vendor/phpstan/phpstan/conf/bleedingEdge.neon
Expand All @@ -28,3 +11,4 @@ includes:
- vendor/phpstan/phpstan-deprecation-rules/rules.neon
- vendor/phpstan/phpstan-phpunit/rules.neon
- vendor/ekino/phpstan-banned-code/extension.neon
- phpstan-baseline.neon
12 changes: 11 additions & 1 deletion src/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace OTPHP;

use function assert;
use function count;
use InvalidArgumentException;
use Throwable;
Expand Down Expand Up @@ -55,6 +56,9 @@ private static function populateOTP(OTPInterface $otp, Url $data): void
);
$otp->setIssuerIncludedAsParameter(true);
}

assert($result[0] !== '');

$otp->setIssuer($result[0]);
}

Expand All @@ -76,10 +80,16 @@ private static function createOTP(Url $parsed_url): OTPInterface
}
}

/**
* @param non-empty-string $data
* @return non-empty-string
*/
private static function getLabel(string $data): string
{
$result = explode(':', rawurldecode(mb_substr($data, 1)));
$label = count($result) === 2 ? $result[1] : $result[0];
assert($label !== '');

return count($result) === 2 ? $result[1] : $result[0];
return $label;
}
}
2 changes: 2 additions & 0 deletions src/FactoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ interface FactoryInterface
/**
* This method is the unique public method of the class. It can load a provisioning Uri and convert it into an OTP
* object.
*
* @param non-empty-string $uri
*/
public static function loadFromProvisioningUri(string $uri): OTPInterface;
}
15 changes: 13 additions & 2 deletions src/HOTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public static function generate(): self
public function getCounter(): int
{
$value = $this->getParameter('counter');
is_int($value) || throw new InvalidArgumentException('Invalid "counter" parameter.');
(is_int($value) && $value >= 0) || throw new InvalidArgumentException('Invalid "counter" parameter.');

return $value;
}
Expand Down Expand Up @@ -83,7 +83,7 @@ public function setCounter(int $counter): void
}

/**
* @return array<string, callable>
* @return array<non-empty-string, callable>
*/
protected function getParameterMap(): array
{
Expand All @@ -97,16 +97,27 @@ protected function getParameterMap(): array
]];
}

/**
* @param positive-int $counter
*/
private function updateCounter(int $counter): void
{
$this->setCounter($counter);
}

/**
* @param null|0|positive-int $window
*/
private function getWindow(null|int $window): int
{
return abs($window ?? self::DEFAULT_WINDOW);
}

/**
* @param non-empty-string $otp
* @param 0|positive-int $counter
* @param null|0|positive-int $window
*/
private function verifyOtpWithWindow(string $otp, int $counter, null|int $window): bool
{
$window = $this->getWindow($window);
Expand Down
7 changes: 7 additions & 0 deletions src/HOTPInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ interface HOTPInterface extends OTPInterface

/**
* The initial counter (a positive integer).
*
* @return 0|positive-int
*/
public function getCounter(): int;

Expand All @@ -19,7 +21,9 @@ public function getCounter(): int;
* If the secret is null, a random 64 bytes secret will be generated.
*
* @param null|non-empty-string $secret
* @param 0|positive-int $counter
* @param non-empty-string $digest
* @param positive-int $digits
*
* @deprecated Deprecated since v11.1, use ::createFromSecret or ::generate instead
*/
Expand All @@ -30,5 +34,8 @@ public static function create(
int $digits = 6
): self;

/**
* @param 0|positive-int $counter
*/
public function setCounter(int $counter): void;
}
27 changes: 24 additions & 3 deletions src/OTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace OTPHP;

use function assert;
use function chr;
use function count;
use Exception;
Expand Down Expand Up @@ -49,6 +50,10 @@ final protected static function generateSecret(): string

/**
* The OTP at the specified input.
*
* @param 0|positive-int $input
*
* @return non-empty-string
*/
protected function generateOTP(int $input): string
{
Expand All @@ -65,7 +70,7 @@ protected function generateOTP(int $input): string
}

/**
* @param array<string, mixed> $options
* @param array<non-empty-string, mixed> $options
*/
protected function filterOptions(array &$options): void
{
Expand All @@ -83,7 +88,10 @@ protected function filterOptions(array &$options): void
}

/**
* @param array<string, mixed> $options
* @param non-empty-string $type
* @param array<non-empty-string, mixed> $options
*
* @return non-empty-string
*/
protected function generateURI(string $type, array $options): string
{
Expand All @@ -102,20 +110,33 @@ protected function generateURI(string $type, array $options): string
);
}

/**
* @param non-empty-string $safe
* @param non-empty-string $user
*/
protected function compareOTP(string $safe, string $user): bool
{
return hash_equals($safe, $user);
}

/**
* @return non-empty-string
*/
private function getDecodedSecret(): string
{
try {
return Base32::decodeUpper($this->getSecret());
$decoded = Base32::decodeUpper($this->getSecret());
} catch (Exception) {
throw new RuntimeException('Unable to decode the secret. Is it correctly base32 encoded?');
}
assert($decoded !== '');

return $decoded;
}

/**
* @param 0|positive-int $int
*/
private function intToByteString(int $int): string
{
$result = [];
Expand Down
Loading