Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117955
Message-ID: <e96c8676-d5fb-f3c9-d0ca-a93f8ec0f1c1@bastelstu.be>
Date: Wed, 15 Jun 2022 19:23:12 +0200
MIME-Version: 1.0
Content-Language: en-US
To: Go Kudo <g-kudo@colopl.co.jp>, internals@lists.php.net
References: <CAMs36Het5f=ojSw8H4kooZxub-COvhKmiJfYUUM7ioBSpVKFVA@mail.gmail.com>
In-Reply-To: <CAMs36Het5f=ojSw8H4kooZxub-COvhKmiJfYUUM7ioBSpVKFVA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] [RFC] [VOTE] Random Extension 5.x
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 6/14/22 02:01, Go Kudo wrote:
> Voting began on 2022-06-14 00:00:00 (UTC) and will end on 2022-06-28
> 00:00:00 (UTC).
> 
> https://wiki.php.net/rfc/rng_extension
> 
> The implementation is not yet complete and has some issues.
> See TODO in Pull Request for details.
> 
> https://github.com/php/php-src/pull/8094
> 

Unfortunately the vote has already started and I'm not sure if that's a 
change that might change the outcome of the vote, but while looking 
through the implementation once more I noticed that the engine 
implementations are not 'final' (and extending those engines is actually 
tested with the existing tests).

However I believe they should be final:

a) I generally believe that it's a best practice to make everything 
'final' by default.

b) It's easily possible to use composition with engines, as the 
interface only has a single method.

c) Especially for 'Random\Engine\Secure' I believe that allowing 
subclassing is actively harmful, as basically any adjustment of the 
engine's behavior violates the contract that the engine returns 
cryptographically secure randomness. But also for other engines changing 
the behavior also changes the implied behavior given by the engine's name.

What do you think?

Best regards
Tim Düsterhus