diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index fed4ce031..36b224ba6 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -27,26 +27,6 @@ jobs: profile: minimal override: true - - name: Generate Cargo.lock - uses: actions-rs/cargo@v1 - with: - command: generate-lockfile - - name: Cache cargo registry - uses: actions/cache@v1 - with: - path: ~/.cargo/registry - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-registry-trimmed-${{ hashFiles('**/Cargo.lock') }} - - name: Cache cargo index - uses: actions/cache@v1 - with: - path: ~/.cargo/git - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-index-trimmed-${{ hashFiles('**/Cargo.lock') }} - - name: Cache cargo build - uses: actions/cache@v1 - with: - path: target - key: ${{ matrix.version }}-x86_64-pc-windows-msvc-cargo-build-trimmed-${{ hashFiles('**/Cargo.lock') }} - - name: Install OpenSSL run: | vcpkg integrate install @@ -74,8 +54,5 @@ jobs: --skip=test_expect_continue --skip=test_http10_keepalive --skip=test_slow_request - - - name: Clear the cargo caches - run: | - cargo install cargo-cache --no-default-features --features ci-autoclean - cargo-cache + --skip=test_connection_force_close + --skip=test_connection_server_close diff --git a/actix-http/src/config.rs b/actix-http/src/config.rs index 50322bf20..a38a80e76 100644 --- a/actix-http/src/config.rs +++ b/actix-http/src/config.rs @@ -1,4 +1,4 @@ -use std::cell::UnsafeCell; +use std::cell::Cell; use std::fmt::Write; use std::rc::Rc; use std::time::Duration; @@ -228,24 +228,24 @@ impl fmt::Write for Date { struct DateService(Rc); struct DateServiceInner { - current: UnsafeCell>, + current: Cell>, } impl DateServiceInner { fn new() -> Self { DateServiceInner { - current: UnsafeCell::new(None), + current: Cell::new(None), } } fn reset(&self) { - unsafe { (&mut *self.current.get()).take() }; + self.current.take(); } fn update(&self) { let now = Instant::now(); let date = Date::new(); - *(unsafe { &mut *self.current.get() }) = Some((date, now)); + self.current.set(Some((date, now))); } } @@ -255,7 +255,7 @@ impl DateService { } fn check_date(&self) { - if unsafe { (&*self.0.current.get()).is_none() } { + if self.0.current.get().is_none() { self.0.update(); // periodic date update @@ -269,12 +269,12 @@ impl DateService { fn now(&self) -> Instant { self.check_date(); - unsafe { (&*self.0.current.get()).as_ref().unwrap().1 } + self.0.current.get().unwrap().1 } fn set_date(&self, mut f: F) { self.check_date(); - f(&unsafe { (&*self.0.current.get()).as_ref().unwrap().0 }) + f(&self.0.current.get().unwrap().0); } } @@ -282,6 +282,19 @@ impl DateService { mod tests { use super::*; + + // Test modifying the date from within the closure + // passed to `set_date` + #[test] + fn test_evil_date() { + let service = DateService::new(); + // Make sure that `check_date` doesn't try to spawn a task + service.0.update(); + service.set_date(|_| { + service.0.reset() + }); + } + #[test] fn test_date_len() { assert_eq!(DATE_VALUE_LENGTH, "Sun, 06 Nov 1994 08:49:37 GMT".len()); diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index fd0fe927f..b6637075c 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -60,6 +60,12 @@ impl Error { } } +/// A struct with a private constructor, for use with +/// `__private_get_type_id__`. Its single field is private, +/// ensuring that it can only be constructed from this module +#[doc(hidden)] +pub struct PrivateHelper(()); + /// Error that can be converted to `Response` pub trait ResponseError: fmt::Debug + fmt::Display { /// Response's status code @@ -83,19 +89,37 @@ pub trait ResponseError: fmt::Debug + fmt::Display { resp.set_body(Body::from(buf)) } + /// A helper method to get the type ID of the type + /// this trait is implemented on. + /// This method is unsafe to *implement*, since `downcast_ref` relies + /// on the returned `TypeId` to perform a cast. + /// + /// Unfortunately, Rust has no notion of a trait method that is + /// unsafe to implement (marking it as `unsafe` makes it unsafe + /// to *call*). As a workaround, we require this method + /// to return a private type along with the `TypeId`. This + /// private type (`PrivateHelper`) has a private constructor, + /// making it impossible for safe code to construct outside of + /// this module. This ensures that safe code cannot violate + /// type-safety by implementing this method. #[doc(hidden)] - fn __private_get_type_id__(&self) -> TypeId + fn __private_get_type_id__(&self) -> (TypeId, PrivateHelper) where Self: 'static, { - TypeId::of::() + (TypeId::of::(), PrivateHelper(())) } } impl dyn ResponseError + 'static { /// Downcasts a response error to a specific type. pub fn downcast_ref(&self) -> Option<&T> { - if self.__private_get_type_id__() == TypeId::of::() { + if self.__private_get_type_id__().0 == TypeId::of::() { + // Safety: external crates cannot override the default + // implementation of `__private_get_type_id__`, since + // it requires returning a private type. We can therefore + // rely on the returned `TypeId`, which ensures that this + // case is correct. unsafe { Some(&*(self as *const dyn ResponseError as *const T)) } } else { None diff --git a/actix-session/CHANGES.md b/actix-session/CHANGES.md index f2c1a5c0b..f6753ae58 100644 --- a/actix-session/CHANGES.md +++ b/actix-session/CHANGES.md @@ -3,6 +3,7 @@ ## [Unreleased] - 2020-01-xx * Update the `time` dependency to 0.2.5 +* [#1292](https://github.com/actix/actix-web/pull/1292) Long lasting auto-prolonged session ## [0.3.0] - 2019-12-20 diff --git a/actix-session/src/cookie.rs b/actix-session/src/cookie.rs index bc0262935..b5297f561 100644 --- a/actix-session/src/cookie.rs +++ b/actix-session/src/cookie.rs @@ -58,6 +58,7 @@ struct CookieSessionInner { secure: bool, http_only: bool, max_age: Option, + expires_in: Option, same_site: Option, } @@ -72,6 +73,7 @@ impl CookieSessionInner { secure: true, http_only: true, max_age: None, + expires_in: None, same_site: None, } } @@ -97,6 +99,10 @@ impl CookieSessionInner { cookie.set_domain(domain.clone()); } + if let Some(expires_in) = self.expires_in { + cookie.set_expires(OffsetDateTime::now() + expires_in); + } + if let Some(max_age) = self.max_age { cookie.set_max_age(max_age); } @@ -272,6 +278,17 @@ impl CookieSession { Rc::get_mut(&mut self.0).unwrap().max_age = Some(value); self } + + /// Sets the `expires` field in the session cookie being built. + pub fn expires_in(self, seconds: i64) -> CookieSession { + self.expires_in_time(Duration::seconds(seconds)) + } + + /// Sets the `expires` field in the session cookie being built. + pub fn expires_in_time(mut self, value: Duration) -> CookieSession { + Rc::get_mut(&mut self.0).unwrap().expires_in = Some(value); + self + } } impl Transform for CookieSession @@ -324,6 +341,7 @@ where fn call(&mut self, mut req: ServiceRequest) -> Self::Future { let inner = self.inner.clone(); let (is_new, state) = self.inner.load(&req); + let prolong_expiration = self.inner.expires_in.is_some(); Session::set_session(state.into_iter(), &mut req); let fut = self.service.call(req); @@ -335,6 +353,9 @@ where | (SessionStatus::Renewed, Some(state)) => { res.checked_expr(|res| inner.set_cookie(res, state)) } + (SessionStatus::Unchanged, Some(state)) if prolong_expiration => { + res.checked_expr(|res| inner.set_cookie(res, state)) + } (SessionStatus::Unchanged, _) => // set a new session cookie upon first request (new client) { @@ -478,4 +499,47 @@ mod tests { let body = test::read_response(&mut app, request).await; assert_eq!(body, Bytes::from_static(b"counter: 100")); } + + #[actix_rt::test] + async fn prolong_expiration() { + let mut app = test::init_service( + App::new() + .wrap(CookieSession::signed(&[0; 32]).secure(false).expires_in(60)) + .service(web::resource("/").to(|ses: Session| { + async move { + let _ = ses.set("counter", 100); + "test" + } + })) + .service( + web::resource("/test/") + .to(|| async move { "no-changes-in-session" }), + ), + ) + .await; + + let request = test::TestRequest::get().to_request(); + let response = app.call(request).await.unwrap(); + let expires_1 = response + .response() + .cookies() + .find(|c| c.name() == "actix-session") + .expect("Cookie is set") + .expires() + .expect("Expiration is set"); + + actix_rt::time::delay_for(std::time::Duration::from_secs(1)).await; + + let request = test::TestRequest::with_uri("/test/").to_request(); + let response = app.call(request).await.unwrap(); + let expires_2 = response + .response() + .cookies() + .find(|c| c.name() == "actix-session") + .expect("Cookie is set") + .expires() + .expect("Expiration is set"); + + assert!(expires_2 - expires_1 >= Duration::seconds(1)); + } }