diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index ff8ccd640..9f606dfcd 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -1,7 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +* Fix If-Modified-Since and If-Unmodified-Since to not compare using sub-second timestamps. [#1887] +[#1887]: https://github.com/actix/actix-web/pull/1887 ## 0.6.0-beta.1 - 2021-01-07 * `HttpRange::parse` now has its own error type. diff --git a/actix-files/src/files.rs b/actix-files/src/files.rs index 98dd26880..0cf0a91ba 100644 --- a/actix-files/src/files.rs +++ b/actix-files/src/files.rs @@ -82,8 +82,9 @@ impl Files { /// be inaccessible. Register more specific handlers and services first. /// /// `Files` uses a threadpool for blocking filesystem operations. By default, the pool uses a - /// number of threads equal to 5x the number of available logical CPUs. Pool size can be changed - /// by setting ACTIX_THREADPOOL environment variable. + /// max number of threads equal to `512 * HttpServer::worker`. Real time thread count are + /// adjusted with work load. More threads would spawn when need and threads goes idle for a + /// period of time would be de-spawned. pub fn new>(mount_path: &str, serve_from: T) -> Files { let orig_dir = serve_from.into(); let dir = match orig_dir.canonicalize() { diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index b7225fbc0..f4314f6bc 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -107,6 +107,18 @@ mod tests { assert_eq!(resp.status(), StatusCode::NOT_MODIFIED); } + #[actix_rt::test] + async fn test_if_modified_since_without_if_none_match_same() { + let file = NamedFile::open("Cargo.toml").unwrap(); + let since = file.last_modified().unwrap(); + + let req = TestRequest::default() + .header(header::IF_MODIFIED_SINCE, since) + .to_http_request(); + let resp = file.respond_to(&req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_MODIFIED); + } + #[actix_rt::test] async fn test_if_modified_since_with_if_none_match() { let file = NamedFile::open("Cargo.toml").unwrap(); @@ -121,6 +133,30 @@ mod tests { assert_ne!(resp.status(), StatusCode::NOT_MODIFIED); } + #[actix_rt::test] + async fn test_if_unmodified_since() { + let file = NamedFile::open("Cargo.toml").unwrap(); + let since = file.last_modified().unwrap(); + + let req = TestRequest::default() + .header(header::IF_UNMODIFIED_SINCE, since) + .to_http_request(); + let resp = file.respond_to(&req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + + #[actix_rt::test] + async fn test_if_unmodified_since_failed() { + let file = NamedFile::open("Cargo.toml").unwrap(); + let since = header::HttpDate::from(SystemTime::UNIX_EPOCH); + + let req = TestRequest::default() + .header(header::IF_UNMODIFIED_SINCE, since) + .to_http_request(); + let resp = file.respond_to(&req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::PRECONDITION_FAILED); + } + #[actix_rt::test] async fn test_named_file_text() { assert!(NamedFile::open("test--").is_err()); diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index b3c247b1f..dc461e29a 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -331,7 +331,7 @@ impl NamedFile { let t2: SystemTime = since.clone().into(); match (t1.duration_since(UNIX_EPOCH), t2.duration_since(UNIX_EPOCH)) { - (Ok(t1), Ok(t2)) => t1 > t2, + (Ok(t1), Ok(t2)) => t1.as_secs() > t2.as_secs(), _ => false, } } else { @@ -350,7 +350,7 @@ impl NamedFile { let t2: SystemTime = since.clone().into(); match (t1.duration_since(UNIX_EPOCH), t2.duration_since(UNIX_EPOCH)) { - (Ok(t1), Ok(t2)) => t1 <= t2, + (Ok(t1), Ok(t2)) => t1.as_secs() <= t2.as_secs(), _ => false, } } else { diff --git a/actix-http/src/h1/encoder.rs b/actix-http/src/h1/encoder.rs index 4427174ec..bb89905fb 100644 --- a/actix-http/src/h1/encoder.rs +++ b/actix-http/src/h1/encoder.rs @@ -8,7 +8,7 @@ use bytes::{BufMut, BytesMut}; use crate::body::BodySize; use crate::config::ServiceConfig; -use crate::header::map; +use crate::header::{map::Value, HeaderName}; use crate::helpers; use crate::http::header::{CONNECTION, CONTENT_LENGTH, DATE, TRANSFER_ENCODING}; use crate::http::{HeaderMap, StatusCode, Version}; @@ -121,21 +121,11 @@ pub(crate) trait MessageType: Sized { _ => {} } - // merging headers from head and extra headers. HeaderMap::new() does not allocate. - let empty_headers = HeaderMap::new(); - let extra_headers = self.extra_headers().unwrap_or(&empty_headers); - let headers = self - .headers() - .inner - .iter() - .filter(|(name, _)| !extra_headers.contains_key(*name)) - .chain(extra_headers.inner.iter()); - // write headers let mut has_date = false; - let mut buf = dst.chunk_mut().as_mut_ptr() as *mut u8; + let mut buf = dst.chunk_mut().as_mut_ptr(); let mut remaining = dst.capacity() - dst.len(); // tracks bytes written since last buffer resize @@ -143,10 +133,10 @@ pub(crate) trait MessageType: Sized { // container's knowledge, this is used to sync the containers cursor after data is written let mut pos = 0; - for (key, value) in headers { + self.write_headers(|key, value| { match *key { - CONNECTION => continue, - TRANSFER_ENCODING | CONTENT_LENGTH if skip_len => continue, + CONNECTION => return, + TRANSFER_ENCODING | CONTENT_LENGTH if skip_len => return, DATE => has_date = true, _ => {} } @@ -155,7 +145,7 @@ pub(crate) trait MessageType: Sized { let k_len = k.len(); match value { - map::Value::One(ref val) => { + Value::One(ref val) => { let v = val.as_ref(); let v_len = v.len(); @@ -177,7 +167,7 @@ pub(crate) trait MessageType: Sized { // re-assign buf raw pointer since it's possible that the buffer was // reallocated and/or resized - buf = dst.chunk_mut().as_mut_ptr() as *mut u8; + buf = dst.chunk_mut().as_mut_ptr(); } // SAFETY: on each write, it is enough to ensure that the advancement of the @@ -206,7 +196,7 @@ pub(crate) trait MessageType: Sized { remaining -= len; } - map::Value::Multi(ref vec) => { + Value::Multi(ref vec) => { for val in vec { let v = val.as_ref(); let v_len = v.len(); @@ -224,7 +214,7 @@ pub(crate) trait MessageType: Sized { // re-assign buf raw pointer since it's possible that the buffer was // reallocated and/or resized - buf = dst.chunk_mut().as_mut_ptr() as *mut u8; + buf = dst.chunk_mut().as_mut_ptr(); } // SAFETY: on each write, it is enough to ensure that the advancement of @@ -253,7 +243,7 @@ pub(crate) trait MessageType: Sized { } } } - } + }); // final cursor synchronization with the bytes container // @@ -273,6 +263,24 @@ pub(crate) trait MessageType: Sized { Ok(()) } + + fn write_headers(&mut self, mut f: F) + where + F: FnMut(&HeaderName, &Value), + { + match self.extra_headers() { + Some(headers) => { + // merging headers from head and extra headers. + self.headers() + .inner + .iter() + .filter(|(name, _)| !headers.contains_key(*name)) + .chain(headers.inner.iter()) + .for_each(|(k, v)| f(k, v)) + } + None => self.headers().inner.iter().for_each(|(k, v)| f(k, v)), + } + } } impl MessageType for Response<()> {