Page MenuHomePhabricator

Harden "Filesystem::readRandomInteger()" against misuse and builtin failure
ClosedPublic

Authored by epriestley on Jan 16 2019, 8:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 28, 6:45 AM
Unknown Object (File)
Thu, Apr 25, 12:41 AM
Unknown Object (File)
Thu, Apr 11, 5:16 AM
Unknown Object (File)
Mar 11 2024, 2:24 AM
Unknown Object (File)
Jan 3 2024, 9:48 PM
Unknown Object (File)
Jan 3 2024, 6:07 AM
Unknown Object (File)
Dec 31 2023, 6:47 PM
Unknown Object (File)
Dec 30 2023, 6:19 PM
Subscribers
None

Details

Summary

See D19981. Make more of an effort to make sure that nothing unexpected-but-in-our-control can go awry here.

Test Plan

See unit tests.

Diff Detail

Repository
rPHU libphutil
Branch
mfa25
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21548
Build 29369: Run Core Tests
Build 29368: arc lint + arc unit

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 16 2019, 8:41 PM
Harbormaster failed remote builds in B21547: Diff 47695!

Looks like PHP_INT_MIN wasn't introduced until PHP 7.

  • Remove PHP_INT_MIN test that only works on PHP 7.0.0 or newer.
  • Add a check for mt_getrandmax() before hitting the MT code.
src/filesystem/Filesystem.php
576–579

This specific test came out of the "Notes" section in the mt_rand() documentation.

I think the implementation is: generate a value, normalize it to [0, max - min], then add min -- so it's only the difference between min and max that matters, not the actual value of min or max.

This revision is now accepted and ready to land.Jan 16 2019, 8:53 PM
This revision was automatically updated to reflect the committed changes.