Skip to content

Commit 54e406c

Browse files
TimWollaTimWolla
authored
Clean up nested exceptions without value-add in ext/random (#9211)
* 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. * Remove exception in Randomizer::shuffleArray() Same reasoning as in the previous commit applies. * Remove exception in Randomizer::getInt() Same reasoning as in the previous commit applies. * 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. * Remove exception in Randomizer::getBytes() Same reasoning as in the previous commit applies. * 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. * [ci skip] Add cleanup of Randomizer exceptions to NEWS
1 parent 150456e commit 54e406c

File tree

5 files changed

+27
-52
lines changed

5 files changed

+27
-52
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ PHP NEWS
5656
for invalid $mode. (timwolla)
5757
. Splitted Random\Randomizer::getInt() (without arguments) to
5858
Random\Randomizer::nextInt(). (zeriyoshi)
59+
. Removed redundant RuntimeExceptions from Randomizer methods. The
60+
exceptions thrown by the engines will be exposed directly. (timwolla)
5961

6062
- Sockets:
6163
. Added SOL_FILTER socket option for Solaris. (David Carlier)

‎ext/random/engine_mt19937.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ PHP_METHOD(Random_Engine_Mt19937, generate)
302302
generated = engine->algo->generate(engine->status);
303303
size = engine->status->last_generated_size;
304304
if (EG(exception)) {
305-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
306305
RETURN_THROWS();
307306
}
308307

‎ext/random/randomizer.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,11 @@ PHP_METHOD(Random_Randomizer, nextInt)
100100
ZEND_PARSE_PARAMETERS_NONE();
101101

102102
result = randomizer->algo->generate(randomizer->status);
103-
if (randomizer->status->last_generated_size > sizeof(zend_long)) {
104-
zend_throw_exception(spl_ce_RuntimeException, "Generated value exceeds size of int", 0);
103+
if (EG(exception)) {
105104
RETURN_THROWS();
106105
}
107-
if (EG(exception)) {
108-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
106+
if (randomizer->status->last_generated_size > sizeof(zend_long)) {
107+
zend_throw_exception(spl_ce_RuntimeException, "Generated value exceeds size of int", 0);
109108
RETURN_THROWS();
110109
}
111110

@@ -132,7 +131,6 @@ PHP_METHOD(Random_Randomizer, getInt)
132131

133132
result = randomizer->algo->range(randomizer->status, min, max);
134133
if (EG(exception)) {
135-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
136134
RETURN_THROWS();
137135
}
138136

@@ -165,7 +163,6 @@ PHP_METHOD(Random_Randomizer, getBytes)
165163
result = randomizer->algo->generate(randomizer->status);
166164
if (EG(exception)) {
167165
zend_string_free(retval);
168-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
169166
RETURN_THROWS();
170167
}
171168
for (size_t i = 0; i < randomizer->status->last_generated_size; i++) {
@@ -193,7 +190,6 @@ PHP_METHOD(Random_Randomizer, shuffleArray)
193190

194191
ZVAL_DUP(return_value, array);
195192
if (!php_array_data_shuffle(randomizer->algo, randomizer->status, return_value)) {
196-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
197193
RETURN_THROWS();
198194
}
199195
}
@@ -215,7 +211,6 @@ PHP_METHOD(Random_Randomizer, shuffleBytes)
215211

216212
RETVAL_STRINGL(ZSTR_VAL(bytes), ZSTR_LEN(bytes));
217213
if (!php_binary_string_shuffle(randomizer->algo, randomizer->status, Z_STRVAL_P(return_value), (zend_long) Z_STRLEN_P(return_value))) {
218-
zend_throw_exception(spl_ce_RuntimeException, "Random number generation failed", 0);
219214
RETURN_THROWS();
220215
}
221216
}

‎ext/random/tests/03_randomizer/user_throws.phpt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,4 @@ Stack trace:
2121
#0 [internal function]: Random\Engine@anonymous->generate()
2222
#1 %s(%d): Random\Randomizer->getBytes(1)
2323
#2 {main}
24-
25-
Next RuntimeException: Random number generation failed in %s:%d
26-
Stack trace:
27-
#0 %s(%d): Random\Randomizer->getBytes(1)
28-
#1 {main}
2924
thrown in %s on line %d

‎ext/random/tests/03_randomizer/user_unsafe.phpt

Lines changed: 22 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ final class EmptyStringEngine implements \Random\Engine {
1515
final class HeavilyBiasedEngine implements \Random\Engine {
1616
public function generate(): string
1717
{
18-
return "\xff\xff\xff\xff\xff\xff\xff\xff";
18+
return \str_repeat("\xff", PHP_INT_SIZE);
1919
}
2020
}
2121

@@ -32,25 +32,33 @@ foreach ([
3232
} catch (Throwable $e) {
3333
echo $e, PHP_EOL;
3434
}
35-
35+
3636
echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;
37-
37+
38+
try {
39+
var_dump((new Randomizer(new $engine()))->nextInt());
40+
} catch (Throwable $e) {
41+
echo $e, PHP_EOL;
42+
}
43+
44+
echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;
45+
3846
try {
3947
var_dump(bin2hex((new Randomizer(new $engine()))->getBytes(1)));
4048
} catch (Throwable $e) {
4149
echo $e, PHP_EOL;
4250
}
43-
51+
4452
echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;
45-
53+
4654
try {
4755
var_dump((new Randomizer(new $engine()))->shuffleArray(\range(1, 10)));
4856
} catch (Throwable $e) {
4957
echo $e, PHP_EOL;
5058
}
51-
59+
5260
echo PHP_EOL, "-------", PHP_EOL, PHP_EOL;
53-
61+
5462
try {
5563
var_dump((new Randomizer(new $engine()))->shuffleBytes('foobar'));
5664
} catch (Throwable $e) {
@@ -71,19 +79,16 @@ Stack trace:
7179
#0 %s(%d): Random\Randomizer->getInt(0, 123)
7280
#1 {main}
7381

74-
Next RuntimeException: Random number generation failed in %s:%d
75-
Stack trace:
76-
#0 %s(%d): Random\Randomizer->getInt(0, 123)
77-
#1 {main}
78-
7982
-------
8083

8184
Error: A random engine must return a non-empty string in %s:%d
8285
Stack trace:
83-
#0 %s(%d): Random\Randomizer->getBytes(1)
86+
#0 %s(%d): Random\Randomizer->nextInt()
8487
#1 {main}
8588

86-
Next RuntimeException: Random number generation failed in %s:%d
89+
-------
90+
91+
Error: A random engine must return a non-empty string in %s:%d
8792
Stack trace:
8893
#0 %s(%d): Random\Randomizer->getBytes(1)
8994
#1 {main}
@@ -95,23 +100,13 @@ Stack trace:
95100
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
96101
#1 {main}
97102

98-
Next RuntimeException: Random number generation failed in %s:%d
99-
Stack trace:
100-
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
101-
#1 {main}
102-
103103
-------
104104

105105
Error: A random engine must return a non-empty string in %s:%d
106106
Stack trace:
107107
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
108108
#1 {main}
109109

110-
Next RuntimeException: Random number generation failed in %s:%d
111-
Stack trace:
112-
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
113-
#1 {main}
114-
115110
=====================
116111
HeavilyBiasedEngine
117112
=====================
@@ -121,10 +116,9 @@ Stack trace:
121116
#0 %s(%d): Random\Randomizer->getInt(0, 123)
122117
#1 {main}
123118

124-
Next RuntimeException: Random number generation failed in %s:%d
125-
Stack trace:
126-
#0 %s(%d): Random\Randomizer->getInt(0, 123)
127-
#1 {main}
119+
-------
120+
121+
int(%d)
128122

129123
-------
130124

@@ -137,21 +131,11 @@ Stack trace:
137131
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
138132
#1 {main}
139133

140-
Next RuntimeException: Random number generation failed in %s:%d
141-
Stack trace:
142-
#0 %s(%d): Random\Randomizer->shuffleArray(Array)
143-
#1 {main}
144-
145134
-------
146135

147136
Error: Failed to generate an acceptable random number in 50 attempts in %s:%d
148137
Stack trace:
149138
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
150139
#1 {main}
151140

152-
Next RuntimeException: Random number generation failed in %s:%d
153-
Stack trace:
154-
#0 %s(%d): Random\Randomizer->shuffleBytes('foobar')
155-
#1 {main}
156-
157141
=====================

0 commit comments

Comments
 (0)