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 2672477feccb..bec38a0444f0 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 4497088ba9da..8e8107695409 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 bec38a0444f0..45850478fe88 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 8e8107695409..4c0618ec4400 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 45850478fe88..058c124bb644 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 4c0618ec4400..45ab94c938a8 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 058c124bb644..ab13f413cda8 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 45ab94c938a8..bd6d29876f6d 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 ab13f413cda8..7d6690cd7e01 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 c0708e051441..41cd004b6387 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 bd6d29876f6d..e95a15840057 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 609efae861b2..bc73513fe216 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 e755d98d18ae..f55f60450333 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)