Remove duplicate test and move test helper function out of actix-http-test

CHANGES.md

@@ -3,7 +3,6 @@
 ## Unreleased - 2021-xx-xx
 ### Added
 * `HttpServer::worker_max_blocking_threads` for setting block thread pool. [#2200]
-* Added `TestServer::get_negotiated_alpn_protocol` method.
 
 ### Changed
 * `ServiceResponse::error_response` now uses body type of `Body`. [#2201]
@@ -11,7 +10,7 @@
 * Update `language-tags` to `0.3`.
 * `ServiceResponse::take_body`. [#2201]
 * `ServiceResponse::map_body` closure receives and returns `B` instead of `ResponseBody<B>` types. [#2201]
-* Extends Rustls ALPN protocols instead of replacing them when creating Rustls based services
+* `HttpServer::{listen_rustls(), bind_rustls()}` now honor the ALPN protocols in the configuation parameter. [#2226]
 * `middleware::normalize` now will not try to normalize URIs with no valid path [#2246]
 
 ### Removed

actix-http-test/CHANGES.md

@@ -1,7 +1,6 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
-* Added `TestServer::get_negotiated_alpn_protocol` method.
 
 ## 3.0.0-beta.4 - 2021-04-02
 * Added `TestServer::client_headers` method. [#2097]

actix-http-test/Cargo.toml

@@ -27,7 +27,6 @@ default = []
 
 # openssl
 openssl = ["tls-openssl", "awc/openssl"]
-rustls = ["tls-rustls", "webpki"]
 
 [dependencies]
 actix-service = "2.0.0"
@@ -50,8 +49,6 @@ slab = "0.4"
 serde_urlencoded = "0.7"
 time = { version = "0.2.23", default-features = false, features = ["std"] }
 tls-openssl = { version = "0.10.9", package = "openssl", optional = true }
-tls-rustls = { version = "0.19", package = "rustls", optional = true }
-webpki = { version = "0.21.0", optional = true }
 
 [dev-dependencies]
 actix-web = { version = "4.0.0-beta.6", default-features = false, features = ["cookies"] }

actix-http-test/src/lib.rs

@@ -7,17 +7,8 @@
 #[cfg(feature = "openssl")]
 extern crate tls_openssl as openssl;
 
-#[cfg(feature = "rustls")]
-extern crate tls_rustls as rustls;
-
 use std::sync::mpsc;
 use std::{net, thread, time};
-#[cfg(feature = "rustls")]
-use {
-    rustls::Session,
-    std::{io::Write, net::TcpStream as StdTcpStream, sync::Arc},
-    webpki::DNSNameRef,
-};
 
 use actix_codec::{AsyncRead, AsyncWrite, Framed};
 use actix_rt::{net::TcpStream, System};
@@ -232,24 +223,6 @@ impl TestServer {
         self.client.request(method, path.as_ref())
     }
 
-    #[cfg(feature = "rustls")]
-    /// Get the negotiated ALPN protocol with the server
-    pub fn get_negotiated_alpn_protocol(&self, client_alpn_protocol: &[u8]) -> Option<Vec<u8>> {
-        let mut config = rustls::ClientConfig::new();
-        config.alpn_protocols.push(client_alpn_protocol.to_vec());
-        let mut sess = rustls::ClientSession::new(
-            &Arc::new(config),
-            DNSNameRef::try_from_ascii_str("localhost").unwrap(),
-        );
-        let mut sock = StdTcpStream::connect(self.addr).unwrap();
-        let mut stream = rustls::Stream::new(&mut sess, &mut sock);
-        // The handshake will fails because the client will not be able to verify the server
-        // certificate, but it doesn't matter here as we are just interested in the negotiated ALPN
-        // protocol
-        let _ = stream.flush();
-        sess.get_alpn_protocol().map(|proto| proto.to_vec())
-    }
-
     pub async fn load_body<S>(
         &mut self,
         mut response: ClientResponse<S>,

actix-http/CHANGES.md

@@ -19,7 +19,7 @@
 * Update `language-tags` to `0.3`.
 * Reduce the level from `error` to `debug` for the log line that is emitted when a `500 Internal Server Error` is built using `HttpResponse::from_error`. [#2201]
 * `ResponseBuilder::message_body` now returns a `Result`. [#2201]
-* Extends Rustls ALPN protocols instead of replacing them when creating Rustls based services
+* `HttpServer::{listen_rustls(), bind_rustls()}` now honor the ALPN protocols in the configuation parameter. [#2226]
 
 ### Removed
 * Stop re-exporting `http` crate's `HeaderMap` types in addition to ours. [#2171]

actix-http/Cargo.toml

@@ -82,7 +82,7 @@ trust-dns-resolver = { version = "0.20.0", optional = true }
 
 [dev-dependencies]
 actix-server = "2.0.0-beta.3"
-actix-http-test = { version = "3.0.0-beta.4", features = ["openssl", "rustls"] }
+actix-http-test = { version = "3.0.0-beta.4", features = ["openssl"] }
 actix-tls = { version = "3.0.0-beta.5", features = ["openssl"] }
 criterion = "0.3"
 env_logger = "0.8"
@@ -91,6 +91,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 tls-openssl = { version = "0.10", package = "openssl" }
 tls-rustls = { version = "0.19", package = "rustls" }
+webpki = { version = "0.21.0" }
 
 [[example]]
 name = "ws"

actix-http/src/h2/service.rs

@@ -171,7 +171,7 @@ mod rustls {
             Error = TlsError<io::Error, DispatchError>,
             InitError = S::InitError,
         > {
-            let mut protos = vec!["h2".to_string().into()];
+            let mut protos = vec![b"h2".to_vec()];
             protos.extend_from_slice(&config.alpn_protocols);
             config.set_protocols(&protos);
 

actix-http/src/service.rs

@@ -305,8 +305,7 @@ mod rustls {
             Error = TlsError<io::Error, DispatchError>,
             InitError = (),
         > {
-            let mut protos =
+            let mut protos = vec![b"h2".to_vec(), b"http/1.1".to_vec()];
-                vec!["h2".to_string().into(), "http/1.1".to_string().into()];
             protos.extend_from_slice(&config.alpn_protocols);
             config.set_protocols(&protos);
 

actix-http/tests/test_rustls.rs

@@ -20,10 +20,15 @@ use futures_core::Stream;
 use futures_util::stream::{once, StreamExt as _};
 use rustls::{
     internal::pemfile::{certs, pkcs8_private_keys},
-    NoClientAuth, ServerConfig as RustlsServerConfig,
+    NoClientAuth, ServerConfig as RustlsServerConfig, Session,
 };
+use webpki::DNSNameRef;
 
-use std::io::{self, BufReader};
+use std::{
+    io::{self, BufReader, Write},
+    net::{SocketAddr, TcpStream as StdTcpStream},
+    sync::Arc,
+};
 
 async fn load_body<S>(mut stream: S) -> Result<BytesMut, PayloadError>
 where
@@ -52,6 +57,25 @@ fn tls_config() -> RustlsServerConfig {
     config
 }
 
+pub fn get_negotiated_alpn_protocol(
+    addr: SocketAddr,
+    client_alpn_protocol: &[u8],
+) -> Option<Vec<u8>> {
+    let mut config = rustls::ClientConfig::new();
+    config.alpn_protocols.push(client_alpn_protocol.to_vec());
+    let mut sess = rustls::ClientSession::new(
+        &Arc::new(config),
+        DNSNameRef::try_from_ascii_str("localhost").unwrap(),
+    );
+    let mut sock = StdTcpStream::connect(addr).unwrap();
+    let mut stream = rustls::Stream::new(&mut sess, &mut sock);
+    // The handshake will fails because the client will not be able to verify the server
+    // certificate, but it doesn't matter here as we are just interested in the negotiated ALPN
+    // protocol
+    let _ = stream.flush();
+    sess.get_alpn_protocol().map(|proto| proto.to_vec())
+}
+
 #[actix_rt::test]
 async fn test_h1() -> io::Result<()> {
     let srv = test_server(move || {
@@ -477,7 +501,7 @@ async fn test_alpn_h1() -> io::Result<()> {
     .await;
 
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(CUSTOM_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), CUSTOM_ALPN_PROTOCOL),
         Some(CUSTOM_ALPN_PROTOCOL.to_vec())
     );
 
@@ -499,33 +523,11 @@ async fn test_alpn_h2() -> io::Result<()> {
     .await;
 
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(H2_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), H2_ALPN_PROTOCOL),
         Some(H2_ALPN_PROTOCOL.to_vec())
     );
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(CUSTOM_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), CUSTOM_ALPN_PROTOCOL),
-        Some(CUSTOM_ALPN_PROTOCOL.to_vec())
-    );
-
-    let response = srv.sget("/").send().await.unwrap();
-    assert!(response.status().is_success());
-
-    Ok(())
-}
-
-#[actix_rt::test]
-async fn test_alpn_h1_1() -> io::Result<()> {
-    let srv = test_server(move || {
-        let mut config = tls_config();
-        config.alpn_protocols.push(CUSTOM_ALPN_PROTOCOL.to_vec());
-        HttpService::build()
-            .h1(|_| ok::<_, Error>(Response::ok()))
-            .rustls(config)
-    })
-    .await;
-
-    assert_eq!(
-        srv.get_negotiated_alpn_protocol(CUSTOM_ALPN_PROTOCOL),
         Some(CUSTOM_ALPN_PROTOCOL.to_vec())
     );
 
@@ -547,15 +549,15 @@ async fn test_alpn_h2_1() -> io::Result<()> {
     .await;
 
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(H2_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), H2_ALPN_PROTOCOL),
         Some(H2_ALPN_PROTOCOL.to_vec())
     );
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(HTTP1_1_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), HTTP1_1_ALPN_PROTOCOL),
         Some(HTTP1_1_ALPN_PROTOCOL.to_vec())
     );
     assert_eq!(
-        srv.get_negotiated_alpn_protocol(CUSTOM_ALPN_PROTOCOL),
+        get_negotiated_alpn_protocol(srv.addr(), CUSTOM_ALPN_PROTOCOL),
         Some(CUSTOM_ALPN_PROTOCOL.to_vec())
     );