HeaderMap::Drain can cause data race

[Qwaz]

http/src/header/map.rs

Lines 2099 to 2102 in 9c05e39

	impl<'a, T> Iterator for Drain<'a, T> {
	type Item = (HeaderName, ValueDrain<'a, T>);
	fn next(&mut self) -> Option<Self::Item> {

HeaderMap::Drain implements Iterator trait with Item = (HeaderName, ValueDrain<'a, T>). This definition makes it possible to create multiple mutable references to the underlying HeaderMap by creating multiple ValueDrain, because there is no relation between the lifetime of &mut self and the returned ValueDrain.

This bug may invalidate ValueDrain iterators or make them return incorrect results. In multi-threaded scenario, the bug allows multiple threads to mutate underlying Vec at the same time, which I believe exploitable. However, I expect no code is sending ValueDrain to the other thread in practice.

Demonstration

[seanmonstar]

Yea, the call to extra_values.swap_remove isn't synchronized, so that's a problem. I think the solutions both have downsides, but I'll list them out:

• Remove the impl Send for ValueDrain, but this is an API breaking change. I suspect this should be incredibly uncommon. Perhaps the only sort of way someone would be affected is if using rayon::par_iter or similar with this.
• Put a Mutex into the ValueDrain. This negatively affects everyone for such a rare occurrence.
• Re-write the internals of HeaderMap to not store all extra values in a single Vec. This seems like the worst option, as it will take longer to do, and also negatively impacts memory usage and fragmentation.