Re: [RFC] [Under Discussion] Random Extension 4.0

2022年2月16日(水) 21:25 Tim Düsterhus <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 = new Bug(); > $g = new Randomizer($e); > > var_dump(\bin2hex($g->getBytes(8))); > > Results in: > > > ==116755== Use of uninitialised value of size 8 > > ==116755== at 0x6180C8: php_bin2hex (string.c:111) > > ==116755== by 0x6185D2: zif_bin2hex (string.c:220) > > ==116755== by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER > (zend_vm_execute.h:1312) > > ==116755== by 0x8194F0: execute_ex (zend_vm_execute.h:55503) > > ==116755== by 0x81ED86: zend_execute (zend_vm_execute.h:59858) > > ==116755== by 0x75A923: zend_execute_scripts (zend.c:1744) > > ==116755== by 0x69C8C4: php_execute_script (main.c:2535) > > ==116755== by 0x8E0B19: do_cli (php_cli.c:965) > > ==116755== by 0x8E1DF9: main (php_cli.c:1367) > > ==116755== > > ==116755== Use of uninitialised value of size 8 > > ==116755== at 0x618100: php_bin2hex (string.c:112) > > ==116755== by 0x6185D2: zif_bin2hex (string.c:220) > > ==116755== by 0x79BDB4: ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER > (zend_vm_execute.h:1312) > > ==116755== by 0x8194F0: execute_ex (zend_vm_execute.h:55503) > > ==116755== by 0x81ED86: zend_execute (zend_vm_execute.h:59858) > > ==116755== by 0x75A923: zend_execute_scripts (zend.c:1744) > > ==116755== by 0x69C8C4: php_execute_script (main.c:2535) > > ==116755== by 0x8E0B19: do_cli (php_cli.c:965) > > ==116755== by 0x8E1DF9: main (php_cli.c:1367) > > ==116755== > > 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üsterhus > 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/random/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/random/tests/engine/user_xoshiro128plusplus.phpt This seems to have solved the whole problem. How about it? Regards Go Kudo