From 8daa5fe9b6f8fac5f04c633780f56dfa89a2a199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:11:31 +0200 Subject: [PATCH 01/10] Use `php_random_bytes_throw()` in Secure engine's generate() This exposes the underlying exception, improving debugging: Fatal error: Uncaught Exception: Cannot open source device in php-src/test.php:5 Stack trace: #0 php-src/test.php(5): Random\Engine\Secure->generate() #1 {main} Next RuntimeException: Random number generation failed in php-src/test.php:5 Stack trace: #0 php-src/test.php(5): Random\Engine\Secure->generate() #1 {main} thrown in php-src/test.php on line 5 --- ext/random/engine_secure.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/random/engine_secure.c b/ext/random/engine_secure.c index 464472dbe04d..1d6319a6a496 100644 --- a/ext/random/engine_secure.c +++ b/ext/random/engine_secure.c @@ -29,7 +29,7 @@ static uint64_t generate(php_random_status *status) { zend_ulong r = 0; - if (php_random_bytes_silent(&r, sizeof(zend_ulong)) == FAILURE) { + if (php_random_bytes_throw(&r, sizeof(zend_ulong)) == FAILURE) { status->last_unsafe = true; } From b7bdf1aca647f9bf834a4224ef693131f25386bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:32:04 +0200 Subject: [PATCH 02/10] Use `php_random_int_throw()` in Secure engine's range() This exposes the underlying exception, improving debugging: Exception: Cannot open source device in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getInt(1, 3) #1 {main} Next RuntimeException: Random number generation failed in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getInt(1, 3) #1 {main} --- ext/random/engine_secure.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/random/engine_secure.c b/ext/random/engine_secure.c index 1d6319a6a496..7a0a4a5b00da 100644 --- a/ext/random/engine_secure.c +++ b/ext/random/engine_secure.c @@ -38,9 +38,9 @@ static uint64_t generate(php_random_status *status) static zend_long range(php_random_status *status, zend_long min, zend_long max) { - zend_long result; + zend_long result = 0; - if (php_random_int_silent(min, max, &result) == FAILURE) { + if (php_random_int_throw(min, max, &result) == FAILURE) { status->last_unsafe = true; } From f93daeb551fe31f6311307fc880e3acfef07e749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:19:59 +0200 Subject: [PATCH 03/10] Throw exception when a user engine returns an empty string This improves debugging, because the actual reason for the failure is available as a previous Exception: DomainException: The returned string must not be empty in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getBytes(123) #1 {main} Next RuntimeException: Random number generation failed in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getBytes(123) #1 {main} --- ext/random/engine_user.c | 4 + .../tests/03_randomizer/user_unsafe.phpt | 101 +++++++++++++++--- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/ext/random/engine_user.c b/ext/random/engine_user.c index 1b86c93d0ac5..2e1d254f686d 100644 --- a/ext/random/engine_user.c +++ b/ext/random/engine_user.c @@ -21,6 +21,9 @@ #include "php.h" #include "php_random.h" +#include "ext/spl/spl_exceptions.h" +#include "Zend/zend_exceptions.h" + static uint64_t generate(php_random_status *status) { php_random_status_state_user *s = status->state; @@ -50,6 +53,7 @@ static uint64_t generate(php_random_status *status) result += ((uint64_t) (unsigned char) Z_STRVAL(retval)[i]) << (8 * i); } } else { + zend_throw_exception(spl_ce_DomainException, "A random engine must return a non-empty string", 0); status->last_unsafe = true; return 0; } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 659159671f29..7f82dc28ecb2 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -16,27 +16,35 @@ $randomizer = (new \Random\Randomizer( try { var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX)); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump(bin2hex($randomizer->getBytes(1))); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump($randomizer->shuffleArray(\range(1, 10))); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump($randomizer->shuffleBytes('foobar')); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "=======", PHP_EOL, PHP_EOL; + // Infinite loop $randomizer = (new \Random\Randomizer( new class () implements \Random\Engine { @@ -50,34 +58,99 @@ $randomizer = (new \Random\Randomizer( try { var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX)); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump(bin2hex($randomizer->getBytes(1))); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump($randomizer->shuffleArray(\range(1, 10))); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } +echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump($randomizer->shuffleBytes('foobar')); } catch (\RuntimeException $e) { - echo $e->getMessage() . PHP_EOL; + echo $e, PHP_EOL; } ?> --EXPECTF-- -Random number generation failed -Random number generation failed -Random number generation failed -Random number generation failed +DomainException: A random engine must return a non-empty string in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getInt(%d, %d) +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getInt(%d, %d) +#1 {main} + +------- + +DomainException: A random engine must return a non-empty string in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getBytes(1) +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getBytes(1) +#1 {main} + +------- + +DomainException: A random engine must return a non-empty string in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#1 {main} + +------- + +DomainException: A random engine must return a non-empty string in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleBytes('foobar') +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleBytes('foobar') +#1 {main} + +======= + int(%d) + +------- + string(2) "ff" -Random number generation failed -Random number generation failed + +------- + +RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#1 {main} + +------- + +RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleBytes('foobar') +#1 {main} From fdfa2f3c2818b3ea132c758b8962d4587dcf5dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:29:43 +0200 Subject: [PATCH 04/10] Throw exception when the range selector fails to get acceptable numbers in 50 attempts This improves debugging, because the actual reason for the failure is available as a previous Exception: RuntimeException: Failed to generate an acceptable random number in 50 attempts in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getInt(1, 3) #1 {main} Next RuntimeException: Random number generation failed in php-src/test.php:17 Stack trace: #0 php-src/test.php(17): Random\Randomizer->getInt(1, 3) #1 {main} --- ext/random/random.c | 2 ++ ext/random/tests/03_randomizer/user_unsafe.phpt | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/ext/random/random.c b/ext/random/random.c index 1763842fc0dd..67bc9b4b347d 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -119,6 +119,7 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat while (UNEXPECTED(result > limit)) { /* If the requirements cannot be met in a cycles, return fail */ if (++count > 50) { + zend_throw_exception(spl_ce_RuntimeException, "Failed to generate an acceptable random number in 50 attempts", 0); status->last_unsafe = true; return 0; } @@ -175,6 +176,7 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat while (UNEXPECTED(result > limit)) { /* If the requirements cannot be met in a cycles, return fail */ if (++count > 50) { + zend_throw_exception(spl_ce_RuntimeException, "Failed to generate an acceptable random number in 50 attempts", 0); status->last_unsafe = true; return 0; } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 7f82dc28ecb2..8057e073b06b 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -143,7 +143,12 @@ string(2) "ff" ------- -RuntimeException: Random number generation failed in %s:%d +RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleArray(Array) #1 {main} From 12b07ced2e006365aba1fa575d7b9261a925d42a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:36:26 +0200 Subject: [PATCH 05/10] Improve user_unsafe test Select parameters for ->getInt() that will actually lead to unsafe behavior. --- .../tests/03_randomizer/user_unsafe.phpt | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 8057e073b06b..a787f2444e34 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -14,7 +14,7 @@ $randomizer = (new \Random\Randomizer( )); try { - var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX)); + var_dump($randomizer->getInt(0, 123)); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -56,7 +56,7 @@ $randomizer = (new \Random\Randomizer( )); try { - var_dump($randomizer->getInt(\PHP_INT_MIN, \PHP_INT_MAX)); + var_dump($randomizer->getInt(0, 123)); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -89,12 +89,12 @@ try { --EXPECTF-- DomainException: A random engine must return a non-empty string in %s:%d Stack trace: -#0 %s(%d): Random\Randomizer->getInt(%d, %d) +#0 %s(%d): Random\Randomizer->getInt(0, 123) #1 {main} Next RuntimeException: Random number generation failed in %s:%d Stack trace: -#0 %s(%d): Random\Randomizer->getInt(%d, %d) +#0 %s(%d): Random\Randomizer->getInt(0, 123) #1 {main} ------- @@ -135,20 +135,26 @@ Stack trace: ======= -int(%d) - -------- +RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getInt(0, 123) +#1 {main} -string(2) "ff" +Next RuntimeException: Random number generation failed in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->getInt(0, 123) +#1 {main} ------- -RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +RuntimeException: Random number generation failed in %s:%d Stack trace: -#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#0 %s(%d): Random\Randomizer->getBytes(1) #1 {main} -Next RuntimeException: Random number generation failed in %s:%d +------- + +RuntimeException: Random number generation failed in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleArray(Array) #1 {main} From 3e1844172dd8b6f2f6261fb02187067d18c803a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:39:20 +0200 Subject: [PATCH 06/10] Fix user_unsafe test If an engine fails once it will be permanently poisoned by setting `->last_unsafe`. This is undesirable for the test, because it skews the results. Fix this by creating a fresh engine for each "assertion". --- .../tests/03_randomizer/user_unsafe.phpt | 63 ++++++++++--------- 1 file changed, 34 insertions(+), 29 deletions(-) diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index a787f2444e34..f47716d2892b 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -3,18 +3,17 @@ Random: Randomizer: User: Engine unsafe --FILE-- getInt(0, 123)); + var_dump((new Randomizer(new EmptyStringEngine()))->getInt(0, 123)); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -22,7 +21,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump(bin2hex($randomizer->getBytes(1))); + var_dump(bin2hex((new Randomizer(new EmptyStringEngine()))->getBytes(1))); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -30,7 +29,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump($randomizer->shuffleArray(\range(1, 10))); + var_dump((new Randomizer(new EmptyStringEngine()))->shuffleArray(\range(1, 10))); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -38,7 +37,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump($randomizer->shuffleBytes('foobar')); + var_dump((new Randomizer(new EmptyStringEngine()))->shuffleBytes('foobar')); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -46,17 +45,16 @@ try { echo PHP_EOL, "=======", PHP_EOL, PHP_EOL; // Infinite loop -$randomizer = (new \Random\Randomizer( - new class () implements \Random\Engine { - public function generate(): string - { - return "\xff\xff\xff\xff\xff\xff\xff\xff"; - } + +final class HeavilyBiasedEngine implements \Random\Engine { + public function generate(): string + { + return "\xff\xff\xff\xff\xff\xff\xff\xff"; } -)); +} try { - var_dump($randomizer->getInt(0, 123)); + var_dump((new Randomizer(new HeavilyBiasedEngine()))->getInt(0, 123)); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -64,7 +62,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump(bin2hex($randomizer->getBytes(1))); + var_dump(bin2hex((new Randomizer(new HeavilyBiasedEngine()))->getBytes(1))); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -72,7 +70,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump($randomizer->shuffleArray(\range(1, 10))); + var_dump((new Randomizer(new HeavilyBiasedEngine()))->shuffleArray(\range(1, 10))); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -80,7 +78,7 @@ try { echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; try { - var_dump($randomizer->shuffleBytes('foobar')); + var_dump((new Randomizer(new HeavilyBiasedEngine()))->shuffleBytes('foobar')); } catch (\RuntimeException $e) { echo $e, PHP_EOL; } @@ -147,21 +145,28 @@ Stack trace: ------- -RuntimeException: Random number generation failed in %s:%d -Stack trace: -#0 %s(%d): Random\Randomizer->getBytes(1) -#1 {main} +string(2) "ff" ------- -RuntimeException: Random number generation failed in %s:%d +RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleArray(Array) +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleArray(Array) #1 {main} ------- -RuntimeException: Random number generation failed in %s:%d +RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->shuffleBytes('foobar') +#1 {main} + +Next RuntimeException: Random number generation failed in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleBytes('foobar') #1 {main} From 8adb67a37ce69c4db141805dae9be1b6a3b6ac35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Jul 2022 19:42:40 +0200 Subject: [PATCH 07/10] Remove duplication in user_unsafe.phpt --- .../tests/03_randomizer/user_unsafe.phpt | 105 ++++++++---------- 1 file changed, 45 insertions(+), 60 deletions(-) diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index f47716d2892b..94740be5db8d 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -12,40 +12,6 @@ final class EmptyStringEngine implements \Random\Engine { } } -try { - var_dump((new Randomizer(new EmptyStringEngine()))->getInt(0, 123)); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - -try { - var_dump(bin2hex((new Randomizer(new EmptyStringEngine()))->getBytes(1))); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - -try { - var_dump((new Randomizer(new EmptyStringEngine()))->shuffleArray(\range(1, 10))); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - -try { - var_dump((new Randomizer(new EmptyStringEngine()))->shuffleBytes('foobar')); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "=======", PHP_EOL, PHP_EOL; - -// Infinite loop - final class HeavilyBiasedEngine implements \Random\Engine { public function generate(): string { @@ -53,38 +19,53 @@ final class HeavilyBiasedEngine implements \Random\Engine { } } -try { - var_dump((new Randomizer(new HeavilyBiasedEngine()))->getInt(0, 123)); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - -try { - var_dump(bin2hex((new Randomizer(new HeavilyBiasedEngine()))->getBytes(1))); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} - -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; +echo "=====================", PHP_EOL; -try { - var_dump((new Randomizer(new HeavilyBiasedEngine()))->shuffleArray(\range(1, 10))); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; -} +foreach ([ + EmptyStringEngine::class, + HeavilyBiasedEngine::class, +] as $engine) { + echo $engine, PHP_EOL, "=====================", PHP_EOL, PHP_EOL; -echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { + var_dump((new Randomizer(new $engine()))->getInt(0, 123)); + } catch (\RuntimeException $e) { + echo $e, PHP_EOL; + } + + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + + try { + var_dump(bin2hex((new Randomizer(new $engine()))->getBytes(1))); + } catch (\RuntimeException $e) { + echo $e, PHP_EOL; + } + + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + + try { + var_dump((new Randomizer(new $engine()))->shuffleArray(\range(1, 10))); + } catch (\RuntimeException $e) { + echo $e, PHP_EOL; + } + + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + + try { + var_dump((new Randomizer(new $engine()))->shuffleBytes('foobar')); + } catch (\RuntimeException $e) { + echo $e, PHP_EOL; + } -try { - var_dump((new Randomizer(new HeavilyBiasedEngine()))->shuffleBytes('foobar')); -} catch (\RuntimeException $e) { - echo $e, PHP_EOL; + echo PHP_EOL, "=====================", PHP_EOL; } ?> --EXPECTF-- +===================== +EmptyStringEngine +===================== + DomainException: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->getInt(0, 123) @@ -131,7 +112,9 @@ Stack trace: #0 %s(%d): Random\Randomizer->shuffleBytes('foobar') #1 {main} -======= +===================== +HeavilyBiasedEngine +===================== RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d Stack trace: @@ -170,3 +153,5 @@ Next RuntimeException: Random number generation failed in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleBytes('foobar') #1 {main} + +===================== From 309037908214345e0b6bf592bfa52c33620f454a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 21 Jul 2022 19:50:15 +0200 Subject: [PATCH 08/10] Catch `Throwable` in user_unsafe.phpt As we print the full stringified exception we implicitly assert the type of the exception. No need to be overly specific in the catch block. --- ext/random/tests/03_randomizer/user_unsafe.phpt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 94740be5db8d..b8e5b379af55 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -29,7 +29,7 @@ foreach ([ try { var_dump((new Randomizer(new $engine()))->getInt(0, 123)); - } catch (\RuntimeException $e) { + } catch (Throwable $e) { echo $e, PHP_EOL; } @@ -37,7 +37,7 @@ foreach ([ try { var_dump(bin2hex((new Randomizer(new $engine()))->getBytes(1))); - } catch (\RuntimeException $e) { + } catch (Throwable $e) { echo $e, PHP_EOL; } @@ -45,7 +45,7 @@ foreach ([ try { var_dump((new Randomizer(new $engine()))->shuffleArray(\range(1, 10))); - } catch (\RuntimeException $e) { + } catch (Throwable $e) { echo $e, PHP_EOL; } @@ -53,7 +53,7 @@ foreach ([ try { var_dump((new Randomizer(new $engine()))->shuffleBytes('foobar')); - } catch (\RuntimeException $e) { + } catch (Throwable $e) { echo $e, PHP_EOL; } From 39b996affed4523ee70905da6472827e670b8a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 24 Jul 2022 11:40:09 +0200 Subject: [PATCH 09/10] Throw an error if an engine returns an empty string --- ext/random/engine_user.c | 5 +---- ext/random/tests/03_randomizer/user_unsafe.phpt | 8 ++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/ext/random/engine_user.c b/ext/random/engine_user.c index 2e1d254f686d..07b4f5ce0086 100644 --- a/ext/random/engine_user.c +++ b/ext/random/engine_user.c @@ -21,9 +21,6 @@ #include "php.h" #include "php_random.h" -#include "ext/spl/spl_exceptions.h" -#include "Zend/zend_exceptions.h" - static uint64_t generate(php_random_status *status) { php_random_status_state_user *s = status->state; @@ -53,7 +50,7 @@ static uint64_t generate(php_random_status *status) result += ((uint64_t) (unsigned char) Z_STRVAL(retval)[i]) << (8 * i); } } else { - zend_throw_exception(spl_ce_DomainException, "A random engine must return a non-empty string", 0); + zend_throw_error(NULL, "A random engine must return a non-empty string"); status->last_unsafe = true; return 0; } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index b8e5b379af55..1073574ec3dd 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -66,7 +66,7 @@ foreach ([ EmptyStringEngine ===================== -DomainException: A random engine must return a non-empty string in %s:%d +Error: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->getInt(0, 123) #1 {main} @@ -78,7 +78,7 @@ Stack trace: ------- -DomainException: A random engine must return a non-empty string in %s:%d +Error: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->getBytes(1) #1 {main} @@ -90,7 +90,7 @@ Stack trace: ------- -DomainException: A random engine must return a non-empty string in %s:%d +Error: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleArray(Array) #1 {main} @@ -102,7 +102,7 @@ Stack trace: ------- -DomainException: A random engine must return a non-empty string in %s:%d +Error: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleBytes('foobar') #1 {main} From 734c79ce6541fa075859095b8aa9b80afc70e063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 24 Jul 2022 11:42:17 +0200 Subject: [PATCH 10/10] Throw an Error if range fails to find an acceptable number in 50 attempts --- ext/random/random.c | 10 ++++++---- ext/random/tests/03_randomizer/user_unsafe.phpt | 6 +++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index 67bc9b4b347d..bc81265065c0 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -82,6 +82,8 @@ static zend_object_handlers random_engine_xoshiro256starstar_object_handlers; static zend_object_handlers random_engine_secure_object_handlers; static zend_object_handlers random_randomizer_object_handlers; +#define RANDOM_RANGE_ATTEMPTS (50) + static inline uint32_t rand_range32(const php_random_algo *algo, php_random_status *status, uint32_t umax) { uint32_t result, limit, r; @@ -118,8 +120,8 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat /* Discard numbers over the limit to avoid modulo bias */ while (UNEXPECTED(result > limit)) { /* If the requirements cannot be met in a cycles, return fail */ - if (++count > 50) { - zend_throw_exception(spl_ce_RuntimeException, "Failed to generate an acceptable random number in 50 attempts", 0); + if (++count > RANDOM_RANGE_ATTEMPTS) { + zend_throw_error(NULL, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS); status->last_unsafe = true; return 0; } @@ -175,8 +177,8 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat /* Discard numbers over the limit to avoid modulo bias */ while (UNEXPECTED(result > limit)) { /* If the requirements cannot be met in a cycles, return fail */ - if (++count > 50) { - zend_throw_exception(spl_ce_RuntimeException, "Failed to generate an acceptable random number in 50 attempts", 0); + if (++count > RANDOM_RANGE_ATTEMPTS) { + zend_throw_error(NULL, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS); status->last_unsafe = true; return 0; } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 1073574ec3dd..4497088ba9da 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -116,7 +116,7 @@ Stack trace: HeavilyBiasedEngine ===================== -RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Error: Failed to generate an acceptable random number in 50 attempts in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->getInt(0, 123) #1 {main} @@ -132,7 +132,7 @@ string(2) "ff" ------- -RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Error: Failed to generate an acceptable random number in 50 attempts in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleArray(Array) #1 {main} @@ -144,7 +144,7 @@ Stack trace: ------- -RuntimeException: Failed to generate an acceptable random number in 50 attempts in %s:%d +Error: Failed to generate an acceptable random number in 50 attempts in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->shuffleBytes('foobar') #1 {main}