Skip to content

Add Randomizer::getBytesFromString() method #9664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 9, 2022
Merged

Add Randomizer::getBytesFromString() method #9664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d69c3c0
Add `Randomizer::getBytesFromAlphabet()` method
joshuaruesweg Sep 26, 2022
0baf646
Rename `getBytesFromAlphabet` to `getBytesFromString`
joshuaruesweg Nov 7, 2022
96ad925
[ci skip] Add NEWS/UPGRADING for Randomizer::getBytesFromString()
TimWolla Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ PHP NEWS
- Posix:
. Added posix_sysconf. (David Carlier)

- Random:
. Added Randomizer::getBytesFromString(). (Joshua Rüsweg)

- Reflection:
. Fix GH-9470 (ReflectionMethod constructor should not find private parent
method). (ilutov)
Expand Down
4 changes: 4 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ PHP 8.3 UPGRADE NOTES
- Posix:
. Added posix_sysconf call to get runtime informations.

- Random:
. Added Randomizer::getBytesFromString().
RFC: https://wiki.php.net/rfc/randomizer_additions

- Sockets:
. Added socket_atmark to checks if the socket is OOB marked.

Expand Down
2 changes: 2 additions & 0 deletions ext/random/php_random.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ PHPAPI double php_combined_lcg(void);

# define MT_N (624)

#define PHP_RANDOM_RANGE_ATTEMPTS (50)

PHPAPI void php_mt_srand(uint32_t seed);
PHPAPI uint32_t php_mt_rand(void);
PHPAPI zend_long php_mt_rand_range(zend_long min, zend_long max);
Expand Down
10 changes: 4 additions & 6 deletions ext/random/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ static zend_object_handlers random_engine_xoshiro256starstar_object_handlers;
static zend_object_handlers random_engine_secure_object_handlers;
static zend_object_handlers random_randomizer_object_handlers;

#define RANDOM_RANGE_ATTEMPTS (50)

