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 Random instance for UUID, or mark it as deprecated #22

Open
bitonic opened this issue Jun 8, 2016 · 8 comments
Open

Remove Random instance for UUID, or mark it as deprecated #22

bitonic opened this issue Jun 8, 2016 · 8 comments

Comments

@bitonic
Copy link
Contributor

bitonic commented Jun 8, 2016

See #15 (comment) for motivations, and entire ticket for context.

@aslatter
Copy link
Collaborator

aslatter commented Jun 9, 2016

Are you worried because someone might try to use the instance with StdGen? Maybe anyone that actually cares about randomness won't even bother with the System.Random class, but I'm not sure what harm comes from letting someone try.

@bitonic
Copy link
Contributor Author

bitonic commented Jun 9, 2016

I'm worried about this kind of usage: silkapp/rest@e4ef6a6 . If you do this, you're going to get 64-bit UUIDs and you don't realize it.

@bitonic
Copy link
Contributor Author

bitonic commented Jun 9, 2016

Note that my comment above is based on #15 (comment) .

I think if the Random instance is properly implemented then it is "dangerous" only in the sense that it's not really suitable to generate UUIDs that are hard to predict. Or as you said,

Maybe anyone that actually cares about randomness won't even bother with the System.Random class, but I'm not sure what harm comes from letting someone try.

I still think this difference in behavior warrants at the very least some documentation/warning, considering that UUIDs are very often used to implement things like user sessions tokens that rely on the UUIDs to be hard to predict.

@prikhi
Copy link
Contributor

prikhi commented Jun 20, 2017

I ran into this, couldn't figure out how to use the Random a class with something other than System.Random and a figured nextRandom was just a wrapper around randomIO because there wasn't anything saying otherwise.

I think it would be helpful to say that one should use nextRandom for secure purposes and randomIO for general purposes.

@hvr
Copy link
Collaborator

hvr commented Jun 20, 2017

@prikhi would you be interested in providing a PR enhancing the Haddock documentation to that effect?

@prikhi
Copy link
Contributor

prikhi commented Jun 20, 2017

@hvr Created in #31

hvr added a commit that referenced this issue Jun 21, 2017
[#22] Document Difference Between randomIO & nextRandom
@axman6
Copy link

axman6 commented May 8, 2020

I think we're running into this in production, we're getting a lot of collisions of UUIDs generated from a bunch of Lambdas, so we're now looking at generating UUID using something like cryptonite instead.

Edit: Looks like we're probably using the wrong way to generate UUIDs, and V4.nextRandom should be what we use.

@phadej
Copy link
Collaborator

phadej commented Feb 16, 2021

Random instance is really little use for AWS Lambda. There the if you generate just a few UUIDs (or even many) they depend heavily on an initial random seed.

https://hackage.haskell.org/package/uuid-1.3.13/docs/Data-UUID-V4.html uses entropy which is a lot better idea in that environment.

I'm tempted to remove Random instance in the next major version of uuid-types and uuid, as I honestly don't see a good reason for it. V4 entropy generation should be good enough for "real" UUID generation, and quickcheck-instances has not-real UUID generation for testing purposes (i.e. essentially generating two Word64s).

... but there aren't any reasons to make major release so for now Random instance will stay.

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

No branches or pull requests

6 participants