From 79434bde724f85d45c3d161a5ce603260043779a Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 10 Mar 2026 10:16:35 +0000 Subject: [PATCH] fix: correct HeaderMap iterators' len and size_hint for multi-value (#3971) * test: add headermap iterator tests confirmed new tests fail as expected * fix: correct HeaderMap iterators' len and size_hint for multi-value headers fixes #3969 --- actix-http/CHANGES.md | 1 + actix-http/src/header/map.rs | 71 ++++++++++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 9edd7b6f0..a627718b0 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -3,6 +3,7 @@ ## Unreleased - Encode the HTTP/1 `Connection: Upgrade` header in Camel-Case when camel-case header formatting is enabled.[#3953] +- Fix `HeaderMap` iterators' `len()` and `size_hint()` implementations for multi-value headers. [#3953]: https://github.com/actix/actix-web/pull/3953 diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index a9a201e1a..69f0aa2a7 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -537,7 +537,7 @@ impl HeaderMap { /// assert!(pairs.contains(&(&header::SET_COOKIE, &HeaderValue::from_static("two=2")))); /// ``` pub fn iter(&self) -> Iter<'_> { - Iter::new(self.inner.iter()) + Iter::new(self.inner.iter(), self.len()) } /// An iterator over all contained header names. @@ -626,7 +626,8 @@ impl HeaderMap { /// assert!(map.is_empty()); /// ``` pub fn drain(&mut self) -> Drain<'_> { - Drain::new(self.inner.drain()) + let len = self.len(); + Drain::new(self.inner.drain(), len) } } @@ -638,7 +639,8 @@ impl IntoIterator for HeaderMap { #[inline] fn into_iter(self) -> Self::IntoIter { - IntoIter::new(self.inner.into_iter()) + let len = self.len(); + IntoIter::new(self.inner.into_iter(), len) } } @@ -648,7 +650,7 @@ impl<'a> IntoIterator for &'a HeaderMap { #[inline] fn into_iter(self) -> Self::IntoIter { - Iter::new(self.inner.iter()) + Iter::new(self.inner.iter(), self.len()) } } @@ -760,14 +762,16 @@ pub struct Iter<'a> { inner: hash_map::Iter<'a, HeaderName, Value>, multi_inner: Option<(&'a HeaderName, &'a SmallVec<[HeaderValue; 4]>)>, multi_idx: usize, + remaining: usize, } impl<'a> Iter<'a> { - fn new(iter: hash_map::Iter<'a, HeaderName, Value>) -> Self { + fn new(iter: hash_map::Iter<'a, HeaderName, Value>, remaining: usize) -> Self { Self { inner: iter, multi_idx: 0, multi_inner: None, + remaining, } } } @@ -781,6 +785,7 @@ impl<'a> Iterator for Iter<'a> { match vals.get(self.multi_idx) { Some(val) => { self.multi_idx += 1; + self.remaining -= 1; return Some((name, val)); } None => { @@ -800,9 +805,7 @@ impl<'a> Iterator for Iter<'a> { #[inline] fn size_hint(&self) -> (usize, Option) { - // take inner lower bound - // make no attempt at an upper bound - (self.inner.size_hint().0, None) + (self.remaining, Some(self.remaining)) } } @@ -818,14 +821,16 @@ pub struct Drain<'a> { inner: hash_map::Drain<'a, HeaderName, Value>, multi_inner: Option<(Option, SmallVec<[HeaderValue; 4]>)>, multi_idx: usize, + remaining: usize, } impl<'a> Drain<'a> { - fn new(iter: hash_map::Drain<'a, HeaderName, Value>) -> Self { + fn new(iter: hash_map::Drain<'a, HeaderName, Value>, remaining: usize) -> Self { Self { inner: iter, multi_inner: None, multi_idx: 0, + remaining, } } } @@ -838,6 +843,7 @@ impl Iterator for Drain<'_> { if let Some((ref mut name, ref mut vals)) = self.multi_inner { if !vals.is_empty() { // OPTIMIZE: array removals + self.remaining -= 1; return Some((name.take(), vals.remove(0))); } else { // no more items in value iterator; reset state @@ -855,9 +861,7 @@ impl Iterator for Drain<'_> { #[inline] fn size_hint(&self) -> (usize, Option) { - // take inner lower bound - // make no attempt at an upper bound - (self.inner.size_hint().0, None) + (self.remaining, Some(self.remaining)) } } @@ -872,13 +876,15 @@ impl iter::FusedIterator for Drain<'_> {} pub struct IntoIter { inner: hash_map::IntoIter, multi_inner: Option<(HeaderName, smallvec::IntoIter<[HeaderValue; 4]>)>, + remaining: usize, } impl IntoIter { - fn new(inner: hash_map::IntoIter) -> Self { + fn new(inner: hash_map::IntoIter, remaining: usize) -> Self { Self { inner, multi_inner: None, + remaining, } } } @@ -891,6 +897,7 @@ impl Iterator for IntoIter { if let Some((ref name, ref mut vals)) = self.multi_inner { match vals.next() { Some(val) => { + self.remaining -= 1; return Some((name.clone(), val)); } None => { @@ -909,9 +916,7 @@ impl Iterator for IntoIter { #[inline] fn size_hint(&self) -> (usize, Option) { - // take inner lower bound - // make no attempt at an upper bound - (self.inner.size_hint().0, None) + (self.remaining, Some(self.remaining)) } } @@ -1160,6 +1165,40 @@ mod tests { assert!(vals.next().is_none()); } + #[test] + fn iter_len_counts_values() { + let mut map = HeaderMap::new(); + map.append(header::SET_COOKIE, HeaderValue::from_static("a=1")); + map.append(header::SET_COOKIE, HeaderValue::from_static("b=2")); + map.append(header::SET_COOKIE, HeaderValue::from_static("c=3")); + + assert_eq!(map.iter().count(), 3); + assert_eq!(map.iter().len(), 3); + } + + #[test] + fn into_iter_len_counts_values() { + let mut map = HeaderMap::new(); + map.append(header::SET_COOKIE, HeaderValue::from_static("a=1")); + map.append(header::SET_COOKIE, HeaderValue::from_static("b=2")); + map.append(header::SET_COOKIE, HeaderValue::from_static("c=3")); + + assert_eq!(map.clone().into_iter().count(), 3); + assert_eq!(map.into_iter().len(), 3); + } + + #[test] + fn drain_len_counts_values() { + let mut map = HeaderMap::new(); + map.append(header::SET_COOKIE, HeaderValue::from_static("a=1")); + map.append(header::SET_COOKIE, HeaderValue::from_static("b=2")); + map.append(header::SET_COOKIE, HeaderValue::from_static("c=3")); + + let mut drained = map.clone(); + assert_eq!(map.drain().count(), 3); + assert_eq!(drained.drain().len(), 3); + } + fn owned_pair<'a>((name, val): (&'a HeaderName, &'a HeaderValue)) -> (HeaderName, HeaderValue) { (name.clone(), val.clone()) }