Skip to content

Commit 13b82ee

Browse files
TimWollacmb69
andauthoredJan 10, 2023
random: Randomizer::getFloat(): Fix check for empty open intervals (#10185)
* random: Randomizer::getFloat(): Fix check for empty open intervals The check for invalid parameters for the IntervalBoundary::OpenOpen variant was not correct: If two consecutive doubles are passed as parameters, the resulting interval is empty, resulting in an uint64 underflow in the γ-section implementation. Instead of checking whether `$min < $max`, we must check that there is at least one more double between `$min` and `$max`, i.e. it must hold that: nextafter($min, $max) != $max Instead of duplicating the comparatively complicated and expensive `nextafter` logic for a rare error case we instead return `NAN` from the γ-section implementation when the parameters result in an empty interval and thus underflow. This allows us to reliably detect this specific error case *after* the fact, but without modifying the engine state. It also provides reliable error reporting for other internal functions that might use the γ-section implementation. * random: γ-section: Also check that that min is smaller than max This extends the empty-interval check in the γ-section implementation with a check that min is actually the smaller of the two parameters. * random: Use PHP_FLOAT_EPSILON in getFloat_error.phpt Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>
1 parent 4280431 commit 13b82ee

File tree

3 files changed

+40
-2
lines changed

3 files changed

+40
-2
lines changed
 

‎ext/random/gammasection.c

+20
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ PHPAPI double php_random_gammasection_closed_open(const php_random_algo *algo, p
6666
{
6767
double g = gamma_max(min, max);
6868
uint64_t hi = ceilint(min, max, g);
69+
70+
if (UNEXPECTED(max <= min || hi < 1)) {
71+
return NAN;
72+
}
73+
6974
uint64_t k = 1 + php_random_range64(algo, status, hi - 1); /* [1, hi] */
7075

7176
if (fabs(min) <= fabs(max)) {
@@ -79,6 +84,11 @@ PHPAPI double php_random_gammasection_closed_closed(const php_random_algo *algo,
7984
{
8085
double g = gamma_max(min, max);
8186
uint64_t hi = ceilint(min, max, g);
87+
88+
if (UNEXPECTED(max < min)) {
89+
return NAN;
90+
}
91+
8292
uint64_t k = php_random_range64(algo, status, hi); /* [0, hi] */
8393

8494
if (fabs(min) <= fabs(max)) {
@@ -92,6 +102,11 @@ PHPAPI double php_random_gammasection_open_closed(const php_random_algo *algo, p
92102
{
93103
double g = gamma_max(min, max);
94104
uint64_t hi = ceilint(min, max, g);
105+
106+
if (UNEXPECTED(max <= min || hi < 1)) {
107+
return NAN;
108+
}
109+
95110
uint64_t k = php_random_range64(algo, status, hi - 1); /* [0, hi - 1] */
96111

97112
if (fabs(min) <= fabs(max)) {
@@ -105,6 +120,11 @@ PHPAPI double php_random_gammasection_open_open(const php_random_algo *algo, php
105120
{
106121
double g = gamma_max(min, max);
107122
uint64_t hi = ceilint(min, max, g);
123+
124+
if (UNEXPECTED(max <= min || hi < 2)) {
125+
return NAN;
126+
}
127+
108128
uint64_t k = 1 + php_random_range64(algo, status, hi - 2); /* [1, hi - 1] */
109129

110130
if (fabs(min) <= fabs(max)) {

‎ext/random/randomizer.c

+8-1
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,14 @@ PHP_METHOD(Random_Randomizer, getFloat)
190190
RETURN_THROWS();
191191
}
192192

193-
RETURN_DOUBLE(php_random_gammasection_open_open(randomizer->algo, randomizer->status, min, max));
193+
RETVAL_DOUBLE(php_random_gammasection_open_open(randomizer->algo, randomizer->status, min, max));
194+
195+
if (UNEXPECTED(isnan(Z_DVAL_P(return_value)))) {
196+
zend_value_error("The given interval is empty, there are no floats between argument #1 ($min) and argument #2 ($max).");
197+
RETURN_THROWS();
198+
}
199+
200+
return;
194201
default:
195202
ZEND_UNREACHABLE();
196203
}

‎ext/random/tests/03_randomizer/methods/getFloat_error.phpt

+12-1
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,17 @@ foreach ([
7373
} catch (ValueError $e) {
7474
echo $e->getMessage(), PHP_EOL;
7575
}
76+
77+
try {
78+
// There is no float between the two parameters, thus making the OpenOpen interval empty.
79+
var_dump(randomizer()->getFloat(1.0, 1 + PHP_FLOAT_EPSILON, $boundary));
80+
} catch (ValueError $e) {
81+
echo $e->getMessage(), PHP_EOL;
82+
}
7683
}
7784

7885
?>
79-
--EXPECT--
86+
--EXPECTF--
8087
ClosedClosed
8188
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
8289
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
@@ -87,6 +94,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite
8794
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than or equal to argument #1 ($min)
8895
float(0)
8996
float(1.0E+17)
97+
float(%f)
9098
ClosedOpen
9199
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
92100
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
@@ -97,6 +105,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite
97105
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
98106
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
99107
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
108+
float(1)
100109
OpenClosed
101110
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
102111
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
@@ -107,6 +116,7 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite
107116
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
108117
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
109118
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
119+
float(1.0000000000000002)
110120
OpenOpen
111121
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
112122
Random\Randomizer::getFloat(): Argument #1 ($min) must be finite
@@ -117,3 +127,4 @@ Random\Randomizer::getFloat(): Argument #2 ($max) must be finite
117127
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
118128
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
119129
Random\Randomizer::getFloat(): Argument #2 ($max) must be greater than argument #1 ($min)
130+
The given interval is empty, there are no floats between argument #1 ($min) and argument #2 ($max).

0 commit comments

Comments
 (0)
Please sign in to comment.