clear conn_data on HttpRequest drop (#2742)

* clear conn_data on HttpRequest drop fixes #2740 * update changelog * fix doc test
actix · Apr 23, 2022 · f2cacc4 · f2cacc4
1 parent 56b9c0d
commit f2cacc4
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 7 deletions.
diff --git a/actix-http/benches/uninit-headers.rs b/actix-http/benches/uninit-headers.rs
@@ -114,7 +114,7 @@ mod _original {
     use std::mem::MaybeUninit;
 
     pub fn parse_headers(src: &mut BytesMut) -> usize {
-        #![allow(clippy::uninit_assumed_init)]
+        #![allow(invalid_value, clippy::uninit_assumed_init)]
 
         let mut headers: [HeaderIndex; MAX_HEADERS] =
             unsafe { MaybeUninit::uninit().assume_init() };

diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md
@@ -1,9 +1,14 @@
 # Changelog
 
 ## Unreleased - 2021-xx-xx
+### Added
 - Add `ServiceRequest::extract` to make it easier to use extractors when writing middlewares. [#2647]
 
+### Fixed
+- Clear connection-level data on `HttpRequest` drop. [#2742]
+
 [#2647]: https://github.com/actix/actix-web/pull/2647
+[#2742]: https://github.com/actix/actix-web/pull/2742
 
 
 ## 4.0.1 - 2022-02-25

diff --git a/actix-web/src/request.rs b/actix-web/src/request.rs
@@ -381,12 +381,16 @@ impl Drop for HttpRequest {
                 inner.app_data.truncate(1);
 
                 // Inner is borrowed mut here and; get req data mutably to reduce borrow check. Also
-                // we know the req_data Rc will not have any cloned at this point to unwrap is okay.
+                // we know the req_data Rc will not have any clones at this point to unwrap is okay.
                 Rc::get_mut(&mut inner.extensions)
                     .unwrap()
                     .get_mut()
                     .clear();
 
+                // We can't use the same trick as req data because the conn_data is held by the
+                // dispatcher, too.
+                inner.conn_data = None;
+
                 // a re-borrow of pool is necessary here.
                 let req = Rc::clone(&self.inner);
                 self.app_state().pool().push(req);
@@ -761,10 +765,8 @@ mod tests {
         assert_eq!(body, Bytes::from_static(b"1"));
     }
 
-    // allow deprecated App::data
-    #[allow(deprecated)]
     #[actix_rt::test]
-    async fn test_extensions_dropped() {
+    async fn test_app_data_dropped() {
         struct Tracker {
             pub dropped: bool,
         }
@@ -780,7 +782,7 @@ mod tests {
         let tracker = Rc::new(RefCell::new(Tracker { dropped: false }));
         {
             let tracker2 = Rc::clone(&tracker);
-            let srv = init_service(App::new().data(10u32).service(web::resource("/").to(
+            let srv = init_service(App::new().service(web::resource("/").to(
                 move |req: HttpRequest| {
                     req.extensions_mut().insert(Foo {
                         tracker: Rc::clone(&tracker2),

diff --git a/actix-web/src/test/test_request.rs b/actix-web/src/test/test_request.rs
@@ -53,7 +53,7 @@ use crate::cookie::{Cookie, CookieJar};
 ///     assert_eq!(resp.status(), StatusCode::OK);
 ///
 ///     let req = test::TestRequest::default().to_http_request();
-///     let resp = index(req).await;
+///     let resp = handler(req).await;
 ///     assert_eq!(resp.status(), StatusCode::BAD_REQUEST);
 /// }
 /// ```