-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Skrifa] Avoid initialization of HashSet with system random numbers #751
Conversation
Tested in Chromium, works as a solution for https://bugs.chromium.org/p/chromium/issues/detail?id=1516634. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay this makes sense.
My one concern here is that it would be easy for someone to introduce other uses of hashmap without being aware that it will break the upstream client? But I can't think of any clean way to prevent that, so I guess it will be a matter for code review.
And just as an observation, if this was a pattern we were doing a lot I might create a trait, DeterministicHash
, where we could define a constructor method, so you could just call, HashSet::new_non_random()
or something. Definitely not necessary, just mentioning it because it's a useful pattern to consider sometimes.
Workaround for [1] and [2] - issues in the Chrome sandbox requiring access to random number generators. [1] https://bugs.chromium.org/p/chromium/issues/detail?id=1516634 [2] https://bugs.chromium.org/p/chromium/issues/detail?id=1516641
And add workaroud to use non random state for chrome on windows. Based on googlefonts/fontations#751.
…umbers (googlefonts#751)" This reverts commit eef6a26. See details in [1] and [2]. Rust upstream landed a change switching to a different, sandbox-compatible call for PRNG, except on Windows 7. Windows 7 is no longer supported by Chromium. Chromium is our only known sandboxed environment where this call would fail. Remove the workaround. [1] https://issues.chromium.org/40277768 [2] rust-lang/rust@08caefb
…umbers (#751)" (#825) This reverts commit eef6a26. See details in [1] and [2]. Rust upstream landed a change switching to a different, sandbox-compatible call for PRNG, except on Windows 7. Windows 7 is no longer supported by Chromium. Chromium is our only known sandboxed environment where this call would fail. Remove the workaround. [1] https://issues.chromium.org/40277768 [2] rust-lang/rust@08caefb
Workaround for [1] and [2] - issues in the Chrome sandbox requiring access to random number generators.
[1] https://bugs.chromium.org/p/chromium/issues/detail?id=1516634
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=1516641