Skip to content

Add ext/random Exception hierarchy - #9220

Merged
TimWolla merged 9 commits into
php:masterfrom
TimWolla:random-exception-cleanup
Aug 2, 2022
Merged

Add ext/random Exception hierarchy#9220
TimWolla merged 9 commits into
php:masterfrom
TimWolla:random-exception-cleanup

Conversation

@TimWolla

@TimWollaTimWolla commented Aug 1, 2022

Copy link
Copy Markdown
Member

This complements #9211.


The hierarchy is roughly based on @Danack's recommendations in this comment: #9071 (comment).

Choice of base Exception

RandomError and RandomException both represent the base Exceptions. I considered using an interface, but thus would make it harder to not catch Error / RandomError:

You can easily do catch (\Random\RandomError|\Random\RandomException) if you want to catch both Errors and Exceptions for whatever reason. But doing catch ((\Random\RandomExceptionInterface & \Random\RandomException)) is not legal if you want to catch just Exceptions, but not Errors.

BrokenRandomEngineError

While you should not catch an Error, I found it useful to have a meaningful class name here. BrokenRandomEngineError is used for:

  • A user engine that returns an empty string (which is equivalent to 0 bits of randomness and thus disallowed).
  • An engine that fails to generate acceptable randomness for use with range() (e.g. getInt()) in 50 attempts. The chance of rejection for a uniform bytestring is less that 50%. Thus the chance of failure for an unbiased engine is less than 0.550 (~ 8.8×10-16). If an engine fails that then it must be heavily biased.

RandomException

This is used directly for:

  • Failure of the CSPRNG in random_bytes() and random_int() (which should not happen on modern OSes).
  • Failure to seed Mt19937, Xoshiro256StarStar, PcgOneseq128XslRr64 from the CSPRNG when a null seed is used to improve the error reporting as per Improve error message of an engine fails to seed from the CSPRNG #9160. Thus the failure mode is identical to the one above (none of modern systems).

@zeriyoshizeriyoshi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so clean and beautiful!

@Danack

Copy link
Copy Markdown
Contributor

Prediction, the name "RandomException" will be used by multiple people as a point of humour in talks about the new and improved random capabilities of PHP.

So, all looks good to me.

@GirgiasGirgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@TimWolla
TimWollaforce-pushed the random-exception-cleanup branch from c63efc7 to 3fb1275CompareAugust 2, 2022 15:53

@cmb69cmb69 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@TimWolla
TimWollaforce-pushed the random-exception-cleanup branch from 2113833 to 75db12aCompareAugust 2, 2022 16:56
@TimWolla
TimWolla merged commit 3331832 into php:masterAug 2, 2022
@TimWolla
TimWolla deleted the random-exception-cleanup branch August 25, 2022 09:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@TimWolla@Danack@cmb69@Girgias@zeriyoshi