From 6903a2ad7cb73eabe362c8dd9ef950b81eedba73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 20:28:32 +0200 Subject: [PATCH 1/7] Remove exception in Randomizer::shuffleBytes() The only way that `php_binary_string_shuffle` fails is when the engine itself fails. With the currently available list of engines we have: - Mt19937 : Infallible. - PcgOneseq128XslRr64: Infallible. - Xoshiro256StarStar : Infallible. - Secure : Practically infallible on modern systems. Exception messages were cleaned up in GH-9169. - User : Error when returning an empty string. Error when seriously biased (range() fails). And whatever Throwable the userland developer decides to use. So the existing engines are either infallible or throw an Exception/Error with a high quality message themselves, making this exception not a value-add and possibly confusing. --- ext/random/randomizer.c | 1 - ext/random/tests/03_randomizer/user_unsafe.phpt | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 2672477feccb8..bec38a0444f07 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -215,7 +215,6 @@ PHP_METHOD(Random_Randomizer, shuffleBytes) RETVAL_STRINGL(ZSTR_VAL(bytes), ZSTR_LEN(bytes)); if (!php_binary_string_shuffle(randomizer->algo, randomizer->status, Z_STRVAL_P(return_value), (zend_long) Z_STRLEN_P(return_value))) { - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); RETURN_THROWS(); } } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 4497088ba9da1..8e81076954096 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -107,11 +107,6 @@ 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} - ===================== HeavilyBiasedEngine ===================== @@ -149,9 +144,4 @@ 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 1d661f8e2e9bee4c052b65fae95a7fb4a5456268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 20:34:08 +0200 Subject: [PATCH 2/7] Remove exception in Randomizer::shuffleArray() Same reasoning as in the previous commit applies. --- ext/random/randomizer.c | 1 - ext/random/tests/03_randomizer/user_unsafe.phpt | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index bec38a0444f07..45850478fe88e 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -193,7 +193,6 @@ PHP_METHOD(Random_Randomizer, shuffleArray) ZVAL_DUP(return_value, array); if (!php_array_data_shuffle(randomizer->algo, randomizer->status, return_value)) { - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); RETURN_THROWS(); } } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 8e81076954096..4c0618ec44000 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -95,11 +95,6 @@ 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} - ------- Error: A random engine must return a non-empty string in %s:%d @@ -132,11 +127,6 @@ 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} - ------- Error: Failed to generate an acceptable random number in 50 attempts in %s:%d From 1c9b3c681341df863ed996126cca502b0242c658 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 20:40:10 +0200 Subject: [PATCH 3/7] Remove exception in Randomizer::getInt() Same reasoning as in the previous commit applies. --- ext/random/randomizer.c | 1 - ext/random/tests/03_randomizer/user_unsafe.phpt | 10 ---------- 2 files changed, 11 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 45850478fe88e..058c124bb644e 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -132,7 +132,6 @@ PHP_METHOD(Random_Randomizer, getInt) result = randomizer->algo->range(randomizer->status, min, max); if (EG(exception)) { - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); RETURN_THROWS(); } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 4c0618ec44000..45ab94c938a84 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -71,11 +71,6 @@ Stack trace: #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(0, 123) -#1 {main} - ------- Error: A random engine must return a non-empty string in %s:%d @@ -111,11 +106,6 @@ Stack trace: #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(0, 123) -#1 {main} - ------- string(2) "ff" From 6d8b54b9c8e2e5e226b81133b6bdeda12c0e5c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 1 Aug 2022 19:11:19 +0200 Subject: [PATCH 4/7] Remove exception in Randomizer::nextInt() Same reasoning as in the previous commit applies, except that it won't throw on a seriously biased user engine, as `range()` is not used. --- ext/random/randomizer.c | 7 ++-- .../tests/03_randomizer/user_unsafe.phpt | 33 +++++++++++++++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 058c124bb644e..ab13f413cda81 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -100,12 +100,11 @@ PHP_METHOD(Random_Randomizer, nextInt) ZEND_PARSE_PARAMETERS_NONE(); result = randomizer->algo->generate(randomizer->status); - if (randomizer->status->last_generated_size > sizeof(zend_long)) { - zend_throw_exception(spl_ce_RuntimeException, "Generated value exceeds size of int", 0); + if (EG(exception)) { RETURN_THROWS(); } - if (EG(exception)) { - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); + if (randomizer->status->last_generated_size > sizeof(zend_long)) { + zend_throw_exception(spl_ce_RuntimeException, "Generated value exceeds size of int", 0); RETURN_THROWS(); } diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index 45ab94c938a84..bd6d29876f6dd 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -15,7 +15,7 @@ final class EmptyStringEngine implements \Random\Engine { final class HeavilyBiasedEngine implements \Random\Engine { public function generate(): string { - return "\xff\xff\xff\xff\xff\xff\xff\xff"; + return \str_repeat("\xff", PHP_INT_SIZE); } } @@ -32,25 +32,33 @@ foreach ([ } catch (Throwable $e) { echo $e, PHP_EOL; } - + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - + + try { + var_dump((new Randomizer(new $engine()))->nextInt()); + } catch (Throwable $e) { + echo $e, PHP_EOL; + } + + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; + try { var_dump(bin2hex((new Randomizer(new $engine()))->getBytes(1))); } catch (Throwable $e) { echo $e, PHP_EOL; } - + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - + try { var_dump((new Randomizer(new $engine()))->shuffleArray(\range(1, 10))); } catch (Throwable $e) { echo $e, PHP_EOL; } - + echo PHP_EOL, "-------", PHP_EOL, PHP_EOL; - + try { var_dump((new Randomizer(new $engine()))->shuffleBytes('foobar')); } catch (Throwable $e) { @@ -73,6 +81,13 @@ Stack trace: ------- +Error: A random engine must return a non-empty string in %s:%d +Stack trace: +#0 %s(%d): Random\Randomizer->nextInt() +#1 {main} + +------- + Error: A random engine must return a non-empty string in %s:%d Stack trace: #0 %s(%d): Random\Randomizer->getBytes(1) @@ -108,6 +123,10 @@ Stack trace: ------- +int(%d) + +------- + string(2) "ff" ------- From 21e1a0d4cf79def04c8f1d4f7cd01108c300dd79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 20:36:13 +0200 Subject: [PATCH 5/7] Remove exception in Randomizer::getBytes() Same reasoning as in the previous commit applies. --- ext/random/randomizer.c | 1 - ext/random/tests/03_randomizer/user_throws.phpt | 5 ----- ext/random/tests/03_randomizer/user_unsafe.phpt | 5 ----- 3 files changed, 11 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index ab13f413cda81..7d6690cd7e01a 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -163,7 +163,6 @@ PHP_METHOD(Random_Randomizer, getBytes) result = randomizer->algo->generate(randomizer->status); if (EG(exception)) { zend_string_free(retval); - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); RETURN_THROWS(); } for (size_t i = 0; i < randomizer->status->last_generated_size; i++) { diff --git a/ext/random/tests/03_randomizer/user_throws.phpt b/ext/random/tests/03_randomizer/user_throws.phpt index c0708e0514419..41cd004b63879 100644 --- a/ext/random/tests/03_randomizer/user_throws.phpt +++ b/ext/random/tests/03_randomizer/user_throws.phpt @@ -21,9 +21,4 @@ Stack trace: #0 [internal function]: Random\Engine@anonymous->generate() #1 %s(%d): Random\Randomizer->getBytes(1) #2 {main} - -Next RuntimeException: Random number generation failed in %s:%d -Stack trace: -#0 %s(%d): Random\Randomizer->getBytes(1) -#1 {main} thrown in %s on line %d diff --git a/ext/random/tests/03_randomizer/user_unsafe.phpt b/ext/random/tests/03_randomizer/user_unsafe.phpt index bd6d29876f6dd..e95a15840057d 100644 --- a/ext/random/tests/03_randomizer/user_unsafe.phpt +++ b/ext/random/tests/03_randomizer/user_unsafe.phpt @@ -93,11 +93,6 @@ 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} - ------- Error: A random engine must return a non-empty string in %s:%d From 1b39d5efcd37b4399eb1e121202be204458d08c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 20:41:43 +0200 Subject: [PATCH 6/7] Remove exception in Mt19937::generate() This implementation is shared across all native engines. Thus the same reasoning as the previous commits applies, except that the User engine does not use this method. Thus is only applicable to the Secure engine, which is the only fallible native engine. --- ext/random/engine_mt19937.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index 609efae861b25..bc73513fe216c 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -302,7 +302,6 @@ PHP_METHOD(Random_Engine_Mt19937, generate) generated = engine->algo->generate(engine->status); size = engine->status->last_generated_size; if (EG(exception)) { - zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0); RETURN_THROWS(); } From e6786e9e73a8c495132002699a97de74748e5362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Jul 2022 21:19:34 +0200 Subject: [PATCH 7/7] [ci skip] Add cleanup of Randomizer exceptions to NEWS --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index e755d98d18aee..f55f604503334 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,8 @@ PHP NEWS for invalid $mode. (timwolla) . Splitted Random\Randomizer::getInt() (without arguments) to Random\Randomizer::nextInt(). (zeriyoshi) + . Removed redundant RuntimeExceptions from Randomizer methods. The + exceptions thrown by the engines will be exposed directly. (timwolla) - Sockets: . Added SOL_FILTER socket option for Solaris. (David Carlier)