Merge branch 'master' into robjtede/issue2502

fixes #2502

CHANGES.md

@@ -15,7 +15,7 @@
 * Rename `test::{default_service => simple_service}`. Old name is deprecated. [#2518]
 * Rename `test::{read_response_json => call_and_read_body_json}`. Old name is deprecated. [#2518]
 * Rename `test::{read_response => call_and_read_body}`. Old name is deprecated. [#2518]
-* Relax body type and error bounds on test utilities.
+* Relax body type and error bounds on test utilities. [#2518]
 
 ### Removed
 * Top-level `EitherExtractError` export. [#2510]

actix-http-test/src/lib.rs

@@ -107,7 +107,7 @@ pub async fn test_server_with_addr<F: ServiceFactory<TcpStream>>(
             Connector::new()
                 .conn_lifetime(Duration::from_secs(0))
                 .timeout(Duration::from_millis(30000))
-                .ssl(builder.build())
+                .openssl(builder.build())
         };
 
         #[cfg(not(feature = "openssl"))]

actix-http/CHANGES.md

@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+### Removed
+* `header::map::GetAll` iterator, its `Iterator::size_hint` method was wrongly implemented. Replaced with `std::slice::Iter`. [#2527]
+
+[#2527]: https://github.com/actix/actix-web/pull/2527
 
 
 ## 3.0.0-beta.16 - 2021-12-17

actix-http/src/header/map.rs

@@ -306,8 +306,11 @@ impl HeaderMap {
     /// assert_eq!(set_cookies_iter.next().unwrap(), "two=2");
     /// assert!(set_cookies_iter.next().is_none());
     /// ```
-    pub fn get_all(&self, key: impl AsHeaderName) -> GetAll<'_> {
+    pub fn get_all(&self, key: impl AsHeaderName) -> std::slice::Iter<'_, HeaderValue> {
-        GetAll::new(self.get_value(key))
+        match self.get_value(key) {
+            Some(value) => value.iter(),
+            None => (&[]).iter(),
+        }
     }
 
     // TODO: get_all_mut ?
@@ -602,52 +605,6 @@ impl<'a> IntoIterator for &'a HeaderMap {
     }
 }
 
-/// Iterator over borrowed values with the same associated name.
-///
-/// See [`HeaderMap::get_all`].
-#[derive(Debug)]
-pub struct GetAll<'a> {
-    idx: usize,
-    value: Option<&'a Value>,
-}
-
-impl<'a> GetAll<'a> {
-    fn new(value: Option<&'a Value>) -> Self {
-        Self { idx: 0, value }
-    }
-}
-
-impl<'a> Iterator for GetAll<'a> {
-    type Item = &'a HeaderValue;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let val = self.value?;
-
-        match val.get(self.idx) {
-            Some(val) => {
-                self.idx += 1;
-                Some(val)
-            }
-            None => {
-                // current index is none; remove value to fast-path future next calls
-                self.value = None;
-                None
-            }
-        }
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        match self.value {
-            Some(val) => (val.len(), Some(val.len())),
-            None => (0, Some(0)),
-        }
-    }
-}
-
-impl ExactSizeIterator for GetAll<'_> {}
-
-impl iter::FusedIterator for GetAll<'_> {}
-
 /// Iterator over removed, owned values with the same associated name.
 ///
 /// Returned from methods that remove or replace items. See [`HeaderMap::insert`]
@@ -895,7 +852,7 @@ mod tests {
 
     assert_impl_all!(HeaderMap: IntoIterator);
     assert_impl_all!(Keys<'_>: Iterator, ExactSizeIterator, FusedIterator);
-    assert_impl_all!(GetAll<'_>: Iterator, ExactSizeIterator, FusedIterator);
+    assert_impl_all!(std::slice::Iter<'_, HeaderValue>: Iterator, ExactSizeIterator, FusedIterator);
     assert_impl_all!(Removed: Iterator, ExactSizeIterator, FusedIterator);
     assert_impl_all!(Iter<'_>: Iterator, ExactSizeIterator, FusedIterator);
     assert_impl_all!(IntoIter: Iterator, ExactSizeIterator, FusedIterator);

actix-test/src/lib.rs

@@ -340,7 +340,7 @@ where
                 Connector::new()
                     .conn_lifetime(Duration::from_secs(0))
                     .timeout(Duration::from_millis(30000))
-                    .ssl(builder.build())
+                    .openssl(builder.build())
             }
             #[cfg(not(feature = "openssl"))]
             {

awc/CHANGES.md

@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+* Rename `Connector::{ssl => openssl}`. [#2503]
+* Improve `Client` instantiation efficiency when using `openssl` by only building connectors once. [#2503]
+
+[#2503]: https://github.com/actix/actix-web/pull/2503
 
 
 ## 3.0.0-beta.14 - 2021-12-17

awc/Cargo.toml

@@ -62,7 +62,7 @@ actix-codec = "0.4.1"
 actix-service = "2.0.0"
 actix-http = "3.0.0-beta.16"
 actix-rt = { version = "2.1", default-features = false }
-actix-tls = { version = "3.0.0-rc.1", features = ["connect", "uri"] }
+actix-tls = { version = "3.0.0-rc.2", features = ["connect", "uri"] }
 actix-utils = "3.0.0"
 
 ahash = "0.7"

awc/src/client/connector.rs

@@ -22,11 +22,13 @@ use futures_core::{future::LocalBoxFuture, ready};
 use http::Uri;
 use pin_project_lite::pin_project;
 
-use super::config::ConnectorConfig;
+use super::{
-use super::connection::{Connection, ConnectionIo};
+    config::ConnectorConfig,
-use super::error::ConnectError;
+    connection::{Connection, ConnectionIo},
-use super::pool::ConnectionPool;
+    error::ConnectError,
-use super::Connect;
+    pool::ConnectionPool,
+    Connect,
+};
 
 enum OurTlsConnector {
     #[allow(dead_code)] // only dead when no TLS feature is enabled
@@ -35,6 +37,12 @@ enum OurTlsConnector {
     #[cfg(feature = "openssl")]
     Openssl(actix_tls::connect::openssl::reexports::SslConnector),
 
+    /// Provided because building the OpenSSL context on newer versions can be very slow.
+    /// This prevents unnecessary calls to `.build()` while constructing the client connector.
+    #[cfg(feature = "openssl")]
+    #[allow(dead_code)] // false positive; used in build_ssl
+    OpensslBuilder(actix_tls::connect::openssl::reexports::SslConnectorBuilder),
+
     #[cfg(feature = "rustls")]
     Rustls(std::sync::Arc<actix_tls::connect::rustls::reexports::ClientConfig>),
 }
@@ -57,7 +65,7 @@ pub struct Connector<T> {
     config: ConnectorConfig,
 
     #[allow(dead_code)] // only dead when no TLS feature is enabled
-    ssl: OurTlsConnector,
+    tls: OurTlsConnector,
 }
 
 impl Connector<()> {
@@ -72,7 +80,7 @@ impl Connector<()> {
         Connector {
             connector: TcpConnector::new(resolver::resolver()).service(),
             config: ConnectorConfig::default(),
-            ssl: Self::build_ssl(vec![b"h2".to_vec(), b"http/1.1".to_vec()]),
+            tls: Self::build_ssl(vec![b"h2".to_vec(), b"http/1.1".to_vec()]),
         }
     }
 
@@ -116,7 +124,7 @@ impl Connector<()> {
             log::error!("Can not set ALPN protocol: {:?}", err);
         }
 
-        OurTlsConnector::Openssl(ssl.build())
+        OurTlsConnector::OpensslBuilder(ssl)
     }
 }
 
@@ -134,7 +142,7 @@ impl<S> Connector<S> {
         Connector {
             connector,
             config: self.config,
-            ssl: self.ssl,
+            tls: self.tls,
         }
     }
 }
@@ -167,23 +175,35 @@ where
         self
     }
 
+    /// Use custom OpenSSL `SslConnector` instance.
     #[cfg(feature = "openssl")]
-    /// Use custom `SslConnector` instance.
+    pub fn openssl(
+        mut self,
+        connector: actix_tls::connect::openssl::reexports::SslConnector,
+    ) -> Self {
+        self.tls = OurTlsConnector::Openssl(connector);
+        self
+    }
+
+    /// See docs for [`Connector::openssl`].
+    #[doc(hidden)]
+    #[cfg(feature = "openssl")]
+    #[deprecated(since = "3.0.0", note = "Renamed to `Connector::openssl`.")]
     pub fn ssl(
         mut self,
         connector: actix_tls::connect::openssl::reexports::SslConnector,
     ) -> Self {
-        self.ssl = OurTlsConnector::Openssl(connector);
+        self.tls = OurTlsConnector::Openssl(connector);
         self
     }
 
+    /// Use custom Rustls `ClientConfig` instance.
     #[cfg(feature = "rustls")]
-    /// Use custom `ClientConfig` instance.
     pub fn rustls(
         mut self,
         connector: std::sync::Arc<actix_tls::connect::rustls::reexports::ClientConfig>,
     ) -> Self {
-        self.ssl = OurTlsConnector::Rustls(connector);
+        self.tls = OurTlsConnector::Rustls(connector);
         self
     }
 
@@ -198,7 +218,7 @@ where
                 unimplemented!("actix-http client only supports versions http/1.1 & http/2")
             }
         };
-        self.ssl = Connector::build_ssl(versions);
+        self.tls = Connector::build_ssl(versions);
         self
     }
 
@@ -270,8 +290,8 @@ where
     }
 
     /// Finish configuration process and create connector service.
-    /// The Connector builder always concludes by calling `finish()` last in
+    ///
-    /// its combinator chain.
+    /// The `Connector` builder always concludes by calling `finish()` last in its combinator chain.
     pub fn finish(self) -> ConnectorService<S, IO> {
         let local_address = self.config.local_address;
         let timeout = self.config.timeout;
@@ -284,7 +304,15 @@ where
             service: tcp_service_inner.clone(),
         };
 
-        let tls_service = match self.ssl {
+        let tls = match self.tls {
+            #[cfg(feature = "openssl")]
+            OurTlsConnector::OpensslBuilder(builder) => {
+                OurTlsConnector::Openssl(builder.build())
+            }
+            tls => tls,
+        };
+
+        let tls_service = match tls {
             OurTlsConnector::None => {
                 #[cfg(not(feature = "dangerous-h2c"))]
                 {
@@ -374,6 +402,11 @@ where
                 Some(actix_service::boxed::rc_service(tls_service))
             }
 
+            #[cfg(feature = "openssl")]
+            OurTlsConnector::OpensslBuilder(_) => {
+                unreachable!("OpenSSL builder is built before this match.");
+            }
+
             #[cfg(feature = "rustls")]
             OurTlsConnector::Rustls(tls) => {
                 const H2: &[u8] = b"h2";
@@ -853,7 +886,7 @@ mod tests {
         let connector = Connector {
             connector: TcpConnector::new(resolver::resolver()).service(),
             config: ConnectorConfig::default(),
-            ssl: OurTlsConnector::None,
+            tls: OurTlsConnector::None,
         };
 
         let client = Client::builder().connector(connector).finish();

awc/tests/test_connector.rs

@@ -58,7 +58,7 @@ async fn test_connection_window_size() {
         .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e));
 
     let client = awc::Client::builder()
-        .connector(awc::Connector::new().ssl(builder.build()))
+        .connector(awc::Connector::new().openssl(builder.build()))
         .initial_window_size(100)
         .initial_connection_window_size(100)
         .finish();

awc/tests/test_ssl_client.rs

@@ -72,7 +72,7 @@ async fn test_connection_reuse_h2() {
         .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e));
 
     let client = awc::Client::builder()
-        .connector(awc::Connector::new().ssl(builder.build()))
+        .connector(awc::Connector::new().openssl(builder.build()))
         .finish();
 
     // req 1

scripts/bump

@@ -55,6 +55,11 @@ else
     read -p "Update version to: " NEW_VERSION
 fi
 
+# strip leading v from input
+if [ "${NEW_VERSION:0:1}" = "v" ]; then
+    NEW_VERSION="${NEW_VERSION:1}"
+fi
+
 DATE="$(date -u +"%Y-%m-%d")"
 echo "updating from $CURRENT_VERSION => $NEW_VERSION ($DATE)"
 
@@ -124,15 +129,20 @@ SHORT_PACKAGE_NAME="$(echo $PACKAGE_NAME | sed 's/^actix-web-//' | sed 's/^actix
 GIT_TAG="$(echo $SHORT_PACKAGE_NAME-v$NEW_VERSION)"
 RELEASE_TITLE="$(echo $PACKAGE_NAME: v$NEW_VERSION)"
 
+if [ "$(echo $NEW_VERSION | grep beta)" ] || [ "$(echo $NEW_VERSION | grep rc)" ] || [ "$(echo $NEW_VERSION | grep alpha)" ]; then
+    PRERELEASE="--prerelease"
+fi
+
 echo
 echo "GitHub release command:"
-echo "gh release create \"$GIT_TAG\" --draft --title \"$RELEASE_TITLE\" --notes-file \"$CHANGE_CHUNK_FILE\" --prerelease"
+GH_CMD="gh release create \"$GIT_TAG\" --draft --title \"$RELEASE_TITLE\" --notes-file \"$CHANGE_CHUNK_FILE\" ${PRERELEASE:-}"
+echo "$GH_CMD"
 
 read -p "Submit draft GH release: (y/N) " GH_RELEASE
 GH_RELEASE="${GH_RELEASE:-n}"
 
 if [ "$GH_RELEASE" = 'y' ] || [ "$GH_RELEASE" = 'Y' ]; then
-    gh release create "$GIT_TAG" --draft --title "$RELEASE_TITLE" --notes-file "$CHANGE_CHUNK_FILE" --prerelease
+    eval "$GH_CMD"
 fi
 
 echo

src/app.rs

@@ -486,19 +486,21 @@ where
 
 #[cfg(test)]
 mod tests {
-    use actix_service::Service;
+    use actix_service::Service as _;
     use actix_utils::future::{err, ok};
     use bytes::Bytes;
 
     use super::*;
-    use crate::http::{
+    use crate::{
-        header::{self, HeaderValue},
+        http::{
-        Method, StatusCode,
+            header::{self, HeaderValue},
+            Method, StatusCode,
+        },
+        middleware::DefaultHeaders,
+        service::ServiceRequest,
+        test::{call_service, init_service, read_body, try_init_service, TestRequest},
+        web, HttpRequest, HttpResponse,
     };
-    use crate::middleware::DefaultHeaders;
-    use crate::service::ServiceRequest;
-    use crate::test::{call_service, init_service, read_body, try_init_service, TestRequest};
-    use crate::{web, HttpRequest, HttpResponse};
 
     #[actix_rt::test]
     async fn test_default_resource() {

src/dev.rs

@@ -102,49 +102,3 @@ impl<B> BodyEncoding for crate::HttpResponse<B> {
         self
     }
 }
-
-// TODO: remove this if it doesn't appear to be needed
-
-#[allow(dead_code)]
-#[derive(Debug)]
-pub(crate) enum AnyBody {
-    None,
-    Full { body: crate::web::Bytes },
-    Boxed { body: actix_http::body::BoxBody },
-}
-
-impl crate::body::MessageBody for AnyBody {
-    type Error = crate::BoxError;
-
-    /// Body size hint.
-    fn size(&self) -> crate::body::BodySize {
-        match self {
-            AnyBody::None => crate::body::BodySize::None,
-            AnyBody::Full { body } => body.size(),
-            AnyBody::Boxed { body } => body.size(),
-        }
-    }
-
-    /// Attempt to pull out the next chunk of body bytes.
-    fn poll_next(
-        self: std::pin::Pin<&mut Self>,
-        cx: &mut std::task::Context<'_>,
-    ) -> std::task::Poll<Option<Result<crate::web::Bytes, Self::Error>>> {
-        match self.get_mut() {
-            AnyBody::None => std::task::Poll::Ready(None),
-            AnyBody::Full { body } => {
-                let bytes = std::mem::take(body);
-                std::task::Poll::Ready(Some(Ok(bytes)))
-            }
-            AnyBody::Boxed { body } => body.as_pin_mut().poll_next(cx),
-        }
-    }
-
-    fn try_into_bytes(self) -> Result<crate::web::Bytes, Self> {
-        match self {
-            AnyBody::None => Ok(crate::web::Bytes::new()),
-            AnyBody::Full { body } => Ok(body),
-            _ => Err(self),
-        }
-    }
-}

src/response/response.rs

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

tests/test_httpserver.rs

@@ -121,7 +121,7 @@ async fn test_start_ssl() {
     let client = awc::Client::builder()
         .connector(
             awc::Connector::new()
-                .ssl(builder.build())
+                .openssl(builder.build())
                 .timeout(Duration::from_millis(100)),
         )
         .finish();