From f6ff056b8a496d1e629ecab16ba50c7f8e5094a3 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 10 Jan 2020 11:26:54 +0600 Subject: [PATCH 1/3] Fix panic with already borrowed: BorrowMutError #1263 --- actix-identity/CHANGES.md | 4 +++ actix-identity/Cargo.toml | 7 ++--- actix-identity/src/lib.rs | 60 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 65 insertions(+), 6 deletions(-) diff --git a/actix-identity/CHANGES.md b/actix-identity/CHANGES.md index 8e89acc15..594c21388 100644 --- a/actix-identity/CHANGES.md +++ b/actix-identity/CHANGES.md @@ -1,5 +1,9 @@ # Changes +## [0.2.1] - 2020-01-10 + +* Fix panic with already borrowed: BorrowMutError #1263 + ## [0.2.0] - 2019-12-20 * Use actix-web 2.0 diff --git a/actix-identity/Cargo.toml b/actix-identity/Cargo.toml index b30246f0b..8cd6b1271 100644 --- a/actix-identity/Cargo.toml +++ b/actix-identity/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-identity" -version = "0.2.0" +version = "0.2.1" authors = ["Nikolay Kim "] description = "Identity service for actix web framework." readme = "README.md" @@ -10,15 +10,14 @@ repository = "https://github.com/actix/actix-web.git" documentation = "https://docs.rs/actix-identity/" license = "MIT/Apache-2.0" edition = "2018" -workspace = ".." [lib] name = "actix_identity" path = "src/lib.rs" [dependencies] -actix-web = { version = "2.0.0-rc", default-features = false, features = ["secure-cookies"] } -actix-service = "1.0.1" +actix-web = { version = "2.0.0", default-features = false, features = ["secure-cookies"] } +actix-service = "1.0.2" futures = "0.3.1" serde = "1.0" serde_json = "1.0" diff --git a/actix-identity/src/lib.rs b/actix-identity/src/lib.rs index b10a419dd..3b9626991 100644 --- a/actix-identity/src/lib.rs +++ b/actix-identity/src/lib.rs @@ -251,6 +251,15 @@ pub struct IdentityServiceMiddleware { service: Rc>, } +impl Clone for IdentityServiceMiddleware { + fn clone(&self) -> Self { + Self { + backend: self.backend.clone(), + service: self.service.clone(), + } + } +} + impl Service for IdentityServiceMiddleware where B: 'static, @@ -279,7 +288,9 @@ where req.extensions_mut() .insert(IdentityItem { id, changed: false }); - let mut res = srv.borrow_mut().call(req).await?; + // https://github.com/actix/actix-web/issues/1263 + let fut = { srv.borrow_mut().call(req) }; + let mut res = fut.await?; let id = res.request().extensions_mut().remove::(); if let Some(id) = id { @@ -606,9 +617,10 @@ mod tests { use std::borrow::Borrow; use super::*; + use actix_service::into_service; use actix_web::http::StatusCode; use actix_web::test::{self, TestRequest}; - use actix_web::{web, App, Error, HttpResponse}; + use actix_web::{error, web, App, Error, HttpResponse}; const COOKIE_KEY_MASTER: [u8; 32] = [0; 32]; const COOKIE_NAME: &'static str = "actix_auth"; @@ -1045,6 +1057,7 @@ mod tests { assert_logged_in(resp, Some(COOKIE_LOGIN)).await; } + // https://github.com/actix/actix-web/issues/1263 #[actix_rt::test] async fn test_identity_cookie_updated_on_visit_deadline() { let mut srv = create_identity_server(|c| { @@ -1069,4 +1082,47 @@ mod tests { ); assert_logged_in(resp, Some(COOKIE_LOGIN)).await; } + + #[actix_rt::test] + async fn test_borrowed_mut_error() { + use futures::future::{lazy, ok, Ready}; + + struct Ident; + impl IdentityPolicy for Ident { + type Future = Ready, Error>>; + type ResponseFuture = Ready>; + + fn from_request(&self, _: &mut ServiceRequest) -> Self::Future { + ok(Some("test".to_string())) + } + + fn to_response( + &self, + _: Option, + _: bool, + _: &mut ServiceResponse, + ) -> Self::ResponseFuture { + ok(()) + } + } + + let mut srv = IdentityServiceMiddleware { + backend: Rc::new(Ident), + service: Rc::new(RefCell::new(into_service(|_: ServiceRequest| { + async move { + actix_rt::time::delay_for(std::time::Duration::from_secs(100)).await; + Err::(error::ErrorBadRequest("error")) + } + }))), + }; + + let mut srv2 = srv.clone(); + let req = TestRequest::default().to_srv_request(); + actix_rt::spawn(async move { + let _ = srv2.call(req).await; + }); + actix_rt::time::delay_for(std::time::Duration::from_millis(50)).await; + + let _ = lazy(|cx| srv.poll_ready(cx)).await; + } } From e66312b664f183a80eae6a9de3e9c8667b08241e Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 10 Jan 2020 11:36:59 +0600 Subject: [PATCH 2/3] add extra constraints --- Cargo.toml | 8 ++++---- actix-http/src/cloneable.rs | 16 +++++----------- actix-http/src/h1/service.rs | 2 +- actix-http/src/h2/service.rs | 2 +- actix-http/src/service.rs | 2 +- 5 files changed, 12 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1adae97b8..9e1b559eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,9 +60,9 @@ rustls = ["actix-tls/rustls", "awc/rustls", "rust-tls"] [dependencies] actix-codec = "0.2.0" -actix-service = "1.0.1" -actix-utils = "1.0.4" -actix-router = "0.2.1" +actix-service = "1.0.2" +actix-utils = "1.0.6" +actix-router = "0.2.4" actix-rt = "1.0.0" actix-server = "1.0.0" actix-testing = "1.0.0" @@ -115,4 +115,4 @@ actix-identity = { path = "actix-identity" } actix-session = { path = "actix-session" } actix-files = { path = "actix-files" } actix-multipart = { path = "actix-multipart" } -awc = { path = "awc" } \ No newline at end of file +awc = { path = "awc" } diff --git a/actix-http/src/cloneable.rs b/actix-http/src/cloneable.rs index 90d198b9c..65c6bec21 100644 --- a/actix-http/src/cloneable.rs +++ b/actix-http/src/cloneable.rs @@ -6,27 +6,21 @@ use actix_service::Service; #[doc(hidden)] /// Service that allows to turn non-clone service to a service with `Clone` impl -pub(crate) struct CloneableService(Rc>); +pub(crate) struct CloneableService(Rc>); -impl CloneableService { - pub(crate) fn new(service: T) -> Self - where - T: Service, - { +impl CloneableService { + pub(crate) fn new(service: T) -> Self { Self(Rc::new(UnsafeCell::new(service))) } } -impl Clone for CloneableService { +impl Clone for CloneableService { fn clone(&self) -> Self { Self(self.0.clone()) } } -impl Service for CloneableService -where - T: Service, -{ +impl Service for CloneableService { type Request = T::Request; type Response = T::Response; type Error = T::Error; diff --git a/actix-http/src/h1/service.rs b/actix-http/src/h1/service.rs index 69c8fc55c..4d1a1dc1b 100644 --- a/actix-http/src/h1/service.rs +++ b/actix-http/src/h1/service.rs @@ -364,7 +364,7 @@ where } /// `Service` implementation for HTTP1 transport -pub struct H1ServiceHandler { +pub struct H1ServiceHandler { srv: CloneableService, expect: CloneableService, upgrade: Option>, diff --git a/actix-http/src/h2/service.rs b/actix-http/src/h2/service.rs index 7cae99f5b..ff3f69faf 100644 --- a/actix-http/src/h2/service.rs +++ b/actix-http/src/h2/service.rs @@ -246,7 +246,7 @@ where } /// `Service` implementation for http/2 transport -pub struct H2ServiceHandler { +pub struct H2ServiceHandler { srv: CloneableService, cfg: ServiceConfig, on_connect: Option Box>>, diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 82618289b..51de95135 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -443,7 +443,7 @@ where } /// `Service` implementation for http transport -pub struct HttpServiceHandler { +pub struct HttpServiceHandler { srv: CloneableService, expect: CloneableService, upgrade: Option>, From abb462ef8540b170d518fea968a3adb13d809ddc Mon Sep 17 00:00:00 2001 From: linkmauve Date: Fri, 10 Jan 2020 18:34:31 +0100 Subject: [PATCH 3/3] Replace sha1 dependency with sha-1 (#1258) * Replace sha1 dependency with sha-1 This other crate is being maintained, and it offers better performances when using the `asm` feature (especially [on AArch64](https://github.com/RustCrypto/hashes/pull/97)). * Update CHANGES.md with the sha-1 migration * Add a test for hash_key() --- CHANGES.md | 6 ++++++ actix-http/Cargo.toml | 2 +- actix-http/src/ws/proto.rs | 13 ++++++++++--- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 2f96dce5d..29f78e0b1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Changes +## [2.0.NEXT] - 2020-01-xx + +### Changed + +* Use `sha-1` crate instead of unmaintained `sha1` crate + ## [2.0.0] - 2019-12-25 ### Changed diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 367dbafec..93aaa756e 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -74,7 +74,7 @@ rand = "0.7" regex = "1.3" serde = "1.0" serde_json = "1.0" -sha1 = "0.6" +sha-1 = "0.8" slab = "0.4" serde_urlencoded = "0.6.1" time = "0.1.42" diff --git a/actix-http/src/ws/proto.rs b/actix-http/src/ws/proto.rs index ad42b7a6b..60af6f08b 100644 --- a/actix-http/src/ws/proto.rs +++ b/actix-http/src/ws/proto.rs @@ -207,12 +207,13 @@ static WS_GUID: &str = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"; // TODO: hash is always same size, we dont need String pub fn hash_key(key: &[u8]) -> String { + use sha1::Digest; let mut hasher = sha1::Sha1::new(); - hasher.update(key); - hasher.update(WS_GUID.as_bytes()); + hasher.input(key); + hasher.input(WS_GUID.as_bytes()); - base64::encode(&hasher.digest().bytes()) + base64::encode(hasher.result().as_ref()) } #[cfg(test)] @@ -277,6 +278,12 @@ mod test { assert_eq!(format!("{}", OpCode::Bad), "BAD"); } + #[test] + fn test_hash_key() { + let hash = hash_key(b"hello actix-web"); + assert_eq!(&hash, "cR1dlyUUJKp0s/Bel25u5TgvC3E="); + } + #[test] fn closecode_from_u16() { assert_eq!(CloseCode::from(1000u16), CloseCode::Normal);