fix random_bytes(0) and Random\Randomizer::getBytes(0)

[divinity76]

random_bytes(0); // emptystring. previously: ValueError: random_bytes(): Argument #1 ($length) must be greater than 0

(new Random\Randomizer())->getBytes(0); // emptystring. previously: ValueError: Random\Randomizer::getBytes(): Argument #1 ($length) must be greater than 0

(new Random\Randomizer())->getBytesFromString("", 0); // emptystring. previously: ValueError (...)

(new Random\Randomizer())->getBytesFromString("", 1); // still a ValueError :)

[TimWolla]

Reference to the old bug tracker for prior discussion: https://bugs.php.net/bug.php?id=80214

[TimWolla]

Randomizer::getBytes() should be adjusted for consistency. I agree that retrieving zero random bytes is not per se wrong, but I understand the concerns in the previous bug. I'll give this some thought, this is a preliminary technical review.

[divinity76]

Randomizer::getBytes() should be adjusted for consistency. I agree that retrieving zero random bytes is not per se wrong, but I understand the concerns in the previous bug. I'll give this some thought, this is a preliminary technical review.

added 0 support for Randomizer, please re-check the code when convenient

[TimWolla]

I've now tested the branch locally and looked through the related bug report and have a question about this:

What should the behavior of

<?php

use Random\Engine;
use Random\Randomizer;

final class ThrowingEngine implements Engine
{
    public function generate(): string
    {
        throw new Exception('Error');
    }
} 

$randomizer = new Randomizer(new ThrowingEngine());

var_dump($randomizer->getBytes(0));

be? And likely what should the behavior of random_bytes(0) be if the CSPRNG fails (both getrandom() and /dev/urandom) unavailable? It would probably be a good thing if random_bytes(0) would allow to test if the CSPRNG is functional.

On the other hand, Randomizer::shuffleBytes() and Randomizer::shuffleArray() do not currently make any calls to the engine for an empty string and empty array respectively.

Unfortunately the last commenter on the bug report just said that:

I can think of at least three potential issues if php_random_bytes(0) always succeeded

But didn't spell out what issues they were thinking of.

This probably does not require a full-blown RFC, but a discussion on the PHP Internals mailing list would probably be appropriate to decide on the desired semantics.

[TimWolla]

On the other hand, Randomizer::shuffleBytes() and Randomizer::shuffleArray() do not currently make any calls to the engine for an empty string and empty array respectively.

In fact they don't do for single-element strings or arrays either.

[TimWolla]

One more question: Should zero also be legal for Randomizer::pickArrayKeys()'s $num parameter?