Merge branch 'main' into feat-files-array-support

.github/workflows/ci-post-merge.yml

@@ -49,7 +49,7 @@ jobs:
           toolchain: ${{ matrix.version.version }}
 
       - name: Install just, cargo-hack, cargo-nextest, cargo-ci-cache-clean
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just,cargo-hack,cargo-nextest,cargo-ci-cache-clean
 
@@ -83,7 +83,7 @@ jobs:
         uses: actions-rust-lang/setup-rust-toolchain@a0b538fa0b742a6aa35d6e2c169b4bd06d225a98 # v1.15.3
 
       - name: Install just, cargo-hack
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just,cargo-hack
 

.github/workflows/ci.yml

@@ -64,7 +64,7 @@ jobs:
           toolchain: ${{ matrix.version.version }}
 
       - name: Install just, cargo-hack, cargo-nextest, cargo-ci-cache-clean
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just,cargo-hack,cargo-nextest,cargo-ci-cache-clean
 
@@ -117,7 +117,7 @@ jobs:
           toolchain: nightly
 
       - name: Install just
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just
 

.github/workflows/coverage.yml

@@ -24,7 +24,7 @@ jobs:
           components: llvm-tools
 
       - name: Install just, cargo-llvm-cov, cargo-nextest
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just,cargo-llvm-cov,cargo-nextest
 

.github/workflows/lint.yml

@@ -77,12 +77,12 @@ jobs:
           toolchain: ${{ vars.RUST_VERSION_EXTERNAL_TYPES }}
 
       - name: Install just
-        uses: taiki-e/install-action@68675c5a5f1a6950c3975d33f3ae0ef155e5bf3d # v2.68.15
+        uses: taiki-e/install-action@a37010ded18ff788be4440302bd6830b1ae50d8b # v2.68.25
         with:
           tool: just
 
       - name: Install cargo-check-external-types
-        uses: taiki-e/cache-cargo-install-action@2bfc3cedaf2ee5e7fa5d0ae034ccd5fb50cf8e1f # v3.0.2
+        uses: taiki-e/cache-cargo-install-action@59027ebf20a9617c4e819eb53ccd2673cb162b89 # v3.0.3
         with:
           tool: cargo-check-external-types
 

Cargo.lock

@@ -126,7 +126,7 @@ dependencies = [
  "serde_json",
  "serde_urlencoded",
  "slab",
- "socket2 0.6.2",
+ "socket2 0.6.3",
  "tokio",
 ]
 
@@ -372,7 +372,7 @@ dependencies = [
  "serde_json",
  "serde_urlencoded",
  "smallvec",
- "socket2 0.6.2",
+ "socket2 0.6.3",
  "static_assertions",
  "time",
  "tokio",
@@ -2215,9 +2215,9 @@ dependencies = [
 
 [[package]]
 name = "quote"
-version = "1.0.44"
+version = "1.0.45"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "21b2ebcf727b7760c461f091f9f0f539b77b8e87f2fd88131e7f1b433b3cece4"
+checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924"
 dependencies = [
  "proc-macro2",
 ]
@@ -2769,12 +2769,12 @@ dependencies = [
 
 [[package]]
 name = "socket2"
-version = "0.6.2"
+version = "0.6.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "86f4aa3ad99f2088c990dfa82d367e19cb29268ed67c574d10d0a4bfe71f07e0"
+checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e"
 dependencies = [
  "libc",
- "windows-sys 0.60.2",
+ "windows-sys 0.61.2",
 ]
 
 [[package]]
@@ -2987,9 +2987,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20"
 
 [[package]]
 name = "tokio"
-version = "1.49.0"
+version = "1.50.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "72a2903cd7736441aac9df9d7688bd0ce48edccaadf181c3b90be801e81d3d86"
+checksum = "27ad5e34374e03cfffefc301becb44e9dc3c17584f414349ebe29ed26661822d"
 dependencies = [
  "bytes",
  "libc",
@@ -2997,7 +2997,7 @@ dependencies = [
  "parking_lot",
  "pin-project-lite",
  "signal-hook-registry",
- "socket2 0.6.2",
+ "socket2 0.6.3",
  "tokio-macros",
  "windows-sys 0.61.2",
 ]

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
 

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<usize>) {
-        // take inner lower bound
+        (self.remaining, Some(self.remaining))
-        // make no attempt at an upper bound
-        (self.inner.size_hint().0, None)
     }
 }
 
@@ -818,14 +821,16 @@ pub struct Drain<'a> {
     inner: hash_map::Drain<'a, HeaderName, Value>,
     multi_inner: Option<(Option<HeaderName>, 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<usize>) {
-        // take inner lower bound
+        (self.remaining, Some(self.remaining))
-        // make no attempt at an upper bound
-        (self.inner.size_hint().0, None)
     }
 }
 
@@ -872,13 +876,15 @@ impl iter::FusedIterator for Drain<'_> {}
 pub struct IntoIter {
     inner: hash_map::IntoIter<HeaderName, Value>,
     multi_inner: Option<(HeaderName, smallvec::IntoIter<[HeaderValue; 4]>)>,
+    remaining: usize,
 }
 
 impl IntoIter {
-    fn new(inner: hash_map::IntoIter<HeaderName, Value>) -> Self {
+    fn new(inner: hash_map::IntoIter<HeaderName, Value>, 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<usize>) {
-        // take inner lower bound
+        (self.remaining, Some(self.remaining))
-        // make no attempt at an upper bound
-        (self.inner.size_hint().0, None)
     }
 }
 
@@ -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())
     }

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

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<String, ReadlinesError> {
+        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<T> Stream for Readlines<T>
@@ -95,18 +112,7 @@ where
                 if ind + 1 > this.limit {
                     return Poll::Ready(Some(Err(ReadlinesError::LimitOverflow)));
                 }
-                let line = if this.encoding == UTF_8 {
+                let line = Self::decode(this.encoding, &this.buf.split_to(ind + 1))?;
-                    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)?
-                };
                 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))
+                    this.buf.extend_from_slice(&bytes.split_to(ind + 1));
-                            .map_err(|_| ReadlinesError::EncodingError)?
+                    let line = Self::decode(this.encoding, &this.buf)?;
-                            .to_owned()
+                    this.buf.clear();
-                    } else {
+
-                        this.encoding
+                    // buffer bytes following the returned line
-                            .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);
-                    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 {
+                let line = Self::decode(this.encoding, &this.buf)?;
-                    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)?
-                };
                 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"
+        ));
+    }
 }