Skip to content

Commit

Permalink
Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP
Browse files Browse the repository at this point in the history
As some left-over comments indicated:

> Legacy mode deliberately not inside php_mt_rand_range()
> to prevent other functions being affected

The broken scaler was only used for `php_mt_rand_common()`, not
`php_mt_rand_range()`. The former is only used for `mt_rand()`, whereas the
latter is used for `array_rand()` and others.

With the refactoring for the introduction of ext/random `php_mt_rand_common()`
and `php_mt_rand_range()` were accidentally unified, thus introducing a
behavioral change that was reported in FakerPHP/Faker#528.

This commit moves the checks for `MT_RAND_PHP` from the general-purpose
`range()` function back into `php_mt_rand_common()` and also into
`Randomizer::getInt()` for drop-in compatibility with `mt_rand()`.
  • Loading branch information
TimWolla committed Oct 26, 2022
1 parent 1ba80ff commit b9bee15
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 19 deletions.
18 changes: 1 addition & 17 deletions ext/random/engine_mt19937.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,23 +162,7 @@ static uint64_t generate(php_random_status *status)

static zend_long range(php_random_status *status, zend_long min, zend_long max)
{
php_random_status_state_mt19937 *s = status->state;

if (s->mode == MT_RAND_MT19937) {
return php_random_range(&php_random_algo_mt19937, status, min, max);
}

/* Legacy mode deliberately not inside php_mt_rand_range()
* to prevent other functions being affected */

uint64_t r = php_random_algo_mt19937.generate(status) >> 1;

/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
* (max - min) > ZEND_LONG_MAX.
*/
zend_ulong offset = (double) ( (double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0));

return (zend_long) (offset + min);
return php_random_range(&php_random_algo_mt19937, status, min, max);
}

static bool serialize(php_random_status *status, HashTable *data)
Expand Down
16 changes: 15 additions & 1 deletion ext/random/random.c
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,21 @@ PHPAPI zend_long php_mt_rand_range(zend_long min, zend_long max)
* rand() allows min > max, mt_rand does not */
PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max)
{
return php_mt_rand_range(min, max);
php_random_status *status = php_random_default_status();
php_random_status_state_mt19937 *s = status->state;

if (s->mode == MT_RAND_MT19937) {
return php_mt_rand_range(min, max);
}

uint64_t r = php_random_algo_mt19937.generate(php_random_default_status()) >> 1;

/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
* (max - min) > ZEND_LONG_MAX.
*/
zend_ulong offset = (double) ( (double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0));

return (zend_long) (offset + min);
}
/* }}} */

Expand Down
17 changes: 16 additions & 1 deletion ext/random/randomizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,22 @@ PHP_METHOD(Random_Randomizer, getInt)
RETURN_THROWS();
}

result = randomizer->algo->range(randomizer->status, min, max);
if (UNEXPECTED(
randomizer->algo->range == php_random_algo_mt19937.range
&& ((php_random_status_state_mt19937 *) randomizer->status->state)->mode != MT_RAND_MT19937
)) {
uint64_t r = php_random_algo_mt19937.generate(randomizer->status) >> 1;

/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
* (max - min) > ZEND_LONG_MAX.
*/
zend_ulong offset = (double) ( (double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0));

result = (zend_long) (offset + min);
} else {
result = randomizer->algo->range(randomizer->status, min, max);
}

if (EG(exception)) {
RETURN_THROWS();
}
Expand Down
32 changes: 32 additions & 0 deletions ext/random/tests/01_functions/array_rand_mt_rand_php.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
array_rand() is not affected by MT_RAND_PHP as with PHP < 8.2
--FILE--
<?php

$array = [
'foo',
'bar',
'baz',
];

mt_srand(1, MT_RAND_PHP);
function custom_array_rand() {
global $array;
$key = array_rand($array);
var_dump('found key ' . $key);
return $array[$key];
}

var_dump(
custom_array_rand(),
custom_array_rand(),
custom_array_rand(),
);
?>
--EXPECTF--
string(11) "found key 0"
string(11) "found key 1"
string(11) "found key 0"
string(3) "foo"
string(3) "bar"
string(3) "foo"

0 comments on commit b9bee15

Please sign in to comment.