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

Remove insecure rng #354

Merged
merged 10 commits into from
Jan 27, 2020
Merged

Remove insecure rng #354

merged 10 commits into from
Jan 27, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Jan 20, 2020

Closes #294 and fixes #173

@ctavan ctavan changed the base branch from master to next January 20, 2020 22:09
@ctavan
Copy link
Member Author

ctavan commented Jan 20, 2020

BTW as expected this now fails in IE 10 where no crypto.getRandomValues() is available:

Screenshot 2020-01-20 23 35 52

@ctavan ctavan changed the title WIP Remove insecure rng Remove insecure rng Jan 23, 2020
@ctavan ctavan force-pushed the remove-insecure-rng branch 2 times, most recently from e87e36b to 1ff90ce Compare January 23, 2020 13:41
@ctavan
Copy link
Member Author

ctavan commented Jan 23, 2020

@broofa do you think that, given the now-wide-spread support for CSRNGs in all platforms, we still need the wiki page over at https://github.com/uuidjs/uuid/wiki#no-secure-rng-error ?

@ctavan ctavan requested a review from broofa January 23, 2020 13:43
@ctavan ctavan force-pushed the remove-insecure-rng branch 4 times, most recently from 7fe0e70 to b41721f Compare January 23, 2020 19:39
@broofa
Copy link
Member

broofa commented Jan 24, 2020

Go ahead and kill that wiki page.

@broofa broofa closed this Jan 24, 2020
@ctavan
Copy link
Member Author

ctavan commented Jan 24, 2020

@broofa did you close this PR on purpose?

@broofa
Copy link
Member

broofa commented Jan 27, 2020

I did not. Sorry about that!

README_js.md Outdated
### Version 1 (Timestamp + Node)

⚠️⚠️⚠️ **Please make sure to check whether you really need the timestamp properties of Version 1
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have a link of some sort that provides information to help with readers' v1 vs v4 decision making.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a link to the standard lib proposal. I think that covers the reasoning.

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.

2 participants