Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117049
MIME-Version: 1.0
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>
In-Reply-To: <cc43d1ce-8807-c0fd-94a4-514ebedea661@bastelstu.be>
Date: Thu, 17 Feb 2022 16:37:50 +0900
Message-ID: <CAMs36HcC7O7YR6VVybw=ChPrWWcWjnCTxR6_=5scFy0+ksZEaA@mail.gmail.com>
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="0000000000008600e405d831d807"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension 4.0
From: g-kudo@colopl.co.jp (Go Kudo)

--0000000000008600e405d831d807
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

2022=E5=B9=B42=E6=9C=8816=E6=97=A5(=E6=B0=B4) 21:25 Tim D=C3=BCsterhus <tim=
@bastelstu.be>:

> Hi
>
> On 2/16/22 12:39, Go Kudo wrote:
> >>   Is the nextByteSize() method actually required? PHP strings already
> know
> > their own length.
> >
> > This is a convenience of the current implementation.
>
> You already said that you will think of some good ideas, but I'd like to
> be clear that the convenience of the internal implementation should not
> be something that affects the user-facing implementation.
>
> In fact with the current implementation I can trivially create a
> memory-unsafety bug:
>
>      <?php
>
>      use Random\Engine;
>      use Random\Randomizer;
>
>      final class Bug implements Engine {
>          public function generate(): string
>          {
>              return '';
>          }
>
>          public function nextByteSize(): int {
>              return 7;
>          }
>      }
>
>      $e =3D new Bug();
>      $g =3D new Randomizer($e);
>
>      var_dump(\bin2hex($g->getBytes(8)));
>
> Results in:
>
> >     =3D=3D116755=3D=3D Use of uninitialised value of size 8
> >     =3D=3D116755=3D=3D    at 0x6180C8: php_bin2hex (string.c:111)
> >     =3D=3D116755=3D=3D    by 0x6185D2: zif_bin2hex (string.c:220)
> >     =3D=3D116755=3D=3D    by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_H=
ANDLER
> (zend_vm_execute.h:1312)
> >     =3D=3D116755=3D=3D    by 0x8194F0: execute_ex (zend_vm_execute.h:55=
503)
> >     =3D=3D116755=3D=3D    by 0x81ED86: zend_execute (zend_vm_execute.h:=
59858)
> >     =3D=3D116755=3D=3D    by 0x75A923: zend_execute_scripts (zend.c:174=
4)
> >     =3D=3D116755=3D=3D    by 0x69C8C4: php_execute_script (main.c:2535)
> >     =3D=3D116755=3D=3D    by 0x8E0B19: do_cli (php_cli.c:965)
> >     =3D=3D116755=3D=3D    by 0x8E1DF9: main (php_cli.c:1367)
> >     =3D=3D116755=3D=3D
> >     =3D=3D116755=3D=3D Use of uninitialised value of size 8
> >     =3D=3D116755=3D=3D    at 0x618100: php_bin2hex (string.c:112)
> >     =3D=3D116755=3D=3D    by 0x6185D2: zif_bin2hex (string.c:220)
> >     =3D=3D116755=3D=3D    by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_H=
ANDLER
> (zend_vm_execute.h:1312)
> >     =3D=3D116755=3D=3D    by 0x8194F0: execute_ex (zend_vm_execute.h:55=
503)
> >     =3D=3D116755=3D=3D    by 0x81ED86: zend_execute (zend_vm_execute.h:=
59858)
> >     =3D=3D116755=3D=3D    by 0x75A923: zend_execute_scripts (zend.c:174=
4)
> >     =3D=3D116755=3D=3D    by 0x69C8C4: php_execute_script (main.c:2535)
> >     =3D=3D116755=3D=3D    by 0x8E0B19: do_cli (php_cli.c:965)
> >     =3D=3D116755=3D=3D    by 0x8E1DF9: main (php_cli.c:1367)
> >     =3D=3D116755=3D=3D
> >     string(16) "0000000000000000"
>
> For userland implementations you really must derive the size from the
> returned bytestring. Otherwise it's easy for a developer to create an
> implementation where nextByteSize() and generate() disagree. Even if the
> memory-unsafety is fixed, this will result in frustration for the user.
>
> For native implementations you can keep some explicit width in the code,
> if that helps with performance.
>
> Best regards
> Tim D=C3=BCsterhus
>

Hi Tim

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

I also had a PHP implementation of Xorshift128Plus on hand, so I included
it in the tests.

https://github.com/colopl/php-src/blob/upstream_rfc/scoped_rng_for_pr/ext/r=
andom/tests/engine/user_xorshift128plus.phpt

Xoshiro128PlusPlus has been dropped from the bundle due to width issues. If
necessary, it will be implemented as an extension to PECL.
However, it is built in as a test of the userland implementation

https://github.com/colopl/php-src/blob/upstream_rfc/scoped_rng_for_pr/ext/r=
andom/tests/engine/user_xoshiro128plusplus.phpt

This seems to have solved the whole problem. How about it?

Regards
Go Kudo

--0000000000008600e405d831d807--