diff --git a/src/header/map.rs b/src/header/map.rs index ad6b66cd..02195f3b 100644 --- a/src/header/map.rs +++ b/src/header/map.rs @@ -206,10 +206,8 @@ pub struct ValueIterMut<'a, T> { /// An drain iterator of all values associated with a single header name. #[derive(Debug)] pub struct ValueDrain<'a, T> { - raw_links: RawLinks, - extra_values: *mut Vec>, first: Option, - next: Option, + next: Option<::std::vec::IntoIter>, lt: PhantomData<&'a mut HeaderMap>, } @@ -1193,13 +1191,16 @@ impl HeaderMap { } let raw_links = self.raw_links(); - let extra_values = &mut self.extra_values as *mut _; + let extra_values = &mut self.extra_values; + + let next = links.map(|l| { + drain_all_extra_values(raw_links, extra_values, l.next) + .into_iter() + }); ValueDrain { - raw_links, - extra_values, first: Some(old), - next: links.map(|l| l.next), + next: next, lt: PhantomData, } } @@ -1368,6 +1369,10 @@ impl HeaderMap { } /// Remove an entry from the map. + /// + /// Warning: To avoid inconsistent state, extra values _must_ be removed + /// for the `found` index (via `remove_all_extra_values` or similar) + /// _before_ this method is called. #[inline] fn remove_found(&mut self, probe: usize, found: usize) -> Bucket { // index `probe` and entry `found` is to be removed @@ -1587,7 +1592,12 @@ impl HeaderMap { /// Removes the `ExtraValue` at the given index. #[inline] -fn remove_extra_value(mut raw_links: RawLinks, extra_values: &mut Vec>, idx: usize) -> ExtraValue { +fn remove_extra_value( + mut raw_links: RawLinks, + extra_values: &mut Vec>, + idx: usize) + -> ExtraValue +{ let prev; let next; @@ -1703,6 +1713,26 @@ fn remove_extra_value(mut raw_links: RawLinks, extra_values: &mut Vec( + raw_links: RawLinks, + extra_values: &mut Vec>, + mut head: usize) + -> Vec +{ + let mut vec = Vec::new(); + loop { + let extra = remove_extra_value(raw_links, extra_values, head); + vec.push(extra.value); + + if let Link::Extra(idx) = extra.next { + head = idx; + } else { + break; + } + } + vec +} + impl<'a, T> IntoIterator for &'a HeaderMap { type Item = (&'a HeaderName, &'a T); type IntoIter = Iter<'a, T>; @@ -2922,12 +2952,12 @@ impl<'a, T> OccupiedEntry<'a, T> { /// assert!(!map.contains_key("host")); /// ``` pub fn remove_entry(self) -> (HeaderName, T) { - let entry = self.map.remove_found(self.probe, self.index); - - if let Some(links) = entry.links { + if let Some(links) = self.map.entries[self.index].links { self.map.remove_all_extra_values(links.next); } + let entry = self.map.remove_found(self.probe, self.index); + (entry.key, entry.value) } @@ -2936,14 +2966,19 @@ impl<'a, T> OccupiedEntry<'a, T> { /// The key and all values associated with the entry are removed and /// returned. pub fn remove_entry_mult(self) -> (HeaderName, ValueDrain<'a, T>) { - let entry = self.map.remove_found(self.probe, self.index); let raw_links = self.map.raw_links(); - let extra_values = &mut self.map.extra_values as *mut _; + let extra_values = &mut self.map.extra_values; + + let next = self.map.entries[self.index].links.map(|l| { + drain_all_extra_values(raw_links, extra_values, l.next) + .into_iter() + }); + + let entry = self.map.remove_found(self.probe, self.index); + let drain = ValueDrain { - raw_links, - extra_values, first: Some(entry.value), - next: entry.links.map(|l| l.next), + next, lt: PhantomData, }; (entry.key, drain) @@ -3036,31 +3071,26 @@ impl<'a, T> Iterator for ValueDrain<'a, T> { fn next(&mut self) -> Option { if self.first.is_some() { self.first.take() - } else if let Some(next) = self.next { - // Remove the extra value - let extra = unsafe { - remove_extra_value(self.raw_links, &mut *self.extra_values, next) - }; - - match extra.next { - Link::Extra(idx) => self.next = Some(idx), - Link::Entry(_) => self.next = None, - } - - Some(extra.value) + } else if let Some(ref mut extras) = self.next { + extras.next() } else { None } } fn size_hint(&self) -> (usize, Option) { - match (&self.first, self.next) { + match (&self.first, &self.next) { // Exactly 1 - (&Some(_), None) => (1, Some(1)), - // At least 1 - (&_, Some(_)) => (1, None), + (&Some(_), &None) => (1, Some(1)), + // 1 + extras + (&Some(_), &Some(ref extras)) => { + let (l, u) = extras.size_hint(); + (l + 1, u.map(|u| u + 1)) + }, + // Extras only + (&None, &Some(ref extras)) => extras.size_hint(), // No more - (&None, None) => (0, Some(0)), + (&None, &None) => (0, Some(0)), } } } diff --git a/tests/header_map.rs b/tests/header_map.rs index 39893568..d02a5106 100644 --- a/tests/header_map.rs +++ b/tests/header_map.rs @@ -175,8 +175,9 @@ fn drain_entry() { "more".parse::().unwrap(), "insertions".parse().unwrap(), ); + assert_eq!(5, headers.len()); - // Using insert + // Using insert_mult { let mut e = match headers.entry("hello") { Entry::Occupied(e) => e, @@ -188,6 +189,8 @@ fn drain_entry() { assert_eq!(vals[0], "world"); assert_eq!(vals[1], "world2"); } + + assert_eq!(5-2+1, headers.len()); } #[test] @@ -421,3 +424,213 @@ fn value_htab() { HeaderValue::from_static("hello\tworld"); HeaderValue::from_str("hello\tworld").unwrap(); } + + +#[test] +fn remove_multiple_a() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + headers.insert(VARY, "*".parse().unwrap()); + + assert_eq!(headers.len(), 6); + + let cookie = headers.remove(SET_COOKIE); + assert_eq!(cookie, Some("cookie_1=value 1".parse().unwrap())); + assert_eq!(headers.len(), 3); + + let via = headers.remove(VIA); + assert_eq!(via, Some("1.1 example.com".parse().unwrap())); + assert_eq!(headers.len(), 1); + + let vary = headers.remove(VARY); + assert_eq!(vary, Some("*".parse().unwrap())); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_multiple_b() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + headers.insert(VARY, "*".parse().unwrap()); + + assert_eq!(headers.len(), 6); + + let vary = headers.remove(VARY); + assert_eq!(vary, Some("*".parse().unwrap())); + assert_eq!(headers.len(), 5); + + let via = headers.remove(VIA); + assert_eq!(via, Some("1.1 example.com".parse().unwrap())); + assert_eq!(headers.len(), 3); + + let cookie = headers.remove(SET_COOKIE); + assert_eq!(cookie, Some("cookie_1=value 1".parse().unwrap())); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_multi_0() { + let mut headers = HeaderMap::new(); + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 0); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_multi_0_others() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 0); + assert_eq!(headers.len(), 2); +} + +#[test] +fn remove_entry_multi_1() { + let mut headers = HeaderMap::new(); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 1); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_multi_1_other() { + let mut headers = HeaderMap::new(); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 1); + assert_eq!(headers.len(), 1); + + let vias = remove_all_values(&mut headers, VIA); + assert_eq!(vias.len(), 1); + assert_eq!(headers.len(), 0); +} + +// For issue hyperimum/http#446 +#[test] +fn remove_entry_multi_2() { + let mut headers = HeaderMap::new(); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 2); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_multi_3() { + let mut headers = HeaderMap::new(); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 3); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_multi_3_others() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + headers.insert(VARY, "*".parse().unwrap()); + + let cookies = remove_all_values(&mut headers, SET_COOKIE); + assert_eq!(cookies.len(), 3); + assert_eq!(headers.len(), 3); + + let vias = remove_all_values(&mut headers, VIA); + assert_eq!(vias.len(), 2); + assert_eq!(headers.len(), 1); + + let varies = remove_all_values(&mut headers, VARY); + assert_eq!(varies.len(), 1); + assert_eq!(headers.len(), 0); +} + +fn remove_all_values(headers: &mut HeaderMap, key: K) -> Vec + where K: IntoHeaderName +{ + match headers.entry(key) { + Entry::Occupied(e) => e.remove_entry_mult().1.collect(), + Entry::Vacant(_) => vec![], + } +} + +#[test] +fn remove_entry_3_others_a() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + headers.insert(VARY, "*".parse().unwrap()); + + assert_eq!(headers.len(), 6); + + let cookie = remove_values(&mut headers, SET_COOKIE); + assert_eq!(cookie, Some("cookie_1=value 1".parse().unwrap())); + assert_eq!(headers.len(), 3); + + let via = remove_values(&mut headers, VIA); + assert_eq!(via, Some("1.1 example.com".parse().unwrap())); + assert_eq!(headers.len(), 1); + + let vary = remove_values(&mut headers, VARY); + assert_eq!(vary, Some("*".parse().unwrap())); + assert_eq!(headers.len(), 0); +} + +#[test] +fn remove_entry_3_others_b() { + let mut headers = HeaderMap::new(); + headers.insert(VIA, "1.1 example.com".parse().unwrap()); + headers.insert(SET_COOKIE, "cookie_1=value 1".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_2=value 2".parse().unwrap()); + headers.append(VIA, "1.1 other.com".parse().unwrap()); + headers.append(SET_COOKIE, "cookie_3=value 3".parse().unwrap()); + headers.insert(VARY, "*".parse().unwrap()); + + assert_eq!(headers.len(), 6); + + let vary = remove_values(&mut headers, VARY); + assert_eq!(vary, Some("*".parse().unwrap())); + assert_eq!(headers.len(), 5); + + let via = remove_values(&mut headers, VIA); + assert_eq!(via, Some("1.1 example.com".parse().unwrap())); + assert_eq!(headers.len(), 3); + + let cookie = remove_values(&mut headers, SET_COOKIE); + assert_eq!(cookie, Some("cookie_1=value 1".parse().unwrap())); + assert_eq!(headers.len(), 0); +} + +fn remove_values(headers: &mut HeaderMap, key: K) -> Option + where K: IntoHeaderName +{ + match headers.entry(key) { + Entry::Occupied(e) => Some(e.remove_entry().1), + Entry::Vacant(_) => None, + } +}