Merge branch 'master' into dep/pin-project

actix-http/CHANGES.md

@@ -3,9 +3,11 @@
 ## Unreleased - 2021-xx-xx
 ### Changed
 * Guarantee ordering of `header::GetAll` iterator to be same as insertion order. [#2467]
-* Expose `header::{GetAll, Removed}` iterators. [#2467]
+* Expose `header::map` module. [#2467]
+* Implement `ExactSizeIterator` and `FusedIterator` for all `HeaderMap` iterators. [#2470]
 
 [#2467]: https://github.com/actix/actix-web/pull/2467
+[#2470]: https://github.com/actix/actix-web/pull/2470
 
 
 ## 3.0.0-beta.13 - 2021-11-22

actix-http/src/header/as_name.rs

@@ -1,11 +1,12 @@
-//! Helper trait for types that can be effectively borrowed as a [HeaderValue].
-//!
-//! [HeaderValue]: crate::http::HeaderValue
+//! Sealed [`AsHeaderName`] trait and implementations.
 
-use std::{borrow::Cow, str::FromStr};
+use std::{borrow::Cow, str::FromStr as _};
 
 use http::header::{HeaderName, InvalidHeaderName};
 
+/// Sealed trait implemented for types that can be effectively borrowed as a [`HeaderValue`].
+///
+/// [`HeaderValue`]: crate::http::HeaderValue
 pub trait AsHeaderName: Sealed {}
 
 pub struct Seal;

actix-http/src/header/into_pair.rs

@@ -1,4 +1,6 @@
-use std::convert::TryFrom;
+//! [`IntoHeaderPair`] trait and implementations.
+
+use std::convert::TryFrom as _;
 
 use http::{
     header::{HeaderName, InvalidHeaderName, InvalidHeaderValue},
@@ -7,7 +9,10 @@ use http::{
 
 use super::{Header, IntoHeaderValue};
 
-/// Transforms structures into header K/V pairs for inserting into `HeaderMap`s.
+/// An interface for types that can be converted into a [`HeaderName`]/[`HeaderValue`] pair for
+/// insertion into a [`HeaderMap`].
+///
+/// [`HeaderMap`]: crate::http::HeaderMap
 pub trait IntoHeaderPair: Sized {
     type Error: Into<HttpError>;
 

actix-http/src/header/into_value.rs

@@ -1,10 +1,12 @@
-use std::convert::TryFrom;
+//! [`IntoHeaderValue`] trait and implementations.
+
+use std::convert::TryFrom as _;
 
 use bytes::Bytes;
 use http::{header::InvalidHeaderValue, Error as HttpError, HeaderValue};
 use mime::Mime;
 
-/// A trait for any object that can be Converted to a `HeaderValue`
+/// An interface for types that can be converted into a [`HeaderValue`].
 pub trait IntoHeaderValue: Sized {
     /// The type returned in the event of a conversion error.
     type Error: Into<HttpError>;

actix-http/src/header/map.rs

@@ -1,6 +1,6 @@
 //! A multi-value [`HeaderMap`] and its iterators.
 
-use std::{borrow::Cow, collections::hash_map, ops};
+use std::{borrow::Cow, collections::hash_map, iter, ops};
 
 use ahash::AHashMap;
 use http::header::{HeaderName, HeaderValue};
@@ -581,7 +581,8 @@ impl HeaderMap {
     }
 }
 
-/// Note that this implementation will clone a [HeaderName] for each value.
+/// Note that this implementation will clone a [HeaderName] for each value. Consider using
+/// [`drain`](Self::drain) to control header name cloning.
 impl IntoIterator for HeaderMap {
     type Item = (HeaderName, HeaderValue);
     type IntoIter = IntoIter;
@@ -602,7 +603,7 @@ impl<'a> IntoIterator for &'a HeaderMap {
     }
 }
 
-/// Iterator for references of [`HeaderValue`]s with the same associated [`HeaderName`].
+/// Iterator over borrowed values with the same associated name.
 ///
 /// See [`HeaderMap::get_all`].
 #[derive(Debug)]
@@ -644,10 +645,14 @@ impl<'a> Iterator for GetAll<'a> {
     }
 }
 
-/// Iterator for owned [`HeaderValue`]s with the same associated [`HeaderName`] returned from
-/// methods that remove or replace items.
+impl ExactSizeIterator for GetAll<'_> {}
+
+impl iter::FusedIterator for GetAll<'_> {}
+
+/// Iterator over removed, owned values with the same associated name.
 ///
-/// See [`HeaderMap::insert`] and [`HeaderMap::remove`].
+/// Returned from methods that remove or replace items. See [`HeaderMap::insert`]
+/// and [`HeaderMap::remove`].
 #[derive(Debug)]
 pub struct Removed {
     inner: Option<smallvec::IntoIter<[HeaderValue; 4]>>,
@@ -689,7 +694,11 @@ impl Iterator for Removed {
     }
 }
 
-/// Iterator over all [`HeaderName`]s in the map.
+impl ExactSizeIterator for Removed {}
+
+impl iter::FusedIterator for Removed {}
+
+/// Iterator over all names in the map.
 #[derive(Debug)]
 pub struct Keys<'a>(hash_map::Keys<'a, HeaderName, Value>);
 
@@ -707,6 +716,11 @@ impl<'a> Iterator for Keys<'a> {
     }
 }
 
+impl ExactSizeIterator for Keys<'_> {}
+
+impl iter::FusedIterator for Keys<'_> {}
+
+/// Iterator over borrowed name-value pairs.
 #[derive(Debug)]
 pub struct Iter<'a> {
     inner: hash_map::Iter<'a, HeaderName, Value>,
@@ -758,6 +772,10 @@ impl<'a> Iterator for Iter<'a> {
     }
 }
 
+impl ExactSizeIterator for Iter<'_> {}
+
+impl iter::FusedIterator for Iter<'_> {}
+
 /// Iterator over drained name-value pairs.
 ///
 /// Iterator items are `(Option<HeaderName>, HeaderValue)` to avoid cloning.
@@ -809,6 +827,10 @@ impl<'a> Iterator for Drain<'a> {
     }
 }
 
+impl ExactSizeIterator for Drain<'_> {}
+
+impl iter::FusedIterator for Drain<'_> {}
+
 /// Iterator over owned name-value pairs.
 ///
 /// Implementation necessarily clones header names for each value.
@@ -859,12 +881,27 @@ impl Iterator for IntoIter {
     }
 }
 
+impl ExactSizeIterator for IntoIter {}
+
+impl iter::FusedIterator for IntoIter {}
+
 #[cfg(test)]
 mod tests {
+    use std::iter::FusedIterator;
+
     use http::header;
+    use static_assertions::assert_impl_all;
 
     use super::*;
 
+    assert_impl_all!(HeaderMap: IntoIterator);
+    assert_impl_all!(Keys<'_>: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(GetAll<'_>: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(Removed: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(Iter<'_>: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(IntoIter: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(Drain<'_>: Iterator, ExactSizeIterator, FusedIterator);
+
     #[test]
     fn create() {
         let map = HeaderMap::new();
@@ -1035,6 +1072,9 @@ mod tests {
         assert_eq!(vals.next().unwrap().as_bytes(), b"8");
         assert_eq!(vals.next().unwrap().as_bytes(), b"9");
         assert!(vals.next().is_none());
+
+        // check for fused-ness
+        assert!(vals.next().is_none());
     }
 
     fn owned_pair<'a>(

actix-http/src/header/mod.rs

@@ -34,7 +34,7 @@ use crate::{error::ParseError, HttpMessage};
 mod as_name;
 mod into_pair;
 mod into_value;
-pub(crate) mod map;
+pub mod map;
 mod shared;
 mod utils;
 
@@ -44,12 +44,12 @@ pub use self::shared::*;
 pub use self::as_name::AsHeaderName;
 pub use self::into_pair::IntoHeaderPair;
 pub use self::into_value::IntoHeaderValue;
-pub use self::map::{GetAll, HeaderMap, Removed};
+pub use self::map::HeaderMap;
 pub use self::utils::{
     fmt_comma_delimited, from_comma_delimited, from_one_raw_str, http_percent_encode,
 };
 
-/// A trait for any object that already represents a valid header field and value.
+/// An interface for types that already represent a valid header.
 pub trait Header: IntoHeaderValue {
     /// Returns the name of the header field
     fn name() -> HeaderName;

actix-http/src/header/shared/content_encoding.rs

@@ -9,14 +9,17 @@ use crate::{
     HttpMessage,
 };
 
-/// Error return when a content encoding is unknown.
-///
-/// Example: 'compress'
+/// Error returned when a content encoding is unknown.
 #[derive(Debug, Display, Error)]
 #[display(fmt = "unsupported content encoding")]
 pub struct ContentEncodingParseError;
 
 /// Represents a supported content encoding.
+///
+/// Includes a commonly-used subset of media types appropriate for use as HTTP content encodings.
+/// See [IANA HTTP Content Coding Registry].
+///
+/// [IANA HTTP Content Coding Registry]: https://www.iana.org/assignments/http-parameters/http-parameters.xhtml
 #[derive(Debug, Clone, Copy, PartialEq)]
 #[non_exhaustive]
 pub enum ContentEncoding {
@@ -32,7 +35,7 @@ pub enum ContentEncoding {
     /// Gzip algorithm.
     Gzip,
 
-    // Zstd algorithm.
+    /// Zstd algorithm.
     Zstd,
 
     /// Indicates the identity function (i.e. no compression, nor modification).

actix-http/src/header/utils.rs

@@ -1,3 +1,5 @@
+//! Header parsing utilities.
+
 use std::{fmt, str::FromStr};
 
 use super::HeaderValue;

actix-multipart/CHANGES.md

@@ -10,8 +10,10 @@
 * Added `Field::name` method for getting the field name. [#2451]
 * `MultipartError` now marks variants with inner errors as the source. [#2451]
 * `MultipartError` is now marked as non-exhaustive. [#2451]
+* Polling `Field` after dropping `Multipart` now fails immediately instead of hanging forever. [#2463]
 
 [#2451]: https://github.com/actix/actix-web/pull/2451
+[#2463]: https://github.com/actix/actix-web/pull/2463
 
 
 ## 0.4.0-beta.7 - 2021-10-20

actix-multipart/src/server.rs

@@ -706,8 +706,11 @@ impl Clone for PayloadRef {
     }
 }
 
-/// Counter. It tracks of number of clones of payloads and give access to payload only to top most
-/// task panics if Safety get destroyed and it not top most task.
+/// Counter. It tracks of number of clones of payloads and give access to payload only to top most.
+/// * When dropped, parent task is awakened. This is to support the case where Field is
+/// dropped in a separate task than Multipart.
+/// * Assumes that parent owners don't move to different tasks; only the top-most is allowed to.
+/// * If dropped and is not top most owner, is_clean flag is set to false.
 #[derive(Debug)]
 struct Safety {
     task: LocalWaker,
@@ -750,9 +753,9 @@ impl Safety {
 
 impl Drop for Safety {
     fn drop(&mut self) {
-        // parent task is dead
         if Rc::strong_count(&self.payload) != self.level {
-            self.clean.set(true);
+            // Multipart dropped leaving a Field
+            self.clean.set(false);
         }
 
         self.task.wake();
@@ -853,10 +856,12 @@ mod tests {
 
     use actix_http::h1::Payload;
     use actix_web::http::header::{DispositionParam, DispositionType};
+    use actix_web::rt;
     use actix_web::test::TestRequest;
     use actix_web::FromRequest;
     use bytes::Bytes;
     use futures_util::{future::lazy, StreamExt};
+    use std::time::Duration;
     use tokio::sync::mpsc;
     use tokio_stream::wrappers::UnboundedReceiverStream;
 
@@ -1286,4 +1291,44 @@ mod tests {
             MultipartError::NoContentDisposition,
         ));
     }
+
+    #[actix_rt::test]
+    async fn test_drop_multipart_dont_hang() {
+        let (sender, payload) = create_stream();
+        let (bytes, headers) = create_simple_request_with_header();
+        sender.send(Ok(bytes)).unwrap();
+        drop(sender); // eof
+
+        let mut multipart = Multipart::new(&headers, payload);
+        let mut field = multipart.next().await.unwrap().unwrap();
+
+        drop(multipart);
+
+        // should fail immediately
+        match field.next().await {
+            Some(Err(MultipartError::NotConsumed)) => {}
+            _ => panic!(),
+        };
+    }
+
+    #[actix_rt::test]
+    async fn test_drop_field_awaken_multipart() {
+        let (sender, payload) = create_stream();
+        let (bytes, headers) = create_simple_request_with_header();
+        sender.send(Ok(bytes)).unwrap();
+        drop(sender); // eof
+
+        let mut multipart = Multipart::new(&headers, payload);
+        let mut field = multipart.next().await.unwrap().unwrap();
+
+        let task = rt::spawn(async move {
+            rt::time::sleep(Duration::from_secs(1)).await;
+            assert_eq!(field.next().await.unwrap().unwrap(), "test");
+            drop(field);
+        });
+
+        // dropping field should awaken current task
+        let _ = multipart.next().await.unwrap().unwrap();
+        task.await.unwrap();
+    }
 }

src/response/response.rs

@@ -296,7 +296,7 @@ impl Future for HttpResponse<AnyBody> {
 
 #[cfg(feature = "cookies")]
 pub struct CookieIter<'a> {
-    iter: header::GetAll<'a>,
+    iter: header::map::GetAll<'a>,
 }
 
 #[cfg(feature = "cookies")]