[RFC] [Under Discussion] Random Extension 5.x

Hi Larry,

• If no engine is provided to the randomizer, what gets used? If the default is listed somewhere in the RFC, I missed it. I'm assuming there should be one, for usability, but changing it in the future would be an RFC-level change (much like changing the defaults for password_hash()).

I couldn't find it in the text either, but it defaults to
Random\Engine\Secure as per my suggestion in the previous discussion
thread for version 4 of the RFC:

https://externals.io/message/117026#117027

Best regards
Tim Düsterhus

2022年3月10日(木) 3:07 Larry Garfield larry@garfieldtech.com:

Hello internals.

Proposed RFCs and implementations have been reorganized. The main changes
are as follows

https://wiki.php.net/rfc/rng_extension
https://github.com/php/php-src/pull/8094

• Remove XorShift128Plus, Xoshiro256StarStar
  • To avoid confusion, Can be added if needed
• All classes can now omit the seed argument
  • Seeding with CSPRNG
• Throws RuntimeException if number generation fails

The merge window to PHP 8.2 will remain open for some time to come. Due
to
the global turmoil, I will try to delay the start of the RFC voting phase
as long as possible.

However, I believe that PHP RFCs tend to become more controversial once
the
voting phase begins.

Therefore, I would like to solicit feedback on this RFC, whatever it may
be. (Just only positive or negative feedback would be okay)

Regards,
Go Kudo

This looks good to me overall, although I have a few comments.

• There's several places where a sentence or portion of a sentence repeats
  itself. I assume this is just an editing error.

• The list headers "implemented of" doesn't really make grammatical
  sense. I think you mean "implemented by"?

• The last section "PRNG Shootout" seems to imply that you're only
  implementing one algorithm, but it doesn't say which one. The earlier
  sections, though, show several algorithms that you are implementing.
  Again, I assume this is an editing error from many revisions but it's
  confusing as is.

• If no engine is provided to the randomizer, what gets used? If the
  default is listed somewhere in the RFC, I missed it. I'm assuming there
  should be one, for usability, but changing it in the future would be an
  RFC-level change (much like changing the defaults for password_hash()).

• The example of using different algorithms in different environments
  needs much higher billing. You're burying the lead there. The ability to
  have a bad but predictable random seed for tests but then a CSPRNG for
  production is huge. The approach is similar to the nearly-done PSR-20
  spec in FIG, which does essentially the same thing for the "current time"
  routines, albeit in userspace. I would highlight this benefit way more, as
  it's probably the biggest benefit from these changes that most developers
  will see and has me more excited about this RFC than anything else in the
  entire document.

• How do the existing random functions interact with the new API? Do they
  shift to be shells over the new classes with an internal shared global, or
  something else?

• A side effect of the current design is that SeedableEngine is just a
  marker interface; there's no actual standardized way to set the seed. The
  examples imply that passing it as a constructor is the recommended way (and
  I agree it probably should be), but that's not explicit. I'm also unclear
  then what value the marker interface is, then. Does the randomizer do
  something different with a seedable engine? I'm not against the current
  design, but its explanation/implications could be improved.

Looking forward to this passing in a few weeks!

--Larry Garfield

--

To unsubscribe, visit: https://www.php.net/unsub.php

Hi Larry,

I have attempted to correct what you pointed out. However, I don't think
the RFC is complete at this point. We will work on correcting this over the
next few days.

I'll answer a few questions. (These have been corrected in the RFC)

If no engine is provided to the randomizer, what gets used? If the
default is listed somewhere in the RFC, I missed it. I'm assuming there
should be one, for usability, but changing it in the future would be an
RFC-level change (much like changing the defaults for password_hash()).

As Tim has answered, Random\Engine\Secure is automatically generated and
used.

The example of using different algorithms in different environments needs
much higher billing

This is a very important aspect, but also a side effect of the current
problem solving. I am very concerned about how to appeal to this.
For the time being, I have added a sentence to the Proposal.

How do the existing random functions interact with the new API

As BC break is none, it does not affect existing APIs. The internal
implementation will be changed.
This is to avoid redundancy in implementation.

https://github.com/php/php-src/pull/8094/files#diff-8ba10cd3f979cdab4491a2d02149ff10638b88854e79664748b3ab620fd3f1bfR255-R259

I am very concerned about how to make the RFC easier to read.
I Will continue to work on improving it.

Regards
Go Kudo

2022年3月10日(木) 3:57 Tim Düsterhus tim@bastelstu.be:

Hi

Proposed RFCs and implementations have been reorganized. The main changes
are as follows

https://wiki.php.net/rfc/rng_extension
https://github.com/php/php-src/pull/8094

I've compared the RFC against the implementation once more:

• The RFC should clarify that returning an empty string in
  Engine::generate() is not allowed.

• The corresponding test
  'ext/random/tests/03_randomizer/user_unsafe.phpt' should verify not just
  an exception is thrown, but also the exception message (I'll add a
  review comment).

• PCG64 effectively has 256 Bits of internal state (128 Bits for 's' and
  128 Bits for 'inc'), but it only accepts 128 Bits for 's'. 'inc' cannot
  be provided by the user. The purpose of 'inc' is not entirely clear to
  me, but the user likely should be able to specify it for full
  reproducibility.

• The default initialization of PCG64 with a null seed will fill 'inc',
  not 's'. This likely is a bug? At least it's inconsistent with the
  previous point.

• The RFC is missing an explanation of the guarantees the implementation
  will (or will not) make. This is important for the user if they are
  relying on reproducibility of the sequences and outputs. The more
  guarantees we give, the less can be changed in the future. I've
  previously mentioned that in the thread for '4.x':
  https://externals.io/message/117026#117061

Best regards
Tim Düsterhus

Hi Tim

Sorry, I must have missed some of your emails.

We will make corrections to the areas you pointed out. I will reply to your
email first as I am not familiar with English and it may take a little time.

Thank you!