close a vulnerability where HashMap::from_iter wasn't being randomized

[erickt]

While HashMap::new and HashMap::with_capacity were being initialized with a random SipHasher, it turns out that HashMap::from_iter was just using the default instance of SipHasher, which wasn't randomized. This closes that bug, and also inlines some important methods.

[alexcrichton]

I've been thinking of doing something like this for the facade to allow insertion of HashMap into libcollections instead of libstd.

The one final piece of the puzzle would be to parameterize HashMap::new()'s with H: Default rather than lock it down to RandomizedSipHasher. Would it be possible to add that as part of this commit as well? (it doesn't have to be done as part of this PR)

[erickt]

I just tried that, and it's going to result is us having to explicitly declare hashmap types. For example this snippet needs an explicit type:

pub fn require_unique_names(diagnostic: &SpanHandler, metas: &[Gc<MetaItem>]) {
    let mut set: HashSet<InternedString> = HashSet::new();
    for meta in metas.iter() {
        let name = meta.name();

        if !set.insert(name.clone()) {
            diagnostic.span_fatal(meta.span,
                                  format!("duplicate meta item `{}`",
                                          name).as_slice());
        }
    }
}

Which is a bit unfortunate. Would it be easier to have an explicit generic HashMap type in libcollections, and a thin wrapper for it in libstd that uses this randomized hasher? Although really the best option would be to allow type HashMap<K, V> = collections::HashMap::<K, V, RandomizedSipHasher> to pass static methods onto the type alias.

[alexcrichton]

Hm, that is indeed unfortunate. I was hoping for something like this:

// src/libstd/collections/hashmap.rs
pub type HashMap<K, V, H = RandomizedSipHasher> = core_collections::HashMap::<K, V, H>;

The key part being that the std::collections::HashMap type still has overridable hashers without having to go through libcollections to get that.

I suppose if you can define methods on type aliases it may work out as you could have impl HashMap in libstd with a fn new, but that's becoming quite a stretch. Sounds like there's definitely some more design work to do here.

[alexcrichton]

This is available at std::hash, so why export it here?

[erickt]

@alexcrichton: I updated the PR to remove libstd::collections::hash.

[alexcrichton]

There's a nice doc-comment in collections::hash which sadly won't get included like this, perhaps it could be moved over to this module instead?

[erickt]

@alexcrichton: updated to include the documentation.