Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117051
Content-Type: multipart/mixed; boundary="------------FglZYnek08KnxIlsazWA72Pu"
Message-ID: <a1369ca5-9d27-8151-300d-73b90cd584d3@bastelstu.be>
Date: Thu, 17 Feb 2022 11:25:05 +0100
MIME-Version: 1.0
Content-Language: en-US
To: Go Kudo <g-kudo@colopl.co.jp>
Cc: internals@lists.php.net
References: <CAMs36HcsGqNanQaro_VTkK_VKHNg-jqtshXpW-aCxdqpxHjMdw@mail.gmail.com>
 <41a1b458-4941-f34e-f1b4-e25b3298b80a@bastelstu.be>
 <CAMs36Hc4GCPUp2m+6N1LTP+hPrUv15iZF2QTCHBuAm+KDyXgfw@mail.gmail.com>
 <553ba7ca-3821-c2d9-f88f-b216013a887b@bastelstu.be>
 <a2683304-542f-e63a-e35f-04e5bb75f5d1@bastelstu.be>
 <CAMs36Hc+W_GkO4W+FB=ySFvc79DYy+M3TaT5qL0Ap9NwmY=6dQ@mail.gmail.com>
 <2c667812-88c8-0b7b-3558-561a1348d0b2@bastelstu.be>
 <CAMs36HctoKykr=aSVCr_DtFEpeF1fZke5LGCOFu_-oQEzL06Ug@mail.gmail.com>
 <a94b1b01-dd9c-bb62-01ab-34c16c3d46cb@bastelstu.be>
 <CAMs36HcVYNVB+mukx27Z90+5DtiHz1JO0i7qF+kP6UaML-y3ZQ@mail.gmail.com>
 <5f496cf9-8754-b009-9cb5-b978222b2249@bastelstu.be>
 <CAMs36HdSf8Mh9Mugwvb643xNfLnpZQ0RAvV_ZuS11vQzQRTmZA@mail.gmail.com>
 <cc43d1ce-8807-c0fd-94a4-514ebedea661@bastelstu.be>
 <CAMs36HcC7O7YR6VVybw=ChPrWWcWjnCTxR6_=5scFy0+ksZEaA@mail.gmail.com>
In-Reply-To: <CAMs36HcC7O7YR6VVybw=ChPrWWcWjnCTxR6_=5scFy0+ksZEaA@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

--------------FglZYnek08KnxIlsazWA72Pu
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi

On 2/17/22 08:37, Go Kudo wrote:
> The following points have been fixed:
> 
> - `nextByteSize(): int` has been removed from Random\Engine
> - If the width of the RNG is statically defined, it will now be used
> preferentially
> - Added Xoshiro256StarStar
> - Fixed an endianness issue
> 
> And updated RFC
> 
> https://wiki.php.net/rfc/rng_extension
> 
> [...]
> This seems to have solved the whole problem. How about it?

Awesome, this is feeling much much better now. As you might've seen I've 
made some comments on GitHub regarding implementation bugs.

I have two more conceptional questions:

-----------

1)

However I believe you did not answer my question regarding the following 
and that's something that should be clear in the RFC and documentation:

     <?php

     use Random\Engine\Xoshiro256StarStar;
     use Random\Randomizer;

     $g1 = new Xoshiro256StarStar(1);
     $g2 = clone $g1;
     $r1 = new Randomizer($g1);
     $r2 = new Randomizer($g2);

     var_dump(\bin2hex($r1->getBytes(8))); // string(16) "c510c70f6daff2b3"
     var_dump(\bin2hex($r2->getBytes(4) . $r2->getBytes(4))); // 
string(16) "c510c70fea4c3647"


In this example I get 8 bytes from the randomizer. One time by getting 
all 8 bytes at once, the second time by getting 4 bytes and then another 
4 bytes.

I think that both lines should result in the same output, because in 
both cases I am getting 8 bytes, without any other operations that might 
affect the engine state in between. As a user I should not be required 
to know how the Randomizer works internally (by always getting 8 bytes 
from the engine and throwing away unused bytes).

If you disagree and think this is the correct behavior then this should 
be documented accordingly in the RFC. If you agree, then this should be 
fixed and a testcase be added.

-----------

2)

This ties into (1): Currently any additional bytes returned by the 
engine are silently ignored. Either the bytes should be processed in 
full, or an error emitted if the returned bytestring is too long.

Consider the attached test case with a Sha1-based RNG. If I grab 20 
bytes (the length of a SHA-1 hash) from the Randomizer then the 
'generate()' function will be called 3 times, despite it returning 
sufficient bytes on the first attempt. If I want to make sure that no 
bytes are wasted, then I need to implement a pretty complex construction 
(Sha1_2) to always return exactly 8 bytes.

Best regards
Tim Düsterhus
--------------FglZYnek08KnxIlsazWA72Pu
Content-Type: application/x-php; name="test_rng.php"
Content-Disposition: attachment; filename="test_rng.php"
Content-Transfer-Encoding: base64

