From 79434bde724f85d45c3d161a5ce603260043779a Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 10 Mar 2026 10:16:35 +0000 Subject: [PATCH 1/2] 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()) } From 9c2864c2c36bb7fc361b7ac8a120677d7bd98e3f Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 10 Mar 2026 10:28:26 +0000 Subject: [PATCH 2/2] fix: yield lines correctly in readlines across split chunks (#3970) --- actix-web/CHANGES.md | 1 + actix-web/src/types/readlines.rs | 116 ++++++++++++++++++++----------- 2 files changed, 78 insertions(+), 39 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index ba53e912a..59901dfd4 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -4,6 +4,7 @@ - Panic when calling `Route::to()` or `Route::service()` after `Route::wrap()` to prevent silently dropping route middleware. [#3944] - Fix `HttpRequest::{match_pattern,match_name}` reporting path-only matches when route guards disambiguate overlapping resources. [#3346] +- Fix `Readlines` handling of lines split across payload chunks so combined line limits are enforced and complete lines are yielded. [#3944]: https://github.com/actix/actix-web/pull/3944 [#3346]: https://github.com/actix/actix-web/issues/3346 diff --git a/actix-web/src/types/readlines.rs b/actix-web/src/types/readlines.rs index e75239968..17998a1a3 100644 --- a/actix-web/src/types/readlines.rs +++ b/actix-web/src/types/readlines.rs @@ -65,6 +65,23 @@ where err: Some(err), } } + + /// Decodes one complete logical line using the request's configured encoding. + /// + /// Callers are expected to pass only the bytes that belong to the line being yielded, + /// whether they came from the internal buffer, the current payload chunk, or both. + fn decode(encoding: &'static Encoding, bytes: &[u8]) -> Result { + if encoding == UTF_8 { + str::from_utf8(bytes) + .map_err(|_| ReadlinesError::EncodingError) + .map(str::to_owned) + } else { + encoding + .decode_without_bom_handling_and_without_replacement(bytes) + .map(Cow::into_owned) + .ok_or(ReadlinesError::EncodingError) + } + } } impl Stream for Readlines @@ -95,18 +112,7 @@ where if ind + 1 > this.limit { return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))); } - let line = if this.encoding == UTF_8 { - str::from_utf8(&this.buf.split_to(ind + 1)) - .map_err(|_| ReadlinesError::EncodingError)? - .to_owned() - } else { - this.encoding - .decode_without_bom_handling_and_without_replacement( - &this.buf.split_to(ind + 1), - ) - .map(Cow::into_owned) - .ok_or(ReadlinesError::EncodingError)? - }; + let line = Self::decode(this.encoding, &this.buf.split_to(ind + 1))?; return Poll::Ready(Some(Ok(line))); } this.checked_buff = true; @@ -125,24 +131,17 @@ where } if let Some(ind) = found { // check if line is longer than limit - if ind + 1 > this.limit { + if this.buf.len() + ind + 1 > this.limit { return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))); } - let line = if this.encoding == UTF_8 { - str::from_utf8(&bytes.split_to(ind + 1)) - .map_err(|_| ReadlinesError::EncodingError)? - .to_owned() - } else { - this.encoding - .decode_without_bom_handling_and_without_replacement( - &bytes.split_to(ind + 1), - ) - .map(Cow::into_owned) - .ok_or(ReadlinesError::EncodingError)? - }; - // extend buffer with rest of the bytes; + + this.buf.extend_from_slice(&bytes.split_to(ind + 1)); + let line = Self::decode(this.encoding, &this.buf)?; + this.buf.clear(); + + // buffer bytes following the returned line this.buf.extend_from_slice(&bytes); - this.checked_buff = false; + this.checked_buff = this.buf.is_empty(); return Poll::Ready(Some(Ok(line))); } this.buf.extend_from_slice(&bytes); @@ -156,16 +155,7 @@ where if this.buf.len() > this.limit { return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))); } - let line = if this.encoding == UTF_8 { - str::from_utf8(&this.buf) - .map_err(|_| ReadlinesError::EncodingError)? - .to_owned() - } else { - this.encoding - .decode_without_bom_handling_and_without_replacement(&this.buf) - .map(Cow::into_owned) - .ok_or(ReadlinesError::EncodingError)? - }; + let line = Self::decode(this.encoding, &this.buf)?; this.buf.clear(); Poll::Ready(Some(Ok(line))) } @@ -177,10 +167,16 @@ where #[cfg(test)] mod tests { - use futures_util::StreamExt as _; + use std::{ + pin::Pin, + task::{Context, Poll}, + }; + + use actix_http::{h1, Request}; + use futures_util::{task::noop_waker_ref, StreamExt as _}; use super::*; - use crate::test::TestRequest; + use crate::{error::ReadlinesError, test::TestRequest}; #[actix_rt::test] async fn test_readlines() { @@ -208,4 +204,46 @@ mod tests { "Contrary to popular belief, Lorem Ipsum is not simply random text." ); } + + #[test] + fn test_readlines_limit_across_chunks() { + let (mut sender, payload) = h1::Payload::create(false); + let payload: actix_http::Payload = payload.into(); + let mut req = Request::with_payload(payload); + let mut stream = Readlines::new(&mut req).limit(10); + let mut cx = Context::from_waker(noop_waker_ref()); + + sender.feed_data(Bytes::from_static(b"AAAAAAAAAA")); + assert!(matches!( + Pin::new(&mut stream).poll_next(&mut cx), + Poll::Pending + )); + + sender.feed_data(Bytes::from_static(b"A\n")); + assert!(matches!( + Pin::new(&mut stream).poll_next(&mut cx), + Poll::Ready(Some(Err(ReadlinesError::LimitOverflow))) + )); + } + + #[test] + fn test_readlines_returns_full_line_across_chunks() { + let (mut sender, payload) = h1::Payload::create(false); + let payload: actix_http::Payload = payload.into(); + let mut req = Request::with_payload(payload); + let mut stream = Readlines::new(&mut req); + let mut cx = Context::from_waker(noop_waker_ref()); + + sender.feed_data(Bytes::from_static(b"hello ")); + assert!(matches!( + Pin::new(&mut stream).poll_next(&mut cx), + Poll::Pending + )); + + sender.feed_data(Bytes::from_static(b"world\nnext")); + assert!(matches!( + Pin::new(&mut stream).poll_next(&mut cx), + Poll::Ready(Some(Ok(ref line))) if line == "hello world\n" + )); + } }