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.

Sign up for GitHub

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

Jump to bottom

Tracking issue for SipHash13/SipHash24 #34767

Closed
alexcrichton opened this issue Jul 11, 2016 · 6 comments
Closed

Tracking issue for SipHash13/SipHash24 #34767

alexcrichton opened this issue Jul 11, 2016 · 6 comments
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

Added in #33940, these two structs which implement Hasher live in the std::hash module. This issue will track their stabilization, and some open questions when we added them were:

  • Do we want to export any more hasher implementations? Right now SipHasher is stable, but perhaps we don't want to expand?
  • If we do stabilize, what to do about the SipHasher name?
@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Feature: Implemented in the nightly compiler and unstable. labels Jul 11, 2016
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 11, 2016
@alexcrichton
std: Correct tracking issue for SipHash{13,24}
767e149 
The referenced tracking issue was closed and was actually about changing the
algorithm.

cc rust-lang#34767
@alexcrichton alexcrichton mentioned this issue Jul 11, 2016
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 16, 2016
@GuillaumeGomez
Rollup merge of rust-lang#34768 - alexcrichton:issue-audit, r=aturon
ad7a697 
std: Correct tracking issue for SipHash{13,24}

The referenced tracking issue was closed and was actually about changing the
algorithm.

cc rust-lang#34767
@sfackler sfackler added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed I-nominated labels Sep 7, 2016
@sfackler
Copy link
Member

sfackler commented Sep 7, 2016

The libs team discussed these types, and are placing them into final comment period to deprecate SipHasher, SipHash13, and SipHash24, and then remove SipHash13 and SipHash24 in a future release.

While RandomState is currently implemented with SipHash-1-3, this will not necessarily continue to be the case in the future if, for example, a faster cryptographic hash is developed or security flaws are found in it. We don't want to be forced to keep these implementations around forever if that happens.

That being said, we are interested in moving forward on standard library support for faster hash maps and sets for small keys like integers where HashDoS is not a concern. This would most likely involve the addition of a FastHasher backed by something like FNV, and possibly FashHashMap and FastHashSet type definitions.

@alexcrichton
Copy link
Member Author

The libs team discussed this during triage yesterday and the conclusion was to deprecate

@alexcrichton alexcrichton mentioned this issue Sep 29, 2016
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this issue Oct 3, 2016
@alexcrichton
std: Stabilize and deprecate APIs for 1.13
10c3134 
This commit is intended to be backported to the 1.13 branch, and works with the
following APIs:

Stabilized

* `i32::checked_abs`
* `i32::wrapping_abs`
* `i32::overflowing_abs`
* `RefCell::try_borrow`
* `RefCell::try_borrow_mut`
* `DefaultHasher`
* `DefaultHasher::new`
* `DefaultHasher::default`

Deprecated

* `BinaryHeap::push_pop`
* `BinaryHeap::replace`
* `SipHash13`
* `SipHash24`
* `SipHasher` - use `DefaultHasher` instead in the `std::collections::hash_map`
  module

Closes rust-lang#28147
Closes rust-lang#34767
Closes rust-lang#35057
Closes rust-lang#35070
bors added a commit that referenced this issue Oct 3, 2016
@bors
Auto merge of #36815 - alexcrichton:stabilize-1.13, r=aturon
7a26aec 
std: Stabilize and deprecate APIs for 1.13

This commit is intended to be backported to the 1.13 branch, and works with the
following APIs:

Stabilized

* `i32::checked_abs`
* `i32::wrapping_abs`
* `i32::overflowing_abs`
* `RefCell::try_borrow`
* `RefCell::try_borrow_mut`

Deprecated

* `BinaryHeap::push_pop`
* `BinaryHeap::replace`
* `SipHash13`
* `SipHash24`
* `SipHasher` - use `DefaultHasher` instead in the `std::collections::hash_map`
  module

Closes #28147
Closes #34767
Closes #35057
Closes #35070
@SimonSapin
Copy link
Contributor

The SipHasher::new_with_keys functionality got deprecated without replacement. This wasn’t discussed specifically, so it looks like to may have been unintentional.

I’d like libcore to provide non-deprecated API for this functionality: #37071

alexcrichton added a commit to alexcrichton/rust that referenced this issue Oct 10, 2016
@alexcrichton
std: Stabilize and deprecate APIs for 1.13
2d1feef 
This commit is intended to be backported to the 1.13 branch, and works with the
following APIs:

Stabilized

* `i32::checked_abs`
* `i32::wrapping_abs`
* `i32::overflowing_abs`
* `RefCell::try_borrow`
* `RefCell::try_borrow_mut`
* `DefaultHasher`
* `DefaultHasher::new`
* `DefaultHasher::default`

Deprecated

* `BinaryHeap::push_pop`
* `BinaryHeap::replace`
* `SipHash13`
* `SipHash24`
* `SipHasher` - use `DefaultHasher` instead in the `std::collections::hash_map`
  module

Closes rust-lang#28147
Closes rust-lang#34767
Closes rust-lang#35057
Closes rust-lang#35070
@jimblandy
Copy link
Contributor

While RandomState is currently implemented with SipHash-1-3, this will not necessarily continue to be the case in the future if, for example, a faster cryptographic hash is developed or security flaws are found in it. We don't want to be forced to keep these implementations around forever if that happens.

@sfackler, is the concern here that the library team doesn't want to keep around hashers that aren't currently used in std::collections or the like, just because they have been used in the past?

@chritchens chritchens mentioned this issue Apr 27, 2017
Merged
@eisterman
Copy link

When the SipHasher13 will be removed and his method moved in DefaultHasher?

@SimonSapin
Copy link
Contributor

SimonSapin commented Mar 31, 2018 •
edited

#49108 "removed" SipHasher13 from the documented public API of the standard library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-unstable Feature: Implemented in the nightly compiler and unstable. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@alexcrichton @SimonSapin @jimblandy @sfackler @eisterman