PD9waHAKCnVzZSBSYW5kb21cRW5naW5lOwp1c2UgUmFuZG9tXFJhbmRvbWl6ZXI7CgpmaW5h
bCBjbGFzcyBTaGExIGltcGxlbWVudHMgRW5naW5lIHsKICAgIHByaXZhdGUgc3RyaW5nICRz
dGF0ZTsKCiAgICBwdWJsaWMgZnVuY3Rpb24gX19jb25zdHJ1Y3Qoc3RyaW5nICRzZWVkKQog
ICAgewogICAgICAgICR0aGlzLT5zdGF0ZSA9IFxzaGExKCRzZWVkLCB0cnVlKTsKICAgIH0K
CiAgICBwdWJsaWMgZnVuY3Rpb24gZ2VuZXJhdGUoKTogc3RyaW5nCiAgICB7CiAgICAgICAg
ZWNobyBfX01FVEhPRF9fLCBQSFBfRU9MOwoKICAgICAgICAkcmVzdWx0ID0gJHRoaXMtPnN0
YXRlOwogICAgICAgICR0aGlzLT5zdGF0ZSA9IFxzaGExKCRyZXN1bHQsIHRydWUpOwoKICAg
ICAgICByZXR1cm4gJHJlc3VsdDsKICAgIH0KfQoKZmluYWwgY2xhc3MgU2hhMV8yIGltcGxl
bWVudHMgRW5naW5lIHsKICAgIHByaXZhdGUgSXRlcmF0b3IgJGdlbjsKCiAgICBwdWJsaWMg
ZnVuY3Rpb24gX19jb25zdHJ1Y3Qoc3RyaW5nICRzZWVkKQogICAgewogICAgICAgICR0aGlz
LT5nZW4gPSAkdGhpcy0+c3RyZWFtKCRzZWVkKTsKICAgIH0KCiAgICBwcml2YXRlIGZ1bmN0
aW9uIHN0cmVhbSgkc3RhdGUpCiAgICB7CiAgICAgICAgd2hpbGUgKHRydWUpIHsKICAgICAg
ICAgICAgJHN0YXRlID0gXHNoYTEoJHN0YXRlLCB0cnVlKTsKICAgICAgICAgICAgZm9yICgk
aSA9IDA7ICRpIDwgXHN0cmxlbigkc3RhdGUpOyAkaSsrKSB7CiAgICAgICAgICAgICAgICB5
aWVsZCAkc3RhdGVbJGldOwogICAgICAgICAgICB9CiAgICAgICAgfQogICAgfQoKICAgIHB1
YmxpYyBmdW5jdGlvbiBnZW5lcmF0ZSgpOiBzdHJpbmcKICAgIHsKICAgICAgICBlY2hvIF9f
TUVUSE9EX18sIFBIUF9FT0w7CgogICAgICAgICRyZXN1bHQgPSAiIjsKICAgICAgICBmb3Ig
KCRpID0gMDsgJGkgPCA4OyAkaSsrKSB7CiAgICAgICAgICAgICRyZXN1bHQgLj0gJHRoaXMt
Pmdlbi0+Y3VycmVudCgpOwogICAgICAgICAgICAkdGhpcy0+Z2VuLT5uZXh0KCk7CiAgICAg
ICAgfQoKICAgICAgICByZXR1cm4gJHJlc3VsdDsKICAgIH0KfQoKCiRzZWVkID0gImZvbyI7
Cgp2YXJfZHVtcChcYmluMmhleChcc2hhMSgkc2VlZCwgdHJ1ZSkpKTsgLy8gc3RyaW5nKDQw
KSAiMGJlZWM3YjVlYTNmMGZkYmM5NWQwZGQ0N2YzYzViYzI3NWRhOGEzMyIKCiRnID0gbmV3
IFNoYTEoJHNlZWQpOwokciA9IG5ldyBSYW5kb21pemVyKCRnKTsKCnZhcl9kdW1wKFxiaW4y
aGV4KCRyLT5nZXRCeXRlcygyMCkpKTsgLy8gc3RyaW5nKDQwKSAiMGJlZWM3YjVlYTNmMGZk
YmYzYTJhNTFhOWIwZjJiZTI3NzhjZGRlNyIKCiRnID0gbmV3IFNoYTFfMigkc2VlZCk7CiRy
ID0gbmV3IFJhbmRvbWl6ZXIoJGcpOwoKdmFyX2R1bXAoXGJpbjJoZXgoJHItPmdldEJ5dGVz
KDIwKSkpOyAvLyBzdHJpbmcoNDApICIwYmVlYzdiNWVhM2YwZmRiYzk1ZDBkZDQ3ZjNjNWJj
Mjc1ZGE4YTMzIgo=

--------------FglZYnek08KnxIlsazWA72Pu--