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
F15503893: D19985.id47697.diff
Mon, Apr 14, 2:13 PM
F15503583: D19985.id47695.diff
Mon, Apr 14, 11:05 AM
F15502295: D19985.id.diff
Sun, Apr 13, 11:38 PM
F15500827: D19985.id47696.diff
Sun, Apr 13, 7:32 PM
F15492319: D19985.diff
Sat, Apr 12, 1:16 PM
F15483139: D19985.id47696.diff
Wed, Apr 9, 10:14 AM
F15409792: D19985.diff
Wed, Mar 19, 5:18 AM
F15376004: D19985.id47695.diff
Mar 13 2025, 1:13 AM
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.