on_connect result isnt added to request extensions for http2 requests #1009

actix-http/CHANGES.md

@@ -1,5 +1,12 @@
 # Changes
 
+## [0.2.10] - 2019-09-xx
+
+### Fixed
+
+* on_connect result isn't added to request extensions for http2 requests #1009
+
+
 ## [0.2.9] - 2019-08-13
 
 ### Changed

actix-http/src/builder.rs

@@ -199,6 +199,7 @@ where
             self.client_disconnect,
         );
         H2Service::with_config(cfg, service.into_new_service())
+            .on_connect(self.on_connect)
     }
 
     /// Finish service configuration and create `HttpService` instance.

actix-http/src/h1/dispatcher.rs

@@ -502,7 +502,7 @@ where
                             let pl = self.codec.message_type();
                             req.head_mut().peer_addr = self.peer_addr;
 
-                            // on_connect data
+                            // set on_connect data
                             if let Some(ref on_connect) = self.on_connect {
                                 on_connect.set(&mut req.extensions_mut());
                             }

actix-http/src/h2/dispatcher.rs

@@ -23,6 +23,7 @@ use crate::cloneable::CloneableService;
 use crate::config::ServiceConfig;
 use crate::error::{DispatchError, Error, ParseError, PayloadError, ResponseError};
 use crate::helpers::DataFactory;
+use crate::httpmessage::HttpMessage;
 use crate::message::ResponseHead;
 use crate::payload::Payload;
 use crate::request::Request;
@@ -122,6 +123,12 @@ where
                     head.version = parts.version;
                     head.headers = parts.headers.into();
                     head.peer_addr = self.peer_addr;
+
+                    // set on_connect data
+                    if let Some(ref on_connect) = self.on_connect {
+                        on_connect.set(&mut req.extensions_mut());
+                    }
+
                     tokio_current_thread::spawn(ServiceResponse::<S::Future, B> {
                         state: ServiceResponseState::ServiceCall(
                             self.service.call(req),

actix-http/tests/test_server.rs

@@ -11,6 +11,7 @@ use futures::stream::{once, Stream};
 use regex::Regex;
 use tokio_timer::sleep;
 
+use actix_http::httpmessage::HttpMessage;
 use actix_http::{
     body, error, http, http::header, Error, HttpService, KeepAlive, Request, Response,
 };
@@ -602,3 +603,18 @@ fn test_h1_service_error() {
     let bytes = srv.load_body(response).unwrap();
     assert_eq!(bytes, Bytes::from_static(b"error"));
 }
+
+#[test]
+fn test_h1_on_connect() {
+    let mut srv = TestServer::new(|| {
+        HttpService::build()
+            .on_connect(|_| 10usize)
+            .h1(|req: Request| {
+                assert!(req.extensions().contains::<usize>());
+                future::ok::<_, ()>(Response::Ok().finish())
+            })
+    });
+
+    let response = srv.block_on(srv.get("/").send()).unwrap();
+    assert!(response.status().is_success());
+}

actix-http/tests/test_ssl_server.rs

@@ -1,9 +1,5 @@
 #![cfg(feature = "ssl")]
 use actix_codec::{AsyncRead, AsyncWrite};
-use actix_http::error::{ErrorBadRequest, PayloadError};
-use actix_http::http::header::{self, HeaderName, HeaderValue};
-use actix_http::http::{Method, StatusCode, Version};
-use actix_http::{body, Error, HttpService, Request, Response};
 use actix_http_test::TestServer;
 use actix_server::ssl::OpensslAcceptor;
 use actix_server_config::ServerConfig;
@@ -15,6 +11,12 @@ use futures::stream::{once, Stream};
 use openssl::ssl::{AlpnError, SslAcceptor, SslFiletype, SslMethod};
 use std::io::Result;
 
+use actix_http::error::{ErrorBadRequest, PayloadError};
+use actix_http::http::header::{self, HeaderName, HeaderValue};
+use actix_http::http::{Method, StatusCode, Version};
+use actix_http::httpmessage::HttpMessage;
+use actix_http::{body, Error, HttpService, Request, Response};
+
 fn load_body<S>(stream: S) -> impl Future<Item = BytesMut, Error = PayloadError>
 where
     S: Stream<Item = Bytes, Error = PayloadError>,
@@ -453,3 +455,26 @@ fn test_h2_service_error() {
     let bytes = srv.load_body(response).unwrap();
     assert!(bytes.is_empty());
 }
+
+#[test]
+fn test_h2_on_connect() {
+    let openssl = ssl_acceptor().unwrap();
+
+    let mut srv = TestServer::new(move || {
+        openssl
+            .clone()
+            .map_err(|e| println!("Openssl error: {}", e))
+            .and_then(
+                HttpService::build()
+                    .on_connect(|_| 10usize)
+                    .h2(|req: Request| {
+                        assert!(req.extensions().contains::<usize>());
+                        ok::<_, ()>(Response::Ok().finish())
+                    })
+                    .map_err(|_| ()),
+            )
+    });
+
+    let response = srv.block_on(srv.sget("/").send()).unwrap();
+    assert!(response.status().is_success());
+}

src/request.rs

@@ -515,7 +515,9 @@ mod tests {
             let tracker2 = Rc::clone(&tracker);
             let mut srv = init_service(App::new().data(10u32).service(
                 web::resource("/").to(move |req: HttpRequest| {
-                    req.extensions_mut().insert(Foo { tracker: Rc::clone(&tracker2) });
+                    req.extensions_mut().insert(Foo {
+                        tracker: Rc::clone(&tracker2),
+                    });
                     HttpResponse::Ok()
                 }),
             ));