From b9bee155a9c68a3c4ba70b9255a3aea5ee2570bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Thu, 27 Oct 2022 00:38:37 +0200 Subject: [PATCH] Fix pre-PHP 8.2 compatibility for php_mt_rand_range() with MT_RAND_PHP 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()`. --- ext/random/engine_mt19937.c | 18 +---------- ext/random/random.c | 16 +++++++++- ext/random/randomizer.c | 17 +++++++++- .../01_functions/array_rand_mt_rand_php.phpt | 32 +++++++++++++++++++ 4 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 ext/random/tests/01_functions/array_rand_mt_rand_php.phpt diff --git a/ext/random/engine_mt19937.c b/ext/random/engine_mt19937.c index 227c1ac02540d..87f96be70abd8 100644 --- a/ext/random/engine_mt19937.c +++ b/ext/random/engine_mt19937.c @@ -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) diff --git a/ext/random/random.c b/ext/random/random.c index 13f1c94c3acf4..161eb8e685203 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -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); } /* }}} */ diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 0e3d90120b6fc..391cc16cc74db 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -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(); } diff --git a/ext/random/tests/01_functions/array_rand_mt_rand_php.phpt b/ext/random/tests/01_functions/array_rand_mt_rand_php.phpt new file mode 100644 index 0000000000000..01a4e3ff5dfcd --- /dev/null +++ b/ext/random/tests/01_functions/array_rand_mt_rand_php.phpt @@ -0,0 +1,32 @@ +--TEST-- +array_rand() is not affected by MT_RAND_PHP as with PHP < 8.2 +--FILE-- + +--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"