static inline uint32_t rand_range32(const php_random_algo *algo, php_random_status *status, uint32_t umax)
{
uint32_t result, limit;
Expand Down Expand Up @@ -124,8 +122,8 @@ static inline uint32_t rand_range32(const php_random_algo *algo, php_random_stat
/* Discard numbers over the limit to avoid modulo bias */
while (UNEXPECTED(result > limit)) {
/* If the requirements cannot be met in a cycles, return fail */
if (++count > RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS);
if (++count > PHP_RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", PHP_RANDOM_RANGE_ATTEMPTS);
return 0;
}

Expand Down Expand Up @@ -180,8 +178,8 @@ static inline uint64_t rand_range64(const php_random_algo *algo, php_random_stat
/* Discard numbers over the limit to avoid modulo bias */
while (UNEXPECTED(result > limit)) {
/* If the requirements cannot be met in a cycles, return fail */
if (++count > RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", RANDOM_RANGE_ATTEMPTS);
if (++count > PHP_RANDOM_RANGE_ATTEMPTS) {
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", PHP_RANDOM_RANGE_ATTEMPTS);
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions ext/random/random.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ public function getInt(int $min, int $max): int {}

public function getBytes(int $length): string {}

public function getBytesFromString(string $string, int $length): string {}

public function shuffleArray(array $array): array {}

public function shuffleBytes(string $bytes): string {}
Expand Down
9 changes: 8 additions & 1 deletion ext/random/random_arginfo.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

98 changes: 97 additions & 1 deletion ext/random/randomizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ PHP_METHOD(Random_Randomizer, getInt)
&& ((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.
*/
Expand Down Expand Up @@ -258,6 +258,102 @@ PHP_METHOD(Random_Randomizer, pickArrayKeys)
}
/* }}} */

/* {{{ Get Random Bytes for String */
PHP_METHOD(Random_Randomizer, getBytesFromString)
{
php_random_randomizer *randomizer = Z_RANDOM_RANDOMIZER_P(ZEND_THIS);
zend_long length;
zend_string *source, *retval;
size_t total_size = 0;

ZEND_PARSE_PARAMETERS_START(2, 2);
Z_PARAM_STR(source)
Z_PARAM_LONG(length)
ZEND_PARSE_PARAMETERS_END();

const size_t source_length = ZSTR_LEN(source);

if (source_length < 1) {
zend_argument_value_error(1, "cannot be empty");
RETURN_THROWS();
}

if (length < 1) {
zend_argument_value_error(2, "must be greater than 0");
RETURN_THROWS();
}

retval = zend_string_alloc(length, 0);

if (source_length > 0xFF) {
while (total_size < length) {
uint64_t offset = randomizer->algo->range(randomizer->status, 0, source_length - 1);

if (EG(exception)) {
zend_string_free(retval);
RETURN_THROWS();
}

ZSTR_VAL(retval)[total_size++] = ZSTR_VAL(source)[offset];
}
} else {
uint64_t mask;
if (source_length <= 0x1) {
mask = 0x0;
} else if (source_length <= 0x2) {
mask = 0x1;
} else if (source_length <= 0x4) {
mask = 0x3;
} else if (source_length <= 0x8) {
mask = 0x7;
} else if (source_length <= 0x10) {
mask = 0xF;
} else if (source_length <= 0x20) {
mask = 0x1F;
} else if (source_length <= 0x40) {
mask = 0x3F;
} else if (source_length <= 0x80) {
mask = 0x7F;
} else {
mask = 0xFF;
}
Comment on lines +300 to +319
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this isn't particularly performant. I wonder whether we should optimize that using __builtin_clz (and _BitScanReverse on Windows). We already have a few usages in php-src (see e.g. zend_mm_small_size_to_bit()), and it may make sense to unify these (to better encapsulate the usage of the intrinsics).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmb69 I noted the same at the end of my previous comment here: #9664 (comment).

While the current if-elseif-else-construction contains several hard-to-predict branches, the performance of it is likely to be dwarfed by the loop that actually selects the random bytes.

I'd likely optimize this in a separate PR when the need arises or when there is a good API for clz. zend_ulong_nlz is not great, because it takes zend_ulong which size differs depending on the platform.


int failures = 0;
while (total_size < length) {
uint64_t result = randomizer->algo->generate(randomizer->status);
if (EG(exception)) {
zend_string_free(retval);
RETURN_THROWS();
}

for (size_t i = 0; i < randomizer->status->last_generated_size; i++) {
uint64_t offset = (result >> (i * 8)) & mask;

if (offset >= source_length) {
if (++failures > PHP_RANDOM_RANGE_ATTEMPTS) {
zend_string_free(retval);
zend_throw_error(random_ce_Random_BrokenRandomEngineError, "Failed to generate an acceptable random number in %d attempts", PHP_RANDOM_RANGE_ATTEMPTS);
RETURN_THROWS();
}

continue;
}

failures = 0;

ZSTR_VAL(retval)[total_size++] = ZSTR_VAL(source)[offset];
if (total_size >= length) {
break;
}
}
}
}

ZSTR_VAL(retval)[length] = '\0';
RETURN_STR(retval);
}
/* }}} */

/* {{{ Random\Randomizer::__serialize() */
PHP_METHOD(Random_Randomizer, __serialize)
{
Expand Down
14 changes: 14 additions & 0 deletions ext/random/tests/03_randomizer/engine_unsafe_biased.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,24 @@ try {
echo $e->getMessage(), PHP_EOL;
}

try {
var_dump(randomizer()->getBytesFromString('123', 10));
} catch (Random\BrokenRandomEngineError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
var_dump(randomizer()->getBytesFromString(str_repeat('a', 500), 10));
} catch (Random\BrokenRandomEngineError $e) {
echo $e->getMessage(), PHP_EOL;
}

?>
--EXPECTF--
Failed to generate an acceptable random number in 50 attempts
int(%d)
string(2) "ff"
Failed to generate an acceptable random number in 50 attempts
Failed to generate an acceptable random number in 50 attempts
Failed to generate an acceptable random number in 50 attempts
Failed to generate an acceptable random number in 50 attempts
14 changes: 14 additions & 0 deletions ext/random/tests/03_randomizer/engine_unsafe_empty_string.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,24 @@ try {
echo $e->getMessage(), PHP_EOL;
}

try {
var_dump(randomizer()->getBytesFromString('123', 10));
} catch (Random\BrokenRandomEngineError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
var_dump(randomizer()->getBytesFromString(str_repeat('a', 500), 10));
} catch (Random\BrokenRandomEngineError $e) {
echo $e->getMessage(), PHP_EOL;
}

?>
--EXPECT--
A random engine must return a non-empty string
A random engine must return a non-empty string
A random engine must return a non-empty string
A random engine must return a non-empty string
A random engine must return a non-empty string
A random engine must return a non-empty string
A random engine must return a non-empty string
63 changes: 63 additions & 0 deletions ext/random/tests/03_randomizer/methods/getBytesFromString.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
--TEST--
Random: Randomizer: getBytesFromString(): Basic functionality
--FILE--
<?php

use Random\Engine;
use Random\Engine\Mt19937;
use Random\Engine\PcgOneseq128XslRr64;
use Random\Engine\Secure;
use Random\Engine\Test\TestShaEngine;
use Random\Engine\Xoshiro256StarStar;
use Random\Randomizer;

require __DIR__ . "/../../engines.inc";

$engines = [];
$engines[] = new Mt19937(null, MT_RAND_MT19937);
$engines[] = new Mt19937(null, MT_RAND_PHP);
$engines[] = new PcgOneseq128XslRr64();
$engines[] = new Xoshiro256StarStar();
$engines[] = new Secure();
$engines[] = new TestShaEngine();

foreach ($engines as $engine) {
echo $engine::class, PHP_EOL;

$randomizer = new Randomizer($engine);
var_dump($randomizer->getBytesFromString('a', 10));
var_dump($randomizer->getBytesFromString(str_repeat('a', 256), 5));

for ($i = 1; $i < 250; $i++) {
$output = $randomizer->getBytesFromString(str_repeat('ab', $i), 500);

// This check can theoretically fail with a chance of 0.5**500.
if (!str_contains($output, 'a') || !str_contains($output, 'b')) {
die("failure: didn't see both a and b at {$i}");
}
}
}

die('success');

?>
--EXPECT--
Random\Engine\Mt19937
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
Random\Engine\Mt19937
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
Random\Engine\PcgOneseq128XslRr64
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
Random\Engine\Xoshiro256StarStar
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
Random\Engine\Secure
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
Random\Engine\Test\TestShaEngine
string(10) "aaaaaaaaaa"
string(5) "aaaaa"
success
28 changes: 28 additions & 0 deletions ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--TEST--
Random: Randomizer: getBytesFromString(): Parameters are correctly validated
--FILE--
<?php

use Random\Randomizer;

function randomizer(): Randomizer
{
return new Randomizer();
}

try {
var_dump(randomizer()->getBytesFromString("", 2));
} catch (ValueError $e) {
echo $e->getMessage(), PHP_EOL;
}

try {
var_dump(randomizer()->getBytesFromString("abc", 0));
} catch (ValueError $e) {
echo $e->getMessage(), PHP_EOL;
}

?>
--EXPECTF--
Random\Randomizer::getBytesFromString(): Argument #1 ($string) cannot be empty
Random\Randomizer::getBytesFromString(): Argument #2 ($length) must be greater than 0