Skip to content

Commit

Permalink
Make new and default implementation not depend on random state
Browse files Browse the repository at this point in the history
This oddly fixes issue #148.

I tried to compose a set in a set. Which failed miserably, because the
HashSet would  not have  the same  hash, because  of the  random state
hasher implementation. This  was done to prevent hand  crafted key DOS
attacks.

To get things  working, you can build hash maps  with exactly the same
deterministic hasher. This becomes  error prone and inconvenient quite
quickly. Sprinkeling ::with_hasher() everywhere.

So this commit generalizes the ::new() and Default implementation over
the hasher (S) implementing Default and BuildHasher traits.

This  makes  it  possible  to  instantiate  the  RandomState  specific
implementations in the  root lib module, while  keeping the default(),
new() and unit() implementations general for each hasher.

So if I want a MyHashmap<K,V> with a SpecificHasher implementing
Default:

type MyHashMap<K,V> = im::hashmap::HashMap<K,V,SpecificHasher>;

Then all functions  still work for the different hasher.  I would then
also have to reimplement the macros. But that is fine.
  • Loading branch information
Daan Oosterveld committed Nov 19, 2020
1 parent 3f4e01a commit 645a1ae
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::iter;

use ::arbitrary::{size_hint, Arbitrary, Result, Unstructured};

use crate::{HashMap, HashSet, OrdMap, OrdSet, Vector};
use crate::{hashmap::HashMap, hashset::HashSet, OrdMap, OrdSet, Vector};

fn empty<T: 'static>() -> Box<dyn Iterator<Item = T>> {
Box::new(iter::empty())
Expand Down
24 changes: 14 additions & 10 deletions src/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ use crate::util::{Pool, PoolRef, Ref};
/// ```
#[macro_export]
macro_rules! hashmap {
() => { $crate::hashmap::HashMap::new() };
() => { $crate::HashMap::new() };

( $( $key:expr => $value:expr ),* ) => {{
let mut map = $crate::hashmap::HashMap::new();
let mut map = $crate::HashMap::new();
$({
map.insert($key, $value);
})*;
map
}};

( $( $key:expr => $value:expr ,)* ) => {{
let mut map = $crate::hashmap::HashMap::new();
let mut map = $crate::HashMap::new();
$({
map.insert($key, $value);
})*;
Expand Down Expand Up @@ -119,7 +119,10 @@ where
}
}

impl<K, V> HashMap<K, V, RandomState> {
impl<K, V, S> HashMap<K, V, S>
where
S: BuildHasher + Default,
{
/// Construct an empty hash map.
#[inline]
#[must_use]
Expand All @@ -141,18 +144,19 @@ impl<K, V> HashMap<K, V, RandomState> {
}
}

impl<K, V> HashMap<K, V, RandomState>
impl<K, V, S> HashMap<K, V, S>
where
K: Hash + Eq + Clone,
V: Clone,
S: BuildHasher + Default,
{
/// Construct a hash map with a single mapping.
///
/// # Examples
///
/// ```
/// # #[macro_use] extern crate im;
/// # use im::hashmap::HashMap;
/// # use im::HashMap;
/// let map = HashMap::unit(123, "onetwothree");
/// assert_eq!(
/// map.get(&123),
Expand All @@ -161,8 +165,8 @@ where
/// ```
#[inline]
#[must_use]
pub fn unit(k: K, v: V) -> HashMap<K, V> {
HashMap::new().update(k, v)
pub fn unit(k: K, v: V) -> HashMap<K, V, S> {
HashMap::default().update(k, v)
}
}

Expand All @@ -175,7 +179,7 @@ impl<K, V, S> HashMap<K, V, S> {
///
/// ```
/// # #[macro_use] extern crate im;
/// # use im::hashmap::HashMap;
/// # use im::HashMap;
/// assert!(
/// !hashmap!{1 => 2}.is_empty()
/// );
Expand Down Expand Up @@ -2219,7 +2223,7 @@ mod test {

#[test]
fn large_map() {
let mut map = HashMap::new();
let mut map = crate::HashMap::new();
let size = 32769;
for i in 0..size {
map.insert(i, i);
Expand Down
18 changes: 11 additions & 7 deletions src/hash/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use std::ops::{Add, Deref, Mul};

use crate::nodes::hamt::{hash_key, Drain as NodeDrain, HashValue, Iter as NodeIter, Node};
use crate::ordset::OrdSet;
use crate::Vector;
use crate::util::{Pool, PoolRef, Ref};
use crate::Vector;

/// Construct a set from a sequence of values.
///
Expand All @@ -52,18 +52,18 @@ use crate::util::{Pool, PoolRef, Ref};
/// ```
#[macro_export]
macro_rules! hashset {
() => { $crate::hashset::HashSet::new() };
() => { $crate::HashSet::new() };

( $($x:expr),* ) => {{
let mut l = $crate::hashset::HashSet::new();
let mut l = $crate::HashSet::new();
$(
l.insert($x);
)*
l
}};

( $($x:expr ,)* ) => {{
let mut l = $crate::hashset::HashSet::new();
let mut l = $crate::HashSet::new();
$(
l.insert($x);
)*
Expand Down Expand Up @@ -125,7 +125,10 @@ where
}
}

impl<A> HashSet<A, RandomState> {
impl<A, S> HashSet<A, S>
where
S: BuildHasher + Default,
{
/// Construct an empty set.
#[must_use]
pub fn new() -> Self {
Expand All @@ -145,17 +148,18 @@ impl<A> HashSet<A, RandomState> {
}
}

impl<A> HashSet<A, RandomState>
impl<A, S> HashSet<A, S>
where
A: Hash + Eq + Clone,
S: BuildHasher + Default,
{
/// Construct a set with a single value.
///
/// # Examples
///
/// ```
/// # #[macro_use] extern crate im;
/// # use im::hashset::HashSet;
/// # use im::HashSet;
/// # use std::sync::Arc;
/// let set = HashSet::unit(123);
/// assert!(set.contains(&123));
Expand Down
6 changes: 4 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,10 @@ compile_error!(
"The `pool` feature is not threadsafe but you've enabled it on a threadsafe version of `im`."
);

pub use crate::hashmap::HashMap;
pub use crate::hashset::HashSet;
/// HashMap with random state hasher
pub type HashMap<K, V> = crate::hashmap::HashMap<K, V, std::collections::hash_map::RandomState>;
/// HashSet with random state hasher
pub type HashSet<A> = crate::hashset::HashSet<A, std::collections::hash_map::RandomState>;
pub use crate::ordmap::OrdMap;
pub use crate::ordset::OrdSet;
#[doc(inline)]
Expand Down
2 changes: 1 addition & 1 deletion src/quickcheck.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{HashMap, HashSet, OrdMap, OrdSet, Vector};
use crate::{hashmap::HashMap, hashset::HashSet, OrdMap, OrdSet, Vector};
use ::quickcheck::{Arbitrary, Gen};
use std::hash::{BuildHasher, Hash};
use std::iter::FromIterator;
Expand Down

0 comments on commit 645a1ae

Please sign in to comment.