Merge branch 'master' into get-all-order

closes #2466

.github/workflows/ci.yml

@@ -159,7 +159,7 @@ jobs:
         with: { file: cobertura.xml }
 
   rustdoc:
-    name: rustdoc
+    name: doc tests
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -177,12 +177,6 @@ jobs:
       - name: Cache Dependencies
         uses: Swatinem/rust-cache@v1.3.0
 
-      # - name: Install cargo-hack
-      #   uses: actions-rs/cargo@v1
-      #   with:
-      #     command: install
-      #     args: cargo-hack
-
       - name: doc tests
         uses: actions-rs/cargo@v1
         timeout-minutes: 60

actix-http/CHANGES.md

@@ -1,6 +1,11 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+### Changed
+* Guarantee ordering of `header::GetAll` iterator to be same as insertion order. [#2467]
+* Expose `header::{GetAll, Removed}` iterators. [#2467]
+
+[#2467]: https://github.com/actix/actix-web/pull/2467
 
 
 ## 3.0.0-beta.13 - 2021-11-22

actix-http/src/header/map.rs

@@ -288,7 +288,7 @@ impl HeaderMap {
     /// Returns an iterator over all values associated with a header name.
     ///
     /// The returned iterator does not incur any allocations and will yield no items if there are no
-    /// values associated with the key. Iteration order is **not** guaranteed to be the same as
+    /// values associated with the key. Iteration order is guaranteed to be the same as
     /// insertion order.
     ///
     /// # Examples
@@ -355,6 +355,19 @@ impl HeaderMap {
     ///
     /// assert_eq!(map.len(), 1);
     /// ```
+    ///
+    /// A convenience method is provided on the returned iterator to check if the insertion replaced
+    /// any values.
+    /// ```
+    /// # use actix_http::http::{header, HeaderMap, HeaderValue};
+    /// let mut map = HeaderMap::new();
+    ///
+    /// let removed = map.insert(header::ACCEPT, HeaderValue::from_static("text/plain"));
+    /// assert!(removed.is_empty());
+    ///
+    /// let removed = map.insert(header::ACCEPT, HeaderValue::from_static("text/html"));
+    /// assert!(!removed.is_empty());
+    /// ```
     pub fn insert(&mut self, key: HeaderName, val: HeaderValue) -> Removed {
         let value = self.inner.insert(key, Value::one(val));
         Removed::new(value)
@@ -393,6 +406,9 @@ impl HeaderMap {
 
     /// Removes all headers for a particular header name from the map.
     ///
+    /// Providing an invalid header names (as a string argument) will have no effect and return
+    /// without error.
+    ///
     /// # Examples
     /// ```
     /// # use actix_http::http::{header, HeaderMap, HeaderValue};
@@ -409,6 +425,21 @@ impl HeaderMap {
     /// assert!(removed.next().is_none());
     ///
     /// assert!(map.is_empty());
+    /// ```
+    ///
+    /// A convenience method is provided on the returned iterator to check if the `remove` call
+    /// actually removed any values.
+    /// ```
+    /// # use actix_http::http::{header, HeaderMap, HeaderValue};
+    /// let mut map = HeaderMap::new();
+    ///
+    /// let removed = map.remove("accept");
+    /// assert!(removed.is_empty());
+    ///
+    /// map.insert(header::ACCEPT, HeaderValue::from_static("text/html"));
+    /// let removed = map.remove("accept");
+    /// assert!(!removed.is_empty());
+    /// ```
     pub fn remove(&mut self, key: impl AsHeaderName) -> Removed {
         let value = match key.try_as_name(super::as_name::Seal) {
             Ok(Cow::Borrowed(name)) => self.inner.remove(name),
@@ -571,7 +602,7 @@ impl<'a> IntoIterator for &'a HeaderMap {
     }
 }
 
-/// Iterator for all values with the same header name.
+/// Iterator for references of [`HeaderValue`]s with the same associated [`HeaderName`].
 ///
 /// See [`HeaderMap::get_all`].
 #[derive(Debug)]
@@ -613,18 +644,32 @@ impl<'a> Iterator for GetAll<'a> {
     }
 }
 
-/// Iterator for owned [`HeaderValue`]s with the same associated [`HeaderName`] returned from methods
-/// on [`HeaderMap`] that remove or replace items.
+/// Iterator for owned [`HeaderValue`]s with the same associated [`HeaderName`] 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]>>,
 }
 
-impl<'a> Removed {
+impl Removed {
     fn new(value: Option<Value>) -> Self {
         let inner = value.map(|value| value.inner.into_iter());
         Self { inner }
     }
+
+    /// Returns true if iterator contains no elements, without consuming it.
+    ///
+    /// If called immediately after [`HeaderMap::insert`] or [`HeaderMap::remove`], it will indicate
+    /// wether any items were actually replaced or removed, respectively.
+    pub fn is_empty(&self) -> bool {
+        match self.inner {
+            // size hint lower bound of smallvec is the correct length
+            Some(ref iter) => iter.size_hint().0 == 0,
+            None => true,
+        }
+    }
 }
 
 impl Iterator for Removed {
@@ -945,6 +990,53 @@ mod tests {
         assert_eq!(vals.next(), removed.next().as_ref());
     }
 
+    #[test]
+    fn get_all_iteration_order_matches_insertion_order() {
+        let mut map = HeaderMap::new();
+
+        let mut vals = map.get_all(header::COOKIE);
+        assert!(vals.next().is_none());
+
+        map.append(header::COOKIE, HeaderValue::from_static("1"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"1");
+        assert!(vals.next().is_none());
+
+        map.append(header::COOKIE, HeaderValue::from_static("2"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"1");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"2");
+        assert!(vals.next().is_none());
+
+        map.append(header::COOKIE, HeaderValue::from_static("3"));
+        map.append(header::COOKIE, HeaderValue::from_static("4"));
+        map.append(header::COOKIE, HeaderValue::from_static("5"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"1");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"2");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"3");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"4");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"5");
+        assert!(vals.next().is_none());
+
+        let _ = map.insert(header::COOKIE, HeaderValue::from_static("6"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"6");
+        assert!(vals.next().is_none());
+
+        let _ = map.insert(header::COOKIE, HeaderValue::from_static("7"));
+        let _ = map.insert(header::COOKIE, HeaderValue::from_static("8"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"8");
+        assert!(vals.next().is_none());
+
+        map.append(header::COOKIE, HeaderValue::from_static("9"));
+        let mut vals = map.get_all(header::COOKIE);
+        assert_eq!(vals.next().unwrap().as_bytes(), b"8");
+        assert_eq!(vals.next().unwrap().as_bytes(), b"9");
+        assert!(vals.next().is_none());
+    }
+
     fn owned_pair<'a>(
         (name, val): (&'a HeaderName, &'a HeaderValue),
     ) -> (HeaderName, HeaderValue) {

actix-http/src/header/mod.rs

@@ -29,16 +29,14 @@ pub use http::header::{
     X_FRAME_OPTIONS, X_XSS_PROTECTION,
 };
 
-use crate::error::ParseError;
-use crate::HttpMessage;
+use crate::{error::ParseError, HttpMessage};
 
 mod as_name;
 mod into_pair;
 mod into_value;
-mod utils;
-
 pub(crate) mod map;
 mod shared;
+mod utils;
 
 #[doc(hidden)]
 pub use self::shared::*;
@@ -46,10 +44,10 @@ pub use self::shared::*;
 pub use self::as_name::AsHeaderName;
 pub use self::into_pair::IntoHeaderPair;
 pub use self::into_value::IntoHeaderValue;
-#[doc(hidden)]
-pub use self::map::GetAll;
-pub use self::map::HeaderMap;
-pub use self::utils::*;
+pub use self::map::{GetAll, HeaderMap, Removed};
+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.
 pub trait Header: IntoHeaderValue {
@@ -68,7 +66,7 @@ impl From<http::HeaderMap> for HeaderMap {
 }
 
 /// This encode set is used for HTTP header values and is defined at
-/// https://tools.ietf.org/html/rfc5987#section-3.2.
+/// <https://tools.ietf.org/html/rfc5987#section-3.2>.
 pub(crate) const HTTP_VALUE: &AsciiSet = &CONTROLS
     .add(b' ')
     .add(b'"')

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

@@ -88,7 +88,7 @@ impl Charset {
             Iso_8859_8_E => "ISO-8859-8-E",
             Iso_8859_8_I => "ISO-8859-8-I",
             Gb2312 => "GB2312",
-            Big5 => "big5",
+            Big5 => "Big5",
             Koi8_R => "KOI8-R",
             Ext(ref s) => s,
         }
@@ -128,7 +128,7 @@ impl FromStr for Charset {
             "ISO-8859-8-E" => Iso_8859_8_E,
             "ISO-8859-8-I" => Iso_8859_8_I,
             "GB2312" => Gb2312,
-            "big5" => Big5,
+            "BIG5" => Big5,
             "KOI8-R" => Koi8_R,
             s => Ext(s.to_owned()),
         })

actix-http/src/header/utils.rs

@@ -56,6 +56,7 @@ where
 
 /// Percent encode a sequence of bytes with a character set defined in
 /// <https://tools.ietf.org/html/rfc5987#section-3.2>
+#[inline]
 pub fn http_percent_encode(f: &mut fmt::Formatter<'_>, bytes: &[u8]) -> fmt::Result {
     let encoded = percent_encoding::percent_encode(bytes, HTTP_VALUE);
     fmt::Display::fmt(&encoded, f)

actix-http/src/ws/proto.rs

@@ -220,7 +220,7 @@ impl<T: Into<String>> From<(CloseCode, T)> for CloseReason {
     }
 }
 
-/// The WebSocket GUID as stated in the spec. See https://tools.ietf.org/html/rfc6455#section-1.3.
+/// The WebSocket GUID as stated in the spec. See <https://tools.ietf.org/html/rfc6455#section-1.3>.
 static WS_GUID: &[u8] = b"258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
 
 /// Hashes the `Sec-WebSocket-Key` header according to the WebSocket spec.

actix-multipart/Cargo.toml

@@ -20,7 +20,6 @@ actix-utils = "3.0.0"
 bytes = "1"
 derive_more = "0.99.5"
 futures-core = { version = "0.3.7", default-features = false, features = ["alloc"] }
-futures-util = { version = "0.3.7", default-features = false, features = ["alloc"] }
 httparse = "1.3"
 local-waker = "0.1"
 log = "0.4"
@@ -30,5 +29,6 @@ twoway = "0.2"
 [dev-dependencies]
 actix-rt = "2.2"
 actix-http = "3.0.0-beta.13"
+futures-util = { version = "0.3.7", default-features = false, features = ["alloc"] }
 tokio = { version = "1", features = ["sync"] }
 tokio-stream = "0.1"

actix-multipart/src/server.rs

@@ -17,7 +17,6 @@ use actix_web::{
 };
 use bytes::{Bytes, BytesMut};
 use futures_core::stream::{LocalBoxStream, Stream};
-use futures_util::stream::StreamExt as _;
 use local_waker::LocalWaker;
 
 use crate::error::MultipartError;
@@ -33,7 +32,7 @@ const MAX_HEADERS: usize = 32;
 pub struct Multipart {
     safety: Safety,
     error: Option<MultipartError>,
-    inner: Option<Rc<RefCell<InnerMultipart>>>,
+    inner: Option<InnerMultipart>,
 }
 
 enum InnerMultipartItem {
@@ -67,7 +66,7 @@ impl Multipart {
     /// Create multipart instance for boundary.
     pub fn new<S>(headers: &HeaderMap, stream: S) -> Multipart
     where
-        S: Stream<Item = Result<Bytes, PayloadError>> + Unpin + 'static,
+        S: Stream<Item = Result<Bytes, PayloadError>> + 'static,
     {
         match Self::boundary(headers) {
             Ok(boundary) => Multipart::from_boundary(boundary, stream),
@@ -77,39 +76,32 @@ impl Multipart {
 
     /// Extract boundary info from headers.
     pub(crate) fn boundary(headers: &HeaderMap) -> Result<String, MultipartError> {
-        if let Some(content_type) = headers.get(&header::CONTENT_TYPE) {
-            if let Ok(content_type) = content_type.to_str() {
-                if let Ok(ct) = content_type.parse::<mime::Mime>() {
-                    if let Some(boundary) = ct.get_param(mime::BOUNDARY) {
-                        Ok(boundary.as_str().to_owned())
-                    } else {
-                        Err(MultipartError::Boundary)
-                    }
-                } else {
-                    Err(MultipartError::ParseContentType)
-                }
-            } else {
-                Err(MultipartError::ParseContentType)
-            }
-        } else {
-            Err(MultipartError::NoContentType)
-        }
+        headers
+            .get(&header::CONTENT_TYPE)
+            .ok_or(MultipartError::NoContentType)?
+            .to_str()
+            .ok()
+            .and_then(|content_type| content_type.parse::<mime::Mime>().ok())
+            .ok_or(MultipartError::ParseContentType)?
+            .get_param(mime::BOUNDARY)
+            .map(|boundary| boundary.as_str().to_owned())
+            .ok_or(MultipartError::Boundary)
     }
 
     /// Create multipart instance for given boundary and stream
     pub(crate) fn from_boundary<S>(boundary: String, stream: S) -> Multipart
     where
-        S: Stream<Item = Result<Bytes, PayloadError>> + Unpin + 'static,
+        S: Stream<Item = Result<Bytes, PayloadError>> + 'static,
     {
         Multipart {
             error: None,
             safety: Safety::new(),
-            inner: Some(Rc::new(RefCell::new(InnerMultipart {
+            inner: Some(InnerMultipart {
                 boundary,
-                payload: PayloadRef::new(PayloadBuffer::new(Box::new(stream))),
+                payload: PayloadRef::new(PayloadBuffer::new(stream)),
                 state: InnerState::FirstBoundary,
                 item: InnerMultipartItem::None,
-            }))),
+            }),
         }
     }
 
@@ -126,20 +118,27 @@ impl Multipart {
 impl Stream for Multipart {
     type Item = Result<Field, MultipartError>;
 
-    fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
-        if let Some(err) = self.error.take() {
-            Poll::Ready(Some(Err(err)))
-        } else if self.safety.current() {
-            let this = self.get_mut();
-            let mut inner = this.inner.as_mut().unwrap().borrow_mut();
-            if let Some(mut payload) = inner.payload.get_mut(&this.safety) {
-                payload.poll_stream(cx)?;
+    fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
+        let this = self.get_mut();
+
+        match this.inner.as_mut() {
+            Some(inner) => {
+                if let Some(mut buffer) = inner.payload.get_mut(&this.safety) {
+                    // check safety and poll read payload to buffer.
+                    buffer.poll_stream(cx)?;
+                } else if !this.safety.is_clean() {
+                    // safety violation
+                    return Poll::Ready(Some(Err(MultipartError::NotConsumed)));
+                } else {
+                    return Poll::Pending;
+                }
+
+                inner.poll(&this.safety, cx)
             }
-            inner.poll(&this.safety, cx)
-        } else if !self.safety.is_clean() {
-            Poll::Ready(Some(Err(MultipartError::NotConsumed)))
-        } else {
-            Poll::Pending
+            None => Poll::Ready(Some(Err(this
+                .error
+                .take()
+                .expect("Multipart polled after finish")))),
         }
     }
 }
@@ -160,17 +159,15 @@ impl InnerMultipart {
                     Ok(httparse::Status::Complete((_, hdrs))) => {
                         // convert headers
                         let mut headers = HeaderMap::with_capacity(hdrs.len());
+
                         for h in hdrs {
-                            if let Ok(name) = HeaderName::try_from(h.name) {
-                                if let Ok(value) = HeaderValue::try_from(h.value) {
-                                    headers.append(name, value);
-                                } else {
-                                    return Err(ParseError::Header.into());
-                                }
-                            } else {
-                                return Err(ParseError::Header.into());
-                            }
+                            let name =
+                                HeaderName::try_from(h.name).map_err(|_| ParseError::Header)?;
+                            let value = HeaderValue::try_from(h.value)
+                                .map_err(|_| ParseError::Header)?;
+                            headers.append(name, value);
                         }
+
                         Ok(Some(headers))
                     }
                     Ok(httparse::Status::Partial) => Err(ParseError::Header.into()),
@@ -466,17 +463,19 @@ impl Stream for Field {
     type Item = Result<Bytes, MultipartError>;
 
     fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
-        if self.safety.current() {
-            let mut inner = self.inner.borrow_mut();
-            if let Some(mut payload) = inner.payload.as_ref().unwrap().get_mut(&self.safety) {
-                payload.poll_stream(cx)?;
-            }
-            inner.poll(&self.safety)
-        } else if !self.safety.is_clean() {
-            Poll::Ready(Some(Err(MultipartError::NotConsumed)))
+        let this = self.get_mut();
+        let mut inner = this.inner.borrow_mut();
+        if let Some(mut buffer) = inner.payload.as_ref().unwrap().get_mut(&this.safety) {
+            // check safety and poll read payload to buffer.
+            buffer.poll_stream(cx)?;
+        } else if !this.safety.is_clean() {
+            // safety violation
+            return Poll::Ready(Some(Err(MultipartError::NotConsumed)));
         } else {
-            Poll::Pending
+            return Poll::Pending;
         }
+
+        inner.poll(&this.safety)
     }
 }
 
@@ -690,10 +689,7 @@ impl PayloadRef {
         }
     }
 
-    fn get_mut<'a, 'b>(&'a self, s: &'b Safety) -> Option<RefMut<'a, PayloadBuffer>>
-    where
-        'a: 'b,
-    {
+    fn get_mut(&self, s: &Safety) -> Option<RefMut<'_, PayloadBuffer>> {
         if s.current() {
             Some(self.payload.borrow_mut())
         } else {
@@ -779,7 +775,7 @@ impl PayloadBuffer {
         PayloadBuffer {
             eof: false,
             buf: BytesMut::new(),
-            stream: stream.boxed_local(),
+            stream: Box::pin(stream),
         }
     }
 
@@ -860,7 +856,7 @@ mod tests {
     use actix_web::test::TestRequest;
     use actix_web::FromRequest;
     use bytes::Bytes;
-    use futures_util::future::lazy;
+    use futures_util::{future::lazy, StreamExt};
     use tokio::sync::mpsc;
     use tokio_stream::wrappers::UnboundedReceiverStream;
 

actix-router/src/resource.rs

@@ -1770,6 +1770,12 @@ mod tests {
         match_methods_agree!(["/v{v}", "/ver/{v}"] => "", "s/v", "/v1", "/v1/xx", "/ver/i3/5", "/ver/1");
     }
 
+    #[test]
+    #[should_panic]
+    fn duplicate_segment_name() {
+        ResourceDef::new("/user/{id}/post/{id}");
+    }
+
     #[test]
     #[should_panic]
     fn invalid_dynamic_segment_delimiter() {

awc/CHANGES.md

@@ -4,6 +4,7 @@
 
 
 ## 3.0.0-beta.11 - 2021-11-22
+* No significant changes from `3.0.0-beta.10`.
 
 
 ## 3.0.0-beta.10 - 2021-11-15

awc/src/builder.rs

@@ -1,18 +1,16 @@
-use std::convert::TryFrom;
-use std::fmt;
-use std::net::IpAddr;
-use std::rc::Rc;
-use std::time::Duration;
+use std::{convert::TryFrom, fmt, net::IpAddr, rc::Rc, time::Duration};
 
 use actix_http::http::{self, header, Error as HttpError, HeaderMap, HeaderName, Uri};
 use actix_rt::net::{ActixStream, TcpStream};
 use actix_service::{boxed, Service};
 
-use crate::client::{Connector, ConnectorService, TcpConnect, TcpConnectError, TcpConnection};
-use crate::connect::DefaultConnector;
-use crate::error::SendRequestError;
-use crate::middleware::{NestTransform, Redirect, Transform};
-use crate::{Client, ClientConfig, ConnectRequest, ConnectResponse};
+use crate::{
+    client::{Connector, ConnectorService, TcpConnect, TcpConnectError, TcpConnection},
+    connect::DefaultConnector,
+    error::SendRequestError,
+    middleware::{NestTransform, Redirect, Transform},
+    Client, ClientConfig, ConnectRequest, ConnectResponse,
+};
 
 /// An HTTP Client builder
 ///