Skip to content
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

Use the system time to seed the default RNG #92

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mgeisler
Copy link
Owner

This gives us a simple way to have lipsum return random output without depending on thread_rng.

This is a followup to #91.

This gives us a simple way to have `lipsum` return random output
without depending on `thread_rng`.

This is a followup to #91.
@mgeisler
Copy link
Owner Author

Hi @reknih and @laurmaedje, can you tell me if this is a terribly idea from the point of view of dependencies? We discussed in #74 that a dependency on rand::thread_rng causes problems for Wasm — are there similar problems with using std::time::SystemTime? I did a small test in a project which uses Wasm and it seems to compile fine.

@laurmaedje
Copy link

It just tried it and while it compiles fine on WASM, it will panic with time not implemented on this platform(for the target wasm32-unknown-unknown). It might work on the wasm32-wasi target (not sure), but that's not what is commonly used for Rust + JavaScript projects.

@mgeisler
Copy link
Owner Author

mgeisler commented Jul 3, 2022

It just tried it and while it compiles fine on WASM, it will panic with time not implemented on this platform(for the target wasm32-unknown-unknown). It might work on the wasm32-wasi target (not sure), but that's not what is commonly used for Rust + JavaScript projects.

Thanks a lot for the info!

I just tried it myself with a local build of https://mgeisler.github.io/textwrap/ and it crashes when I call SystemTime::now(). So I'll see if I can work around this by detecting the target and then use a fixed seed.

@mgeisler
Copy link
Owner Author

mgeisler commented Jul 3, 2022

Alternatively, there seems to be some code here which we could use rust-lang/rust#48564. I'll have to test this in the next few days.

@reknih
Copy link
Contributor

reknih commented Jul 4, 2022

The two snippets in that thread use Date.now() and performance.now(), respectively. This comment mentions that using Date.now() breaks the monotony invariant of std::time::Instant. Personally, I do not know what consequences this has, it might still be enough to seed lipsum.

performance.now() maintains Instant's invariant but is problematic for another reason: It requires Cross Origin Isolation for any embedding webpage to prevent abuse for side channel attacks. Cross Origin Isolation imposes strict CORS policies on a web page to ensure it only runs trusted scripts within its JS context. I believe this sort of requirement would be overkill for a blind text library.

I'm not sure either about the way forward here, the Date.now()-driven implementation might be perfectly fine. It certainly warrants further experimentation.

@mgeisler
Copy link
Owner Author

The two snippets in that thread use Date.now() and performance.now(), respectively. This comment mentions that using Date.now() breaks the monotony invariant of std::time::Instant. Personally, I do not know what consequences this has, it might still be enough to seed lipsum.

Yeah, it should be more than enough :-) We're not after cryptographic randomness here, we just need a value which changes over time, so that calling lipsum twice gives different output.

It just occurred to me that I can add a conditional dependency on wasm-bindgen for the wasm target. That should make it possible to provide a nice randomization on all platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants