From 1cb7aeb9fbd3940662331949c65dc35342ec7be5 Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Fri, 15 Sep 2023 13:29:21 +0200 Subject: [PATCH 1/4] fix random_bytes(0); --- ext/random/random.c | 4 ++-- ext/random/tests/01_functions/random_bytes_error.phpt | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/random/random.c b/ext/random/random.c index f34d55f340727..aeddb438caf08 100644 --- a/ext/random/random.c +++ b/ext/random/random.c @@ -574,8 +574,8 @@ PHP_FUNCTION(random_bytes) Z_PARAM_LONG(size) ZEND_PARSE_PARAMETERS_END(); - if (size < 1) { - zend_argument_value_error(1, "must be greater than 0"); + if (size < 0) { + zend_argument_value_error(1, "must be greater than or equal to 0"); RETURN_THROWS(); } diff --git a/ext/random/tests/01_functions/random_bytes_error.phpt b/ext/random/tests/01_functions/random_bytes_error.phpt index bc0c1ccc20489..325a338d181ee 100644 --- a/ext/random/tests/01_functions/random_bytes_error.phpt +++ b/ext/random/tests/01_functions/random_bytes_error.phpt @@ -3,7 +3,7 @@ Test error operation of random_bytes() --FILE-- getMessage().PHP_EOL; } ?> --EXPECT-- +string(0) "" random_bytes() expects exactly 1 argument, 0 given -random_bytes(): Argument #1 ($length) must be greater than 0 +random_bytes(): Argument #1 ($length) must be greater than or equal to 0 From c56fe83cbfc613682049623624ad4f893341b092 Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Fri, 15 Sep 2023 18:29:18 +0200 Subject: [PATCH 2/4] fix Randomizer::getBytes(0) and other PR feedback --- ext/random/randomizer.c | 12 ++++++------ ext/random/tests/01_functions/random_bytes.phpt | 3 +++ .../tests/01_functions/random_bytes_error.phpt | 3 +-- .../03_randomizer/methods/getBytesFromString.phpt | 7 +++++++ .../methods/getBytesFromString_error.phpt | 8 ++++---- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index fe9ad5fc35a9c..5d5a71c16ca24 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -278,8 +278,8 @@ PHP_METHOD(Random_Randomizer, getBytes) Z_PARAM_LONG(length) ZEND_PARSE_PARAMETERS_END(); - if (length < 1) { - zend_argument_value_error(1, "must be greater than 0"); + if (length < 0) { + zend_argument_value_error(1, "must be greater than or equal to 0"); RETURN_THROWS(); } @@ -390,13 +390,13 @@ PHP_METHOD(Random_Randomizer, getBytesFromString) const size_t source_length = ZSTR_LEN(source); const size_t max_offset = source_length - 1; - if (source_length < 1) { - zend_argument_value_error(1, "cannot be empty"); + if (source_length < 1 && length > 0) { + zend_argument_value_error(1, "cannot be empty when argument #2 ($length) is greater than 0"); RETURN_THROWS(); } - if (length < 1) { - zend_argument_value_error(2, "must be greater than 0"); + if (length < 0) { + zend_argument_value_error(2, "must be greater than or equal to 0"); RETURN_THROWS(); } diff --git a/ext/random/tests/01_functions/random_bytes.phpt b/ext/random/tests/01_functions/random_bytes.phpt index 6a7d438835f95..cb68e167e32bd 100644 --- a/ext/random/tests/01_functions/random_bytes.phpt +++ b/ext/random/tests/01_functions/random_bytes.phpt @@ -4,6 +4,8 @@ Test normal operation of random_bytes() --EXPECT-- +int(0) int(32) bool(true) bool(true) diff --git a/ext/random/tests/01_functions/random_bytes_error.phpt b/ext/random/tests/01_functions/random_bytes_error.phpt index 325a338d181ee..cf9a2a7af0905 100644 --- a/ext/random/tests/01_functions/random_bytes_error.phpt +++ b/ext/random/tests/01_functions/random_bytes_error.phpt @@ -3,7 +3,7 @@ Test error operation of random_bytes() --FILE-- --EXPECT-- -string(0) "" random_bytes() expects exactly 1 argument, 0 given random_bytes(): Argument #1 ($length) must be greater than or equal to 0 diff --git a/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt b/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt index 6a36d20543222..0d2fc7899fc97 100644 --- a/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt @@ -27,6 +27,7 @@ foreach ($engines as $engine) { $randomizer = new Randomizer($engine); var_dump($randomizer->getBytesFromString('a', 10)); var_dump($randomizer->getBytesFromString(str_repeat('a', 256), 5)); + var_dump($randomizer->getBytesFromString('', 0)); for ($i = 1; $i < 250; $i++) { $output = $randomizer->getBytesFromString(str_repeat('ab', $i), 500); @@ -48,19 +49,25 @@ Deprecated: The MT_RAND_PHP variant of Mt19937 is deprecated in %s on line %d Random\Engine\Mt19937 string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" Random\Engine\Mt19937 string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" Random\Engine\PcgOneseq128XslRr64 string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" Random\Engine\Xoshiro256StarStar string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" Random\Engine\Secure string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" Random\Engine\Test\TestShaEngine string(10) "aaaaaaaaaa" string(5) "aaaaa" +string(0) "" success diff --git a/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt b/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt index 7280949d647e8..78ab7318dac5a 100644 --- a/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt @@ -17,12 +17,12 @@ try { } try { - var_dump(randomizer()->getBytesFromString("abc", 0)); + var_dump(randomizer()->getBytesFromString("abc", -1)); } 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 +--EXPECT-- +Random\Randomizer::getBytesFromString(): Argument #1 ($string) cannot be empty when argument #2 ($length) is greater than 0 +Random\Randomizer::getBytesFromString(): Argument #2 ($length) must be greater than or equal to 0 From 326094a99b239aadb3616673e62a01203c68dbc8 Mon Sep 17 00:00:00 2001 From: hanshenrik Date: Fri, 22 Sep 2023 22:31:43 +0200 Subject: [PATCH 3/4] remove support for getBytesFromString("",0); for consistency, I would prefer to actually support this, but TimWolla disagrees, and i cba fighting for it (it's an edge-case after all): https://github.com/php/php-src/pull/12216#discussion_r1327566225 --- ext/random/randomizer.c | 4 ++-- ext/random/tests/03_randomizer/methods/getBytes.phpt | 2 +- .../tests/03_randomizer/methods/getBytesFromString.phpt | 2 +- .../tests/03_randomizer/methods/getBytesFromString_error.phpt | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/random/randomizer.c b/ext/random/randomizer.c index 5d5a71c16ca24..a00648b79558b 100644 --- a/ext/random/randomizer.c +++ b/ext/random/randomizer.c @@ -390,8 +390,8 @@ PHP_METHOD(Random_Randomizer, getBytesFromString) const size_t source_length = ZSTR_LEN(source); const size_t max_offset = source_length - 1; - if (source_length < 1 && length > 0) { - zend_argument_value_error(1, "cannot be empty when argument #2 ($length) is greater than 0"); + if (source_length < 1) { + zend_argument_value_error(1, "cannot be empty"); RETURN_THROWS(); } diff --git a/ext/random/tests/03_randomizer/methods/getBytes.phpt b/ext/random/tests/03_randomizer/methods/getBytes.phpt index d4fbf7c7c8ca9..4149aff181e53 100644 --- a/ext/random/tests/03_randomizer/methods/getBytes.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytes.phpt @@ -27,7 +27,7 @@ foreach ($engines as $engine) { $randomizer = new Randomizer($engine); // Using 10_000 is very slow. - for ($i = 1; $i < 1_000; $i++) { + for ($i = 0; $i < 1_000; ++$i) { if (\strlen($randomizer->getBytes($i)) !== $i) { die("failure: incorrect string length at {$i}"); } diff --git a/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt b/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt index 0d2fc7899fc97..d2f85585fd7b1 100644 --- a/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytesFromString.phpt @@ -27,7 +27,7 @@ foreach ($engines as $engine) { $randomizer = new Randomizer($engine); var_dump($randomizer->getBytesFromString('a', 10)); var_dump($randomizer->getBytesFromString(str_repeat('a', 256), 5)); - var_dump($randomizer->getBytesFromString('', 0)); + var_dump($randomizer->getBytesFromString('a', 0)); for ($i = 1; $i < 250; $i++) { $output = $randomizer->getBytesFromString(str_repeat('ab', $i), 500); diff --git a/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt b/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt index 78ab7318dac5a..566d3b56be58c 100644 --- a/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytesFromString_error.phpt @@ -24,5 +24,5 @@ try { ?> --EXPECT-- -Random\Randomizer::getBytesFromString(): Argument #1 ($string) cannot be empty when argument #2 ($length) is greater than 0 +Random\Randomizer::getBytesFromString(): Argument #1 ($string) cannot be empty Random\Randomizer::getBytesFromString(): Argument #2 ($length) must be greater than or equal to 0 From edeeb3d96f99097e3231d844d75bb901483c5402 Mon Sep 17 00:00:00 2001 From: divinity76 Date: Sun, 24 Sep 2023 09:38:06 +0200 Subject: [PATCH 4/4] Update ext/random/tests/03_randomizer/methods/getBytes.phpt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tim Düsterhus --- ext/random/tests/03_randomizer/methods/getBytes.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/random/tests/03_randomizer/methods/getBytes.phpt b/ext/random/tests/03_randomizer/methods/getBytes.phpt index 4149aff181e53..b1a066e7b40be 100644 --- a/ext/random/tests/03_randomizer/methods/getBytes.phpt +++ b/ext/random/tests/03_randomizer/methods/getBytes.phpt @@ -27,7 +27,7 @@ foreach ($engines as $engine) { $randomizer = new Randomizer($engine); // Using 10_000 is very slow. - for ($i = 0; $i < 1_000; ++$i) { + for ($i = 0; $i < 1_000; $i++) { if (\strlen($randomizer->getBytes($i)) !== $i) { die("failure: incorrect string length at {$i}"); }