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

We should rename std::hash::RandomSipHasher to something more generic #20050

Closed
erickt opened this issue Dec 19, 2014 · 6 comments
Closed

We should rename std::hash::RandomSipHasher to something more generic #20050

erickt opened this issue Dec 19, 2014 · 6 comments
Labels
A-security Area: Security (example: address space layout randomization).

Comments

@erickt
Copy link
Contributor

erickt commented Dec 19, 2014

Right now HashMaps are tied to a specific algorithm implementation, std::hash::RandomSipHasher. While I believe SipHash is considered cryptographically secure, if it were ever to be broken it could be painful transitioning everyone into a new Hasher. Instead, I suggest we change it's name now to save us from headaches later on. Some possible names:

  • DefaultHasher
  • StandardHasher
  • StdHasher
  • RandomHasher
@aturon aturon added A-libs A-security Area: Security (example: address space layout randomization). labels Dec 19, 2014
@sfackler
Copy link
Member

This seems pretty reasonable to me, but we should probably explicitly specify that it's randomly keyed.

@aturon
Copy link
Member

aturon commented Dec 19, 2014

(This has been nominated for the 1.0 milestone due to long-term security concerns.)

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

I think the name should reflect that it is chosen specifically for its security properties, and not, say, performance properties (which seem to be mediocre). Also I do not think we should imply that it is a good secure or default choice for other hashing tasks. It is specifically good for HashMaps.

SecureHashmapHasher?

@pczarn
Copy link
Contributor

pczarn commented Dec 21, 2014

I prefer UnpredictableHasher, RandomHasher. RandomizedHasher.

Also I do not think we should imply that it is a good secure or default choice for other hashing tasks

Won't SipHash be necessary for some other collections, such as HAMT?

@Gankra
Copy link
Contributor

Gankra commented Dec 21, 2014

Oh I assumed this was just going to be some kind of newtype/alias for RandomSipHasher (which is simply a nice, accurate, name). So HAMT would also alias it.

@alexcrichton
Copy link
Member

Fixed in #20654

  • RandomSipHasher was renamed to std::collections::hash_map::RandomState
  • The default hasher for hash maps is now called std::collections::hash_map::Hasher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-security Area: Security (example: address space layout randomization).
Projects
None yet
Development

No branches or pull requests

6 participants