From 07a729043211e819a8494d53f01c50445cf491f2 Mon Sep 17 00:00:00 2001 From: Torin Cooper-Bennun <40573959+tcbennun@users.noreply.github.com> Date: Mon, 19 Sep 2022 18:44:52 +0100 Subject: [PATCH 01/15] Fix typo in error string for i32 parse in router deserialization (#2876) * fix typo in error string for i32 parse * update actix-router changelog for #2876 * Update CHANGES.md Co-authored-by: Rob Ede --- actix-router/CHANGES.md | 3 +++ actix-router/src/de.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index ec4676ea3..45db615e8 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,8 +1,11 @@ # Changes ## Unreleased - 2022-xx-xx +- Fix typo in error string in `Deserializer::deserialize_i32` implementation for `Value`. [#2876] - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. +[#2876]: https://github.com/actix/actix-web/pull/2876 + ## 0.5.0 - 2022-02-22 ### Added diff --git a/actix-router/src/de.rs b/actix-router/src/de.rs index 55fcdc912..458e08930 100644 --- a/actix-router/src/de.rs +++ b/actix-router/src/de.rs @@ -293,7 +293,7 @@ impl<'de> Deserializer<'de> for Value<'de> { parse_value!(deserialize_bool, visit_bool, "bool"); parse_value!(deserialize_i8, visit_i8, "i8"); parse_value!(deserialize_i16, visit_i16, "i16"); - parse_value!(deserialize_i32, visit_i32, "i16"); + parse_value!(deserialize_i32, visit_i32, "i32"); parse_value!(deserialize_i64, visit_i64, "i64"); parse_value!(deserialize_u8, visit_u8, "u8"); parse_value!(deserialize_u16, visit_u16, "u16"); From 894effb8569e53227e98480fd72becc04ea3131d Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 19 Sep 2022 18:52:16 +0100 Subject: [PATCH 02/15] prepare actix-router release 0.5.1 --- actix-router/CHANGES.md | 5 ++++- actix-router/Cargo.toml | 2 +- actix-web-codegen/Cargo.toml | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/actix-router/CHANGES.md b/actix-router/CHANGES.md index 45db615e8..51e7cbc10 100644 --- a/actix-router/CHANGES.md +++ b/actix-router/CHANGES.md @@ -1,7 +1,10 @@ # Changes ## Unreleased - 2022-xx-xx -- Fix typo in error string in `Deserializer::deserialize_i32` implementation for `Value`. [#2876] + + +## 0.5.1 - 2022-09-19 +- Correct typo in error string for `i32` deserialization. [#2876] - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. [#2876]: https://github.com/actix/actix-web/pull/2876 diff --git a/actix-router/Cargo.toml b/actix-router/Cargo.toml index 141b2d39e..19d6abc62 100644 --- a/actix-router/Cargo.toml +++ b/actix-router/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "actix-router" -version = "0.5.0" +version = "0.5.1" authors = [ "Nikolay Kim ", "Ali MJ Al-Nasrawy ", diff --git a/actix-web-codegen/Cargo.toml b/actix-web-codegen/Cargo.toml index a89dde2ee..2477364a6 100644 --- a/actix-web-codegen/Cargo.toml +++ b/actix-web-codegen/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" proc-macro = true [dependencies] -actix-router = "0.5.0" +actix-router = "0.5" proc-macro2 = "1" quote = "1" syn = { version = "1", features = ["full", "extra-traits"] } From 4d3689db5e0dc1f7321994d4405b116094b949b6 Mon Sep 17 00:00:00 2001 From: e-rhodes <33500135+e-rhodes@users.noreply.github.com> Date: Tue, 20 Sep 2022 17:17:58 -0600 Subject: [PATCH 03/15] Remove unnecesary clones in extractor configs (#2884) Co-authored-by: erhodes Co-authored-by: Rob Ede --- actix-web/src/types/json.rs | 2 +- actix-web/src/types/payload.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/actix-web/src/types/json.rs b/actix-web/src/types/json.rs index 8fdbfafa4..4eab55175 100644 --- a/actix-web/src/types/json.rs +++ b/actix-web/src/types/json.rs @@ -290,7 +290,7 @@ const DEFAULT_CONFIG: JsonConfig = JsonConfig { impl Default for JsonConfig { fn default() -> Self { - DEFAULT_CONFIG.clone() + DEFAULT_CONFIG } } diff --git a/actix-web/src/types/payload.rs b/actix-web/src/types/payload.rs index af195b867..f17a4ed6d 100644 --- a/actix-web/src/types/payload.rs +++ b/actix-web/src/types/payload.rs @@ -271,7 +271,7 @@ const DEFAULT_CONFIG: PayloadConfig = PayloadConfig { impl Default for PayloadConfig { fn default() -> Self { - DEFAULT_CONFIG.clone() + DEFAULT_CONFIG } } From ef64d6a27cb57853a62739935a7d124d3500947f Mon Sep 17 00:00:00 2001 From: david-monroe <112688821+david-monroe@users.noreply.github.com> Date: Fri, 23 Sep 2022 14:39:18 +0200 Subject: [PATCH 04/15] update derive_more dependency to 0.99.8 (#2888) --- actix-web/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actix-web/Cargo.toml b/actix-web/Cargo.toml index d57e52f36..c7b54bd46 100644 --- a/actix-web/Cargo.toml +++ b/actix-web/Cargo.toml @@ -80,7 +80,7 @@ bytes = "1" bytestring = "1" cfg-if = "1" cookie = { version = "0.16", features = ["percent-encode"], optional = true } -derive_more = "0.99.5" +derive_more = "0.99.8" encoding_rs = "0.8" futures-core = { version = "0.3.7", default-features = false } futures-util = { version = "0.3.7", default-features = false } From fd6330585978d949627fcdca42a6896031b7fed5 Mon Sep 17 00:00:00 2001 From: Jacob Halsey Date: Fri, 23 Sep 2022 18:06:40 +0100 Subject: [PATCH 05/15] Fix actix-multipart field content_type() to return an Option (#2885) Co-authored-by: Rob Ede --- actix-multipart/CHANGES.md | 3 +++ actix-multipart/src/server.rs | 47 +++++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/actix-multipart/CHANGES.md b/actix-multipart/CHANGES.md index d0da40fb4..655487e54 100644 --- a/actix-multipart/CHANGES.md +++ b/actix-multipart/CHANGES.md @@ -2,6 +2,9 @@ ## Unreleased - 2022-xx-xx - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. +- `Field::content_type()` now returns `Option<&mime::Mime>` [#2880] + +[#2880]: https://github.com/actix/actix-web/pull/2880 ## 0.4.0 - 2022-02-25 diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index 239f7f905..6985cb56d 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -361,17 +361,18 @@ impl InnerMultipart { return Poll::Ready(Some(Err(MultipartError::NoContentDisposition))); }; - let ct: mime::Mime = headers + let ct: Option = headers .get(&header::CONTENT_TYPE) .and_then(|ct| ct.to_str().ok()) - .and_then(|ct| ct.parse().ok()) - .unwrap_or(mime::APPLICATION_OCTET_STREAM); + .and_then(|ct| ct.parse().ok()); self.state = InnerState::Boundary; // nested multipart stream is not supported - if ct.type_() == mime::MULTIPART { - return Poll::Ready(Some(Err(MultipartError::Nested))); + if let Some(mime) = &ct { + if mime.type_() == mime::MULTIPART { + return Poll::Ready(Some(Err(MultipartError::Nested))); + } } let field = @@ -399,7 +400,7 @@ impl Drop for InnerMultipart { /// A single field in a multipart stream pub struct Field { - ct: mime::Mime, + ct: Option, cd: ContentDisposition, headers: HeaderMap, inner: Rc>, @@ -410,7 +411,7 @@ impl Field { fn new( safety: Safety, headers: HeaderMap, - ct: mime::Mime, + ct: Option, cd: ContentDisposition, inner: Rc>, ) -> Self { @@ -428,9 +429,13 @@ impl Field { &self.headers } - /// Returns a reference to the field's content (mime) type. - pub fn content_type(&self) -> &mime::Mime { - &self.ct + /// Returns a reference to the field's content (mime) type, if it is supplied by the client. + /// + /// According to [RFC 7578](https://www.rfc-editor.org/rfc/rfc7578#section-4.4), if it is not + /// present, it should default to "text/plain". Note it is the responsibility of the client to + /// provide the appropriate content type, there is no attempt to validate this by the server. + pub fn content_type(&self) -> Option<&mime::Mime> { + self.ct.as_ref() } /// Returns the field's Content-Disposition. @@ -482,7 +487,11 @@ impl Stream for Field { impl fmt::Debug for Field { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "\nField: {}", self.ct)?; + if let Some(ct) = &self.ct { + writeln!(f, "\nField: {}", ct)?; + } else { + writeln!(f, "\nField:")?; + } writeln!(f, " boundary: {}", self.inner.borrow().boundary)?; writeln!(f, " headers:")?; for (key, val) in self.headers.iter() { @@ -1024,8 +1033,8 @@ mod tests { assert_eq!(cd.disposition, DispositionType::FormData); assert_eq!(cd.parameters[0], DispositionParam::Name("file".into())); - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); match field.next().await.unwrap() { Ok(chunk) => assert_eq!(chunk, "test"), @@ -1041,8 +1050,8 @@ mod tests { match multipart.next().await.unwrap() { Ok(mut field) => { - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); match field.next().await { Some(Ok(chunk)) => assert_eq!(chunk, "data"), @@ -1086,8 +1095,8 @@ mod tests { assert_eq!(cd.disposition, DispositionType::FormData); assert_eq!(cd.parameters[0], DispositionParam::Name("file".into())); - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); assert_eq!(get_whole_field(&mut field).await, "test"); } @@ -1096,8 +1105,8 @@ mod tests { match multipart.next().await { Some(Ok(mut field)) => { - assert_eq!(field.content_type().type_(), mime::TEXT); - assert_eq!(field.content_type().subtype(), mime::PLAIN); + assert_eq!(field.content_type().unwrap().type_(), mime::TEXT); + assert_eq!(field.content_type().unwrap().subtype(), mime::PLAIN); assert_eq!(get_whole_field(&mut field).await, "data"); } From 172c4c7a0a7621e00b45838c8af29b6eaae79b1b Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 25 Sep 2022 15:32:26 +0100 Subject: [PATCH 06/15] use noop hasher in extensions (#2890) --- actix-http/CHANGES.md | 4 ++++ actix-http/src/extensions.rs | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 816d4b71f..045ae461f 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -5,7 +5,11 @@ - Implement `MessageBody` for `&mut B` where `B: MessageBody + Unpin`. [#2868] - Implement `MessageBody` for `Pin` where `B::Target: MessageBody`. [#2868] +### Performance +- Improve overall performance of operations on `Extensions`. [#2890] + [#2868]: https://github.com/actix/actix-web/pull/2868 +[#2890]: https://github.com/actix/actix-web/pull/2890 ## 3.2.2 - 2022-09-11 diff --git a/actix-http/src/extensions.rs b/actix-http/src/extensions.rs index 60b769d13..f2047a9ce 100644 --- a/actix-http/src/extensions.rs +++ b/actix-http/src/extensions.rs @@ -1,9 +1,30 @@ use std::{ any::{Any, TypeId}, + collections::HashMap, fmt, + hash::{BuildHasherDefault, Hasher}, }; -use ahash::AHashMap; +/// A hasher for `TypeId`s that takes advantage of its known characteristics. +/// +/// Author of `anymap` crate has done research on the topic: +/// https://github.com/chris-morgan/anymap/blob/2e9a5704/src/lib.rs#L599 +#[derive(Debug, Default)] +struct NoOpHasher(u64); + +impl Hasher for NoOpHasher { + fn write(&mut self, _bytes: &[u8]) { + unimplemented!("This NoOpHasher can only handle u64s") + } + + fn write_u64(&mut self, i: u64) { + self.0 = i; + } + + fn finish(&self) -> u64 { + self.0 + } +} /// A type map for request extensions. /// @@ -11,7 +32,7 @@ use ahash::AHashMap; #[derive(Default)] pub struct Extensions { /// Use AHasher with a std HashMap with for faster lookups on the small `TypeId` keys. - map: AHashMap>, + map: HashMap, BuildHasherDefault>, } impl Extensions { @@ -19,7 +40,7 @@ impl Extensions { #[inline] pub fn new() -> Extensions { Extensions { - map: AHashMap::new(), + map: HashMap::default(), } } From cc7145d41dd75beb61ee1882ba677c20d5f1fd76 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 25 Sep 2022 20:54:17 +0100 Subject: [PATCH 07/15] rust 1.64 clippy run (#2891) --- actix-files/src/service.rs | 2 +- actix-http/src/body/message_body.rs | 2 +- actix-http/src/body/utils.rs | 2 +- actix-http/src/h1/dispatcher_tests.rs | 4 ++-- actix-http/src/header/map.rs | 2 +- actix-http/src/requests/request.rs | 4 ++-- actix-http/src/responses/response.rs | 4 ++-- actix-http/src/ws/frame.rs | 8 ++++---- actix-http/src/ws/proto.rs | 2 +- actix-http/tests/test_rustls.rs | 2 +- actix-multipart/src/server.rs | 14 ++++++-------- actix-router/src/path.rs | 1 + actix-router/src/resource.rs | 18 +++++++++--------- actix-web/src/app.rs | 2 +- actix-web/src/app_service.rs | 2 +- actix-web/src/config.rs | 2 +- actix-web/src/http/header/cache_control.rs | 2 +- actix-web/src/request.rs | 12 ++++++------ actix-web/src/response/builder.rs | 2 +- actix-web/src/rmap.rs | 12 ++++++------ actix-web/src/scope.rs | 5 ++--- awc/src/client/pool.rs | 2 +- awc/src/ws.rs | 4 ++-- awc/tests/test_client.rs | 15 ++++----------- 24 files changed, 58 insertions(+), 67 deletions(-) diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs index ec09af01c..d94fd5850 100644 --- a/actix-files/src/service.rs +++ b/actix-files/src/service.rs @@ -23,7 +23,7 @@ impl Deref for FilesService { type Target = FilesServiceInner; fn deref(&self) -> &Self::Target { - &*self.0 + &self.0 } } diff --git a/actix-http/src/body/message_body.rs b/actix-http/src/body/message_body.rs index bac2fece1..0cfaa8653 100644 --- a/actix-http/src/body/message_body.rs +++ b/actix-http/src/body/message_body.rs @@ -131,7 +131,7 @@ mod foreign_impls { type Error = B::Error; fn size(&self) -> BodySize { - (&**self).size() + (**self).size() } fn poll_next( diff --git a/actix-http/src/body/utils.rs b/actix-http/src/body/utils.rs index 194af47f8..0a6fb0c15 100644 --- a/actix-http/src/body/utils.rs +++ b/actix-http/src/body/utils.rs @@ -42,7 +42,7 @@ pub async fn to_bytes(body: B) -> Result { let body = body.as_mut(); match ready!(body.poll_next(cx)) { - Some(Ok(bytes)) => buf.extend_from_slice(&*bytes), + Some(Ok(bytes)) => buf.extend_from_slice(&bytes), None => return Poll::Ready(Ok(())), Some(Err(err)) => return Poll::Ready(Err(err)), } diff --git a/actix-http/src/h1/dispatcher_tests.rs b/actix-http/src/h1/dispatcher_tests.rs index b3ee3d2bb..3eea859bf 100644 --- a/actix-http/src/h1/dispatcher_tests.rs +++ b/actix-http/src/h1/dispatcher_tests.rs @@ -637,7 +637,7 @@ async fn expect_handling() { if let DispatcherState::Normal { ref inner } = h1.inner { let io = inner.io.as_ref().unwrap(); - let mut res = (&io.write_buf()[..]).to_owned(); + let mut res = io.write_buf()[..].to_owned(); stabilize_date_header(&mut res); assert_eq!( @@ -699,7 +699,7 @@ async fn expect_eager() { if let DispatcherState::Normal { ref inner } = h1.inner { let io = inner.io.as_ref().unwrap(); - let mut res = (&io.write_buf()[..]).to_owned(); + let mut res = io.write_buf()[..].to_owned(); stabilize_date_header(&mut res); // Despite the content-length header and even though the request payload has not diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 8f6d1cead..28906e835 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -309,7 +309,7 @@ impl HeaderMap { pub fn get_all(&self, key: impl AsHeaderName) -> std::slice::Iter<'_, HeaderValue> { match self.get_value(key) { Some(value) => value.iter(), - None => (&[]).iter(), + None => [].iter(), } } diff --git a/actix-http/src/requests/request.rs b/actix-http/src/requests/request.rs index 0f8e78d46..ac358e8df 100644 --- a/actix-http/src/requests/request.rs +++ b/actix-http/src/requests/request.rs @@ -113,14 +113,14 @@ impl

Request

{ #[inline] /// Http message part of the request pub fn head(&self) -> &RequestHead { - &*self.head + &self.head } #[inline] #[doc(hidden)] /// Mutable reference to a HTTP message part of the request pub fn head_mut(&mut self) -> &mut RequestHead { - &mut *self.head + &mut self.head } /// Mutable reference to the message's headers. diff --git a/actix-http/src/responses/response.rs b/actix-http/src/responses/response.rs index ceb158f65..03908d9ce 100644 --- a/actix-http/src/responses/response.rs +++ b/actix-http/src/responses/response.rs @@ -83,13 +83,13 @@ impl Response { /// Returns a reference to the head of this response. #[inline] pub fn head(&self) -> &ResponseHead { - &*self.head + &self.head } /// Returns a mutable reference to the head of this response. #[inline] pub fn head_mut(&mut self) -> &mut ResponseHead { - &mut *self.head + &mut self.head } /// Returns the status code of this response. diff --git a/actix-http/src/ws/frame.rs b/actix-http/src/ws/frame.rs index 3659b6c3b..c7e0427ea 100644 --- a/actix-http/src/ws/frame.rs +++ b/actix-http/src/ws/frame.rs @@ -313,7 +313,7 @@ mod tests { #[test] fn test_parse_frame_no_mask() { let mut buf = BytesMut::from(&[0b0000_0001u8, 0b0000_0001u8][..]); - buf.extend(&[1u8]); + buf.extend([1u8]); assert!(Parser::parse(&mut buf, true, 1024).is_err()); @@ -326,7 +326,7 @@ mod tests { #[test] fn test_parse_frame_max_size() { let mut buf = BytesMut::from(&[0b0000_0001u8, 0b0000_0010u8][..]); - buf.extend(&[1u8, 1u8]); + buf.extend([1u8, 1u8]); assert!(Parser::parse(&mut buf, true, 1).is_err()); @@ -340,9 +340,9 @@ mod tests { fn test_parse_frame_max_size_recoverability() { let mut buf = BytesMut::new(); // The first text frame with length == 2, payload doesn't matter. - buf.extend(&[0b0000_0001u8, 0b0000_0010u8, 0b0000_0000u8, 0b0000_0000u8]); + buf.extend([0b0000_0001u8, 0b0000_0010u8, 0b0000_0000u8, 0b0000_0000u8]); // Next binary frame with length == 2 and payload == `[0x1111_1111u8, 0x1111_1111u8]`. - buf.extend(&[0b0000_0010u8, 0b0000_0010u8, 0b1111_1111u8, 0b1111_1111u8]); + buf.extend([0b0000_0010u8, 0b0000_0010u8, 0b1111_1111u8, 0b1111_1111u8]); assert_eq!(buf.len(), 8); assert!(matches!( diff --git a/actix-http/src/ws/proto.rs b/actix-http/src/ws/proto.rs index 01fe9dd3c..7222168b7 100644 --- a/actix-http/src/ws/proto.rs +++ b/actix-http/src/ws/proto.rs @@ -244,7 +244,7 @@ pub fn hash_key(key: &[u8]) -> [u8; 28] { }; let mut hash_b64 = [0; 28]; - let n = base64::encode_config_slice(&hash, base64::STANDARD, &mut hash_b64); + let n = base64::encode_config_slice(hash, base64::STANDARD, &mut hash_b64); assert_eq!(n, 28); hash_b64 diff --git a/actix-http/tests/test_rustls.rs b/actix-http/tests/test_rustls.rs index 2bbf1524b..d9ff42b7d 100644 --- a/actix-http/tests/test_rustls.rs +++ b/actix-http/tests/test_rustls.rs @@ -41,7 +41,7 @@ where let body = stream.as_mut(); match ready!(body.poll_next(cx)) { - Some(Ok(bytes)) => buf.extend_from_slice(&*bytes), + Some(Ok(bytes)) => buf.extend_from_slice(&bytes), None => return Poll::Ready(Ok(())), Some(Err(err)) => return Poll::Ready(Err(err)), } diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index 6985cb56d..c3757177f 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -289,10 +289,8 @@ impl InnerMultipart { match self.state { // read until first boundary InnerState::FirstBoundary => { - match InnerMultipart::skip_until_boundary( - &mut *payload, - &self.boundary, - )? { + match InnerMultipart::skip_until_boundary(&mut payload, &self.boundary)? + { Some(eof) => { if eof { self.state = InnerState::Eof; @@ -306,7 +304,7 @@ impl InnerMultipart { } // read boundary InnerState::Boundary => { - match InnerMultipart::read_boundary(&mut *payload, &self.boundary)? { + match InnerMultipart::read_boundary(&mut payload, &self.boundary)? { None => return Poll::Pending, Some(eof) => { if eof { @@ -323,7 +321,7 @@ impl InnerMultipart { // read field headers for next field if self.state == InnerState::Headers { - if let Some(headers) = InnerMultipart::read_headers(&mut *payload)? { + if let Some(headers) = InnerMultipart::read_headers(&mut payload)? { self.state = InnerState::Boundary; headers } else { @@ -652,9 +650,9 @@ impl InnerField { let result = if let Some(mut payload) = self.payload.as_ref().unwrap().get_mut(s) { if !self.eof { let res = if let Some(ref mut len) = self.length { - InnerField::read_len(&mut *payload, len) + InnerField::read_len(&mut payload, len) } else { - InnerField::read_stream(&mut *payload, &self.boundary) + InnerField::read_stream(&mut payload, &self.boundary) }; match res { diff --git a/actix-router/src/path.rs b/actix-router/src/path.rs index 5eef1c1e7..34dabcfbe 100644 --- a/actix-router/src/path.rs +++ b/actix-router/src/path.rs @@ -242,6 +242,7 @@ mod tests { use super::*; + #[allow(clippy::needless_borrow)] #[test] fn deref_impls() { let mut foo = Path::new("/foo"); diff --git a/actix-router/src/resource.rs b/actix-router/src/resource.rs index 3c6754aeb..f198115ad 100644 --- a/actix-router/src/resource.rs +++ b/actix-router/src/resource.rs @@ -1503,31 +1503,31 @@ mod tests { fn build_path_list() { let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/test"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["user1"].iter())); assert_eq!(s, "/user/user1/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/test"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/test"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2"); let mut s = String::new(); let resource = ResourceDef::new("/user/{item1}/{item2}/"); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/"); let mut s = String::new(); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut ["item"].iter())); let mut s = String::new(); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["item", "item2"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["item", "item2"].iter())); assert_eq!(s, "/user/item/item2/"); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&["item"]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut ["item"].iter())); let mut s = String::new(); assert!(resource.resource_path_from_iter(&mut s, &mut vec!["item", "item2"].iter())); @@ -1604,10 +1604,10 @@ mod tests { let resource = ResourceDef::new("/user/{item1}*"); let mut s = String::new(); - assert!(!resource.resource_path_from_iter(&mut s, &mut (&[""; 0]).iter())); + assert!(!resource.resource_path_from_iter(&mut s, &mut [""; 0].iter())); let mut s = String::new(); - assert!(resource.resource_path_from_iter(&mut s, &mut (&["user1"]).iter())); + assert!(resource.resource_path_from_iter(&mut s, &mut ["user1"].iter())); assert_eq!(s, "/user/user1"); let mut s = String::new(); diff --git a/actix-web/src/app.rs b/actix-web/src/app.rs index 213c8beff..ec37ff8a4 100644 --- a/actix-web/src/app.rs +++ b/actix-web/src/app.rs @@ -682,7 +682,7 @@ mod tests { "/test", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["12345"]).unwrap().to_string()) + .body(req.url_for("youtube", ["12345"]).unwrap().to_string()) }), ), ) diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 28ff8c614..0b5ba2ab6 100644 --- a/actix-web/src/app_service.rs +++ b/actix-web/src/app_service.rs @@ -173,7 +173,7 @@ impl AppInitServiceState { #[inline] pub(crate) fn rmap(&self) -> &ResourceMap { - &*self.rmap + &self.rmap } #[inline] diff --git a/actix-web/src/config.rs b/actix-web/src/config.rs index 58a099c75..68bea34ca 100644 --- a/actix-web/src/config.rs +++ b/actix-web/src/config.rs @@ -344,7 +344,7 @@ mod tests { "/test", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["12345"]).unwrap().to_string()) + .body(req.url_for("youtube", ["12345"]).unwrap().to_string()) }), ), ) diff --git a/actix-web/src/http/header/cache_control.rs b/actix-web/src/http/header/cache_control.rs index 490d36558..37629313e 100644 --- a/actix-web/src/http/header/cache_control.rs +++ b/actix-web/src/http/header/cache_control.rs @@ -176,7 +176,7 @@ impl str::FromStr for CacheDirective { _ => match s.find('=') { Some(idx) if idx + 1 < s.len() => { - match (&s[..idx], (&s[idx + 1..]).trim_matches('"')) { + match (&s[..idx], s[idx + 1..].trim_matches('"')) { ("max-age", secs) => secs.parse().map(MaxAge).map_err(Some), ("max-stale", secs) => secs.parse().map(MaxStale).map_err(Some), ("min-fresh", secs) => secs.parse().map(MinFresh).map_err(Some), diff --git a/actix-web/src/request.rs b/actix-web/src/request.rs index e2a9bd4e5..6a32bf838 100644 --- a/actix-web/src/request.rs +++ b/actix-web/src/request.rs @@ -219,7 +219,7 @@ impl HttpRequest { /// for urls that do not contain variable parts. pub fn url_for_static(&self, name: &str) -> Result { const NO_PARAMS: [&str; 0] = []; - self.url_for(name, &NO_PARAMS) + self.url_for(name, NO_PARAMS) } /// Get a reference to a `ResourceMap` of current application. @@ -306,7 +306,7 @@ impl HttpRequest { #[inline] fn app_state(&self) -> &AppInitServiceState { - &*self.inner.app_state + &self.inner.app_state } /// Load request cookies. @@ -583,14 +583,14 @@ mod tests { .to_http_request(); assert_eq!( - req.url_for("unknown", &["test"]), + req.url_for("unknown", ["test"]), Err(UrlGenerationError::ResourceNotFound) ); assert_eq!( - req.url_for("index", &["test"]), + req.url_for("index", ["test"]), Err(UrlGenerationError::NotEnoughElements) ); - let url = req.url_for("index", &["test", "html"]); + let url = req.url_for("index", ["test", "html"]); assert_eq!( url.ok().unwrap().as_str(), "http://www.rust-lang.org/user/test.html" @@ -646,7 +646,7 @@ mod tests { rmap.add(&mut rdef, None); let req = TestRequest::default().rmap(rmap).to_http_request(); - let url = req.url_for("youtube", &["oHg5SJYRHA0"]); + let url = req.url_for("youtube", ["oHg5SJYRHA0"]); assert_eq!( url.ok().unwrap().as_str(), "https://youtube.com/watch/oHg5SJYRHA0" diff --git a/actix-web/src/response/builder.rs b/actix-web/src/response/builder.rs index f50aad9f4..120d4c358 100644 --- a/actix-web/src/response/builder.rs +++ b/actix-web/src/response/builder.rs @@ -457,7 +457,7 @@ mod tests { assert_eq!(ct, HeaderValue::from_static("application/json")); assert_body_eq!(res, br#"["v1","v2","v3"]"#); - let res = HttpResponse::Ok().json(&["v1", "v2", "v3"]); + let res = HttpResponse::Ok().json(["v1", "v2", "v3"]); let ct = res.headers().get(CONTENT_TYPE).unwrap(); assert_eq!(ct, HeaderValue::from_static("application/json")); assert_body_eq!(res, br#"["v1","v2","v3"]"#); diff --git a/actix-web/src/rmap.rs b/actix-web/src/rmap.rs index 6a1a187b2..6e10717c3 100644 --- a/actix-web/src/rmap.rs +++ b/actix-web/src/rmap.rs @@ -449,12 +449,12 @@ mod tests { let req = req.to_http_request(); let url = rmap - .url_for(&req, "post", &["u123", "foobar"]) + .url_for(&req, "post", ["u123", "foobar"]) .unwrap() .to_string(); assert_eq!(url, "http://localhost:8888/user/u123/post/foobar"); - assert!(rmap.url_for(&req, "missing", &["u123"]).is_err()); + assert!(rmap.url_for(&req, "missing", ["u123"]).is_err()); } #[test] @@ -490,7 +490,7 @@ mod tests { assert_eq!(url.path(), OUTPUT); assert!(rmap.url_for(&req, "external.2", INPUT).is_err()); - assert!(rmap.url_for(&req, "external.2", &[""]).is_err()); + assert!(rmap.url_for(&req, "external.2", [""]).is_err()); } #[test] @@ -524,7 +524,7 @@ mod tests { let req = req.to_http_request(); assert_eq!( - rmap.url_for(&req, "duck", &["abcd"]).unwrap().to_string(), + rmap.url_for(&req, "duck", ["abcd"]).unwrap().to_string(), "https://duck.com/abcd" ); } @@ -552,9 +552,9 @@ mod tests { let req = crate::test::TestRequest::default().to_http_request(); - let url = rmap.url_for(&req, "nested", &[""; 0]).unwrap().to_string(); + let url = rmap.url_for(&req, "nested", [""; 0]).unwrap().to_string(); assert_eq!(url, "http://localhost:8080/bar/nested"); - assert!(rmap.url_for(&req, "missing", &["u123"]).is_err()); + assert!(rmap.url_for(&req, "missing", ["u123"]).is_err()); } } diff --git a/actix-web/src/scope.rs b/actix-web/src/scope.rs index 07eb1093a..9af05674b 100644 --- a/actix-web/src/scope.rs +++ b/actix-web/src/scope.rs @@ -1133,7 +1133,7 @@ mod tests { "/", web::get().to(|req: HttpRequest| { HttpResponse::Ok() - .body(req.url_for("youtube", &["xxxxxx"]).unwrap().to_string()) + .body(req.url_for("youtube", ["xxxxxx"]).unwrap().to_string()) }), ); })); @@ -1152,8 +1152,7 @@ mod tests { let srv = init_service(App::new().service(web::scope("/a").service( web::scope("/b").service(web::resource("/c/{stuff}").name("c").route( web::get().to(|req: HttpRequest| { - HttpResponse::Ok() - .body(format!("{}", req.url_for("c", &["12345"]).unwrap())) + HttpResponse::Ok().body(format!("{}", req.url_for("c", ["12345"]).unwrap())) }), )), ))) diff --git a/awc/src/client/pool.rs b/awc/src/client/pool.rs index cc3e4d7c0..5655b5845 100644 --- a/awc/src/client/pool.rs +++ b/awc/src/client/pool.rs @@ -97,7 +97,7 @@ where type Target = ConnectionPoolInnerPriv; fn deref(&self) -> &Self::Target { - &*self.0 + &self.0 } } diff --git a/awc/src/ws.rs b/awc/src/ws.rs index d8ed4c879..b316f68b4 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -321,7 +321,7 @@ impl WebsocketsRequest { // Generate a random key for the `Sec-WebSocket-Key` header which is a base64-encoded // (see RFC 4648 §4) value that, when decoded, is 16 bytes in length (RFC 6455 §1.3). let sec_key: [u8; 16] = rand::random(); - let key = base64::encode(&sec_key); + let key = base64::encode(sec_key); self.head.headers.insert( header::SEC_WEBSOCKET_KEY, @@ -513,7 +513,7 @@ mod tests { .origin("test-origin") .max_frame_size(100) .server_mode() - .protocols(&["v1", "v2"]) + .protocols(["v1", "v2"]) .set_header_if_none(header::CONTENT_TYPE, "json") .set_header_if_none(header::CONTENT_TYPE, "text") .cookie(Cookie::build("cookie1", "value1").finish()); diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index 165d8faf0..c4b468eeb 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -707,8 +707,7 @@ async fn client_cookie_handling() { async move { // Check cookies were sent correctly - let res: Result<(), Error> = req - .cookie("cookie1") + req.cookie("cookie1") .ok_or(()) .and_then(|c1| { if c1.value() == "value1" { @@ -725,16 +724,10 @@ async fn client_cookie_handling() { Err(()) } }) - .map_err(|_| Error::from(IoError::from(ErrorKind::NotFound))); + .map_err(|_| Error::from(IoError::from(ErrorKind::NotFound)))?; - if let Err(e) = res { - Err(e) - } else { - // Send some cookies back - Ok::<_, Error>( - HttpResponse::Ok().cookie(cookie1).cookie(cookie2).finish(), - ) - } + // Send some cookies back + Ok::<_, Error>(HttpResponse::Ok().cookie(cookie1).cookie(cookie2).finish()) } }), ) From 1519ae777273b926091add7865dc1ee30c5c35fe Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 26 Sep 2022 12:29:57 +0100 Subject: [PATCH 08/15] clarify tokio::main docs --- actix-web/src/rt.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actix-web/src/rt.rs b/actix-web/src/rt.rs index 929eadfd8..7973da73c 100644 --- a/actix-web/src/rt.rs +++ b/actix-web/src/rt.rs @@ -25,7 +25,7 @@ //! ``` //! //! # Running Actix Web Using `#[tokio::main]` -//! If you need to run something alongside Actix Web that uses Tokio's work stealing functionality, +//! If you need to run something that uses Tokio's work stealing functionality alongside Actix Web, //! you can run Actix Web under `#[tokio::main]`. The [`Server`](crate::dev::Server) object returned //! from [`HttpServer::run`](crate::HttpServer::run) can also be [`spawn`]ed, if preferred. //! From ad7e67f940f0ebd25e0ad2bf1642b65b92df8e18 Mon Sep 17 00:00:00 2001 From: Benny Nazimov <66024037+benny-n@users.noreply.github.com> Date: Mon, 26 Sep 2022 21:44:51 +0300 Subject: [PATCH 09/15] add `middleware::logger::custom_response_replace` (#2631) Co-authored-by: Rob Ede --- actix-web/CHANGES.md | 2 + actix-web/src/middleware/logger.rs | 186 +++++++++++++++++++++++++---- 2 files changed, 165 insertions(+), 23 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 00ab6c9c8..a018bc248 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -4,7 +4,9 @@ ### Added - Add `ContentDisposition::attachment` constructor. [#2867] - Add `ErrorHandlers::default_handler()` (as well as `default_handler_{server, client}()`) to make registering handlers for groups of response statuses easier. [#2784] +- Add `Logger::custom_response_replace()`. [#2631] +[#2631]: https://github.com/actix/actix-web/pull/2631 [#2784]: https://github.com/actix/actix-web/pull/2784 [#2867]: https://github.com/actix/actix-web/pull/2867 diff --git a/actix-web/src/middleware/logger.rs b/actix-web/src/middleware/logger.rs index 53a3550de..5fec5a013 100644 --- a/actix-web/src/middleware/logger.rs +++ b/actix-web/src/middleware/logger.rs @@ -26,7 +26,7 @@ use crate::{ body::{BodySize, MessageBody}, http::header::HeaderName, service::{ServiceRequest, ServiceResponse}, - Error, HttpResponse, Result, + Error, Result, }; /// Middleware for logging request and response summaries to the terminal. @@ -69,10 +69,11 @@ use crate::{ /// `%D` | Time taken to serve the request, in milliseconds /// `%U` | Request URL /// `%{r}a` | "Real IP" remote address **\*** -/// `%{FOO}i` | `request.headers["FOO"]` +/// `%{FOO}i` | `request.headers["FOO"]` /// `%{FOO}o` | `response.headers["FOO"]` /// `%{FOO}e` | `env_var["FOO"]` /// `%{FOO}xi` | [Custom request replacement](Logger::custom_request_replace) labelled "FOO" +/// `%{FOO}xo` | [Custom response replacement](Logger::custom_response_replace) labelled "FOO" /// /// # Security /// **\*** "Real IP" remote address is calculated using @@ -179,6 +180,55 @@ impl Logger { self } + + /// Register a function that receives a `ServiceResponse` and returns a string for use in the + /// log line. + /// + /// The label passed as the first argument should match a replacement substring in + /// the logger format like `%{label}xo`. + /// + /// It is convention to print "-" to indicate no output instead of an empty string. + /// + /// The replacement function does not have access to the response body. + /// + /// # Examples + /// ``` + /// # use actix_web::{dev::ServiceResponse, middleware::Logger}; + /// fn log_if_error(res: &ServiceResponse) -> String { + /// if res.status().as_u16() >= 400 { + /// "ERROR".to_string() + /// } else { + /// "-".to_string() + /// } + /// } + /// + /// Logger::new("example %{ERROR_STATUS}xo") + /// .custom_response_replace("ERROR_STATUS", |res| log_if_error(res) ); + /// ``` + pub fn custom_response_replace( + mut self, + label: &str, + f: impl Fn(&ServiceResponse) -> String + 'static, + ) -> Self { + let inner = Rc::get_mut(&mut self.0).unwrap(); + + let ft = inner.format.0.iter_mut().find( + |ft| matches!(ft, FormatText::CustomResponse(unit_label, _) if label == unit_label), + ); + + if let Some(FormatText::CustomResponse(_, res_fn)) = ft { + *res_fn = Some(CustomResponseFn { + inner_fn: Rc::new(f), + }); + } else { + debug!( + "Attempted to register custom response logging function for non-existent label: {}", + label + ); + } + + self + } } impl Default for Logger { @@ -210,10 +260,16 @@ where fn new_transform(&self, service: S) -> Self::Future { for unit in &self.0.format.0 { - // missing request replacement function diagnostic if let FormatText::CustomRequest(label, None) = unit { warn!( - "No custom request replacement function was registered for label \"{}\".", + "No custom request replacement function was registered for label: {}", + label + ); + } + + if let FormatText::CustomResponse(label, None) = unit { + warn!( + "No custom response replacement function was registered for label: {}", label ); } @@ -308,11 +364,25 @@ where debug!("Error in response: {:?}", error); } - if let Some(ref mut format) = this.format { + let res = if let Some(ref mut format) = this.format { + // to avoid polluting all the Logger types with the body parameter we swap the body + // out temporarily since it's not usable in custom response functions anyway + + let (req, res) = res.into_parts(); + let (res, body) = res.into_parts(); + + let temp_res = ServiceResponse::new(req, res.map_into_boxed_body()); + for unit in &mut format.0 { - unit.render_response(res.response()); + unit.render_response(&temp_res); } - } + + // re-construct original service response + let (req, res) = temp_res.into_parts(); + ServiceResponse::new(req, res.set_body(body)) + } else { + res + }; let time = *this.time; let format = this.format.take(); @@ -399,7 +469,7 @@ impl Format { /// Returns `None` if the format string syntax is incorrect. pub fn new(s: &str) -> Format { log::trace!("Access log format: {}", s); - let fmt = Regex::new(r"%(\{([A-Za-z0-9\-_]+)\}([aioe]|xi)|[%atPrUsbTD]?)").unwrap(); + let fmt = Regex::new(r"%(\{([A-Za-z0-9\-_]+)\}([aioe]|x[io])|[%atPrUsbTD]?)").unwrap(); let mut idx = 0; let mut results = Vec::new(); @@ -417,7 +487,7 @@ impl Format { if key.as_str() == "r" { FormatText::RealIpRemoteAddr } else { - unreachable!() + unreachable!("regex and code mismatch") } } "i" => { @@ -428,6 +498,7 @@ impl Format { } "e" => FormatText::EnvironHeader(key.as_str().to_owned()), "xi" => FormatText::CustomRequest(key.as_str().to_owned(), None), + "xo" => FormatText::CustomResponse(key.as_str().to_owned(), None), _ => unreachable!(), }) } else { @@ -475,6 +546,7 @@ enum FormatText { ResponseHeader(HeaderName), EnvironHeader(String), CustomRequest(String, Option), + CustomResponse(String, Option), } #[derive(Clone)] @@ -494,6 +566,23 @@ impl fmt::Debug for CustomRequestFn { } } +#[derive(Clone)] +struct CustomResponseFn { + inner_fn: Rc String>, +} + +impl CustomResponseFn { + fn call(&self, res: &ServiceResponse) -> String { + (self.inner_fn)(res) + } +} + +impl fmt::Debug for CustomResponseFn { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("custom_response_fn") + } +} + impl FormatText { fn render( &self, @@ -526,11 +615,12 @@ impl FormatText { } } - fn render_response(&mut self, res: &HttpResponse) { + fn render_response(&mut self, res: &ServiceResponse) { match self { FormatText::ResponseStatus => { *self = FormatText::Str(format!("{}", res.status().as_u16())) } + FormatText::ResponseHeader(ref name) => { let s = if let Some(val) = res.headers().get(name) { if let Ok(s) = val.to_str() { @@ -543,6 +633,16 @@ impl FormatText { }; *self = FormatText::Str(s.to_string()) } + + FormatText::CustomResponse(_, res_fn) => { + let text = match res_fn { + Some(res_fn) => FormatText::Str(res_fn.call(res)), + None => FormatText::Str("-".to_owned()), + }; + + *self = text; + } + _ => {} } } @@ -627,8 +727,11 @@ mod tests { use actix_utils::future::ok; use super::*; - use crate::http::{header, StatusCode}; - use crate::test::{self, TestRequest}; + use crate::{ + http::{header, StatusCode}, + test::{self, TestRequest}, + HttpResponse, + }; #[actix_rt::test] async fn test_logger() { @@ -691,9 +794,10 @@ mod tests { unit.render_request(now, &req); } - let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); + let req = TestRequest::default().to_http_request(); + let res = ServiceResponse::new(req, HttpResponse::Ok().finish()); for unit in &mut format.0 { - unit.render_response(&resp); + unit.render_response(&res); } let entry_time = OffsetDateTime::now_utc(); @@ -723,9 +827,10 @@ mod tests { unit.render_request(now, &req); } - let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); + let req = TestRequest::default().to_http_request(); + let res = ServiceResponse::new(req, HttpResponse::Ok().force_close().finish()); for unit in &mut format.0 { - unit.render_response(&resp); + unit.render_response(&res); } let render = |fmt: &mut fmt::Formatter<'_>| { @@ -755,9 +860,10 @@ mod tests { unit.render_request(now, &req); } - let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); + let req = TestRequest::default().to_http_request(); + let res = ServiceResponse::new(req, HttpResponse::Ok().force_close().finish()); for unit in &mut format.0 { - unit.render_response(&resp); + unit.render_response(&res); } let entry_time = OffsetDateTime::now_utc(); @@ -784,9 +890,10 @@ mod tests { unit.render_request(now, &req); } - let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); + let req = TestRequest::default().to_http_request(); + let res = ServiceResponse::new(req, HttpResponse::Ok().force_close().finish()); for unit in &mut format.0 { - unit.render_response(&resp); + unit.render_response(&res); } let render = |fmt: &mut fmt::Formatter<'_>| { @@ -815,9 +922,10 @@ mod tests { unit.render_request(now, &req); } - let resp = HttpResponse::build(StatusCode::OK).force_close().finish(); + let req = TestRequest::default().to_http_request(); + let res = ServiceResponse::new(req, HttpResponse::Ok().finish()); for unit in &mut format.0 { - unit.render_response(&resp); + unit.render_response(&res); } let entry_time = OffsetDateTime::now_utc(); @@ -832,7 +940,7 @@ mod tests { } #[actix_rt::test] - async fn test_custom_closure_log() { + async fn test_custom_closure_req_log() { let mut logger = Logger::new("test %{CUSTOM}xi") .custom_request_replace("CUSTOM", |_req: &ServiceRequest| -> String { String::from("custom_log") @@ -857,6 +965,38 @@ mod tests { assert_eq!(log_output, "custom_log"); } + #[actix_rt::test] + async fn test_custom_closure_response_log() { + let mut logger = Logger::new("test %{CUSTOM}xo").custom_response_replace( + "CUSTOM", + |res: &ServiceResponse| -> String { + if res.status().as_u16() == 200 { + String::from("custom_log") + } else { + String::from("-") + } + }, + ); + let mut unit = Rc::get_mut(&mut logger.0).unwrap().format.0[1].clone(); + + let label = match &unit { + FormatText::CustomResponse(label, _) => label, + ft => panic!("expected CustomResponse, found {:?}", ft), + }; + + assert_eq!(label, "CUSTOM"); + + let req = TestRequest::default().to_http_request(); + let resp_ok = ServiceResponse::new(req, HttpResponse::Ok().finish()); + let now = OffsetDateTime::now_utc(); + unit.render_response(&resp_ok); + + let render = |fmt: &mut fmt::Formatter<'_>| unit.render(fmt, 1024, now); + + let log_output = FormatDisplay(&render).to_string(); + assert_eq!(log_output, "custom_log"); + } + #[actix_rt::test] async fn test_closure_logger_in_middleware() { let captured = "custom log replacement"; From 73b94e902d8fd7c6936728d00e1229b321c69327 Mon Sep 17 00:00:00 2001 From: kkocdko <31189892+kkocdko@users.noreply.github.com> Date: Sun, 9 Oct 2022 19:44:10 +0800 Subject: [PATCH 10/15] fix xhtml pages' `content-disposition` (#2903) Co-authored-by: Yuki Okushi --- actix-files/CHANGES.md | 2 ++ actix-files/src/named.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index a71bf14fa..6e57bf7a7 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -1,8 +1,10 @@ # Changes ## Unreleased - 2022-xx-xx +- XHTML files now use `Content-Disposition: inline` instead of `attachment`. [#2903] - Minimum supported Rust version (MSRV) is now 1.59 due to transitive `time` dependency. +[#2903]: https://github.com/actix/actix-web/pull/2903 ## 0.6.2 - 2022-07-23 - Allow partial range responses for video content to start streaming sooner. [#2817] diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 1213534c2..23d3093dc 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -132,7 +132,7 @@ impl NamedFile { mime::IMAGE | mime::TEXT | mime::AUDIO | mime::VIDEO => DispositionType::Inline, mime::APPLICATION => match ct.subtype() { mime::JAVASCRIPT | mime::JSON => DispositionType::Inline, - name if name == "wasm" => DispositionType::Inline, + name if name == "wasm" || name == "xhtml" => DispositionType::Inline, _ => DispositionType::Attachment, }, _ => DispositionType::Attachment, From f8cb71e789b7248ea0e61c6af27fc7dc79ddaeb2 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Fri, 14 Oct 2022 13:20:38 +0200 Subject: [PATCH 11/15] remove incomplete doc comment --- actix-web/src/service.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/actix-web/src/service.rs b/actix-web/src/service.rs index 0ad92d8a1..ea23f09f5 100644 --- a/actix-web/src/service.rs +++ b/actix-web/src/service.rs @@ -327,9 +327,7 @@ impl ServiceRequest { .push(extensions); } - /// Creates a context object for use with a [guard](crate::guard). - /// - /// Useful if you are implementing + /// Creates a context object for use with a routing [guard](crate::guard). #[inline] pub fn guard_ctx(&self) -> GuardContext<'_> { GuardContext { req: self } From 068909f1b3a24c4a38e485e265f976f5d7f0237c Mon Sep 17 00:00:00 2001 From: Alex <100831787+crudiedo@users.noreply.github.com> Date: Fri, 14 Oct 2022 17:52:13 +0600 Subject: [PATCH 12/15] Replace deprecated twoway with memchr (#2909) --- actix-multipart/Cargo.toml | 2 +- actix-multipart/src/server.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actix-multipart/Cargo.toml b/actix-multipart/Cargo.toml index 32ea49a24..6f631fcf1 100644 --- a/actix-multipart/Cargo.toml +++ b/actix-multipart/Cargo.toml @@ -24,7 +24,7 @@ httparse = "1.3" local-waker = "0.1" log = "0.4" mime = "0.3" -twoway = "0.2" +memchr = "2.5" [dev-dependencies] actix-rt = "2.2" diff --git a/actix-multipart/src/server.rs b/actix-multipart/src/server.rs index c3757177f..1d0510039 100644 --- a/actix-multipart/src/server.rs +++ b/actix-multipart/src/server.rs @@ -606,7 +606,7 @@ impl InnerField { } loop { - return if let Some(idx) = twoway::find_bytes(&payload.buf[pos..], b"\r") { + return if let Some(idx) = memchr::memmem::find(&payload.buf[pos..], b"\r") { let cur = pos + idx; // check if we have enough data for boundary detection @@ -827,7 +827,7 @@ impl PayloadBuffer { /// Read until specified ending fn read_until(&mut self, line: &[u8]) -> Result, MultipartError> { - let res = twoway::find_bytes(&self.buf, line) + let res = memchr::memmem::find(&self.buf, line) .map(|idx| self.buf.split_to(idx + line.len()).freeze()); if res.is_none() && self.eof { From 83cd061c862e82931895a8003fef5c191db0e5dd Mon Sep 17 00:00:00 2001 From: fakeshadow <24548779@qq.com> Date: Tue, 25 Oct 2022 23:37:04 +0800 Subject: [PATCH 13/15] remove fakeshadow from author lists (#2921) --- actix-files/Cargo.toml | 1 - awc/Cargo.toml | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/actix-files/Cargo.toml b/actix-files/Cargo.toml index 33de0e6d9..018acdfb1 100644 --- a/actix-files/Cargo.toml +++ b/actix-files/Cargo.toml @@ -3,7 +3,6 @@ name = "actix-files" version = "0.6.2" authors = [ "Nikolay Kim ", - "fakeshadow <24548779@qq.com>", "Rob Ede ", ] description = "Static file serving for Actix Web" diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 2f0027725..e7ac43d22 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -1,10 +1,7 @@ [package] name = "awc" version = "3.0.1" -authors = [ - "Nikolay Kim ", - "fakeshadow <24548779@qq.com>", -] +authors = ["Nikolay Kim "] description = "Async HTTP and WebSocket client library" keywords = ["actix", "http", "framework", "async", "web"] categories = [ From a2e2c30d59a2b04880c03953e3d2bb7c7ba48b10 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sun, 30 Oct 2022 19:47:49 +0000 Subject: [PATCH 14/15] use tokio-util deps directly where possible --- .github/workflows/upload-doc.yml | 2 +- actix-http/Cargo.toml | 2 ++ actix-http/examples/ws.rs | 2 +- actix-http/src/h1/client.rs | 2 +- actix-http/src/h1/codec.rs | 2 +- actix-http/src/h1/dispatcher.rs | 6 ++++-- actix-http/src/ws/codec.rs | 2 +- actix-http/src/ws/dispatcher.rs | 4 +++- actix-web-actors/Cargo.toml | 1 + actix-web-actors/src/ws.rs | 2 +- actix-web/src/http/header/accept.rs | 10 +++++----- 11 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.github/workflows/upload-doc.yml b/.github/workflows/upload-doc.yml index 07f839e34..c47ea1d70 100644 --- a/.github/workflows/upload-doc.yml +++ b/.github/workflows/upload-doc.yml @@ -28,7 +28,7 @@ jobs: run: echo '' > target/doc/index.html - name: Deploy to GitHub Pages - uses: JamesIves/github-pages-deploy-action@v4.4.0 + uses: JamesIves/github-pages-deploy-action@v4.4.1 with: folder: target/doc single-commit: true diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 30e436160..a8b888ef4 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -77,6 +77,8 @@ mime = "0.3" percent-encoding = "2.1" pin-project-lite = "0.2" smallvec = "1.6.1" +tokio = { version = "1.13.1", features = [] } +tokio-util = { version = "0.7", features = ["io", "codec"] } tracing = { version = "0.1.30", default-features = false, features = ["log"] } # http2 diff --git a/actix-http/examples/ws.rs b/actix-http/examples/ws.rs index c4f0503cd..6af6d5095 100644 --- a/actix-http/examples/ws.rs +++ b/actix-http/examples/ws.rs @@ -10,13 +10,13 @@ use std::{ time::Duration, }; -use actix_codec::Encoder; use actix_http::{body::BodyStream, error::Error, ws, HttpService, Request, Response}; use actix_rt::time::{interval, Interval}; use actix_server::Server; use bytes::{Bytes, BytesMut}; use bytestring::ByteString; use futures_core::{ready, Stream}; +use tokio_util::codec::Encoder; use tracing::{info, trace}; #[actix_rt::main] diff --git a/actix-http/src/h1/client.rs b/actix-http/src/h1/client.rs index 75c88d00c..6a0d531d0 100644 --- a/actix-http/src/h1/client.rs +++ b/actix-http/src/h1/client.rs @@ -1,9 +1,9 @@ use std::{fmt, io}; -use actix_codec::{Decoder, Encoder}; use bitflags::bitflags; use bytes::{Bytes, BytesMut}; use http::{Method, Version}; +use tokio_util::codec::{Decoder, Encoder}; use super::{ decoder::{self, PayloadDecoder, PayloadItem, PayloadType}, diff --git a/actix-http/src/h1/codec.rs b/actix-http/src/h1/codec.rs index 80afd7455..e11f175c9 100644 --- a/actix-http/src/h1/codec.rs +++ b/actix-http/src/h1/codec.rs @@ -1,9 +1,9 @@ use std::{fmt, io}; -use actix_codec::{Decoder, Encoder}; use bitflags::bitflags; use bytes::BytesMut; use http::{Method, Version}; +use tokio_util::codec::{Decoder, Encoder}; use super::{ decoder::{self, PayloadDecoder, PayloadItem, PayloadType}, diff --git a/actix-http/src/h1/dispatcher.rs b/actix-http/src/h1/dispatcher.rs index 81090667d..60660b85b 100644 --- a/actix-http/src/h1/dispatcher.rs +++ b/actix-http/src/h1/dispatcher.rs @@ -8,13 +8,15 @@ use std::{ task::{Context, Poll}, }; -use actix_codec::{AsyncRead, AsyncWrite, Decoder as _, Encoder as _, Framed, FramedParts}; +use actix_codec::{Framed, FramedParts}; use actix_rt::time::sleep_until; use actix_service::Service; use bitflags::bitflags; use bytes::{Buf, BytesMut}; use futures_core::ready; use pin_project_lite::pin_project; +use tokio::io::{AsyncRead, AsyncWrite}; +use tokio_util::codec::{Decoder as _, Encoder as _}; use tracing::{error, trace}; use crate::{ @@ -1004,7 +1006,7 @@ where this.read_buf.reserve(HW_BUFFER_SIZE - remaining); } - match actix_codec::poll_read_buf(io.as_mut(), cx, this.read_buf) { + match tokio_util::io::poll_read_buf(io.as_mut(), cx, this.read_buf) { Poll::Ready(Ok(n)) => { this.flags.remove(Flags::FINISHED); diff --git a/actix-http/src/ws/codec.rs b/actix-http/src/ws/codec.rs index 4a2e741b6..6a149f9a4 100644 --- a/actix-http/src/ws/codec.rs +++ b/actix-http/src/ws/codec.rs @@ -1,7 +1,7 @@ -use actix_codec::{Decoder, Encoder}; use bitflags::bitflags; use bytes::{Bytes, BytesMut}; use bytestring::ByteString; +use tokio_util::codec::{Decoder, Encoder}; use tracing::error; use super::{ diff --git a/actix-http/src/ws/dispatcher.rs b/actix-http/src/ws/dispatcher.rs index 2f6b2363b..396f1e86c 100644 --- a/actix-http/src/ws/dispatcher.rs +++ b/actix-http/src/ws/dispatcher.rs @@ -76,7 +76,9 @@ mod inner { use pin_project_lite::pin_project; use tracing::debug; - use actix_codec::{AsyncRead, AsyncWrite, Decoder, Encoder, Framed}; + use actix_codec::Framed; + use tokio::io::{AsyncRead, AsyncWrite}; + use tokio_util::codec::{Decoder, Encoder}; use crate::{body::BoxBody, Response}; diff --git a/actix-web-actors/Cargo.toml b/actix-web-actors/Cargo.toml index 8222fc864..26b1c09de 100644 --- a/actix-web-actors/Cargo.toml +++ b/actix-web-actors/Cargo.toml @@ -24,6 +24,7 @@ bytestring = "1" futures-core = { version = "0.3.7", default-features = false } pin-project-lite = "0.2" tokio = { version = "1.13.1", features = ["sync"] } +tokio-util = { version = "0.7", features = ["codec"] } [dev-dependencies] actix-rt = "2.2" diff --git a/actix-web-actors/src/ws.rs b/actix-web-actors/src/ws.rs index 9a4880159..e1110eddb 100644 --- a/actix-web-actors/src/ws.rs +++ b/actix-web-actors/src/ws.rs @@ -74,7 +74,6 @@ use actix::{ Actor, ActorContext, ActorState, Addr, AsyncContext, Handler, Message as ActixMessage, SpawnHandle, }; -use actix_codec::{Decoder as _, Encoder as _}; use actix_http::ws::{hash_key, Codec}; pub use actix_http::ws::{ CloseCode, CloseReason, Frame, HandshakeError, Message, ProtocolError, @@ -92,6 +91,7 @@ use bytestring::ByteString; use futures_core::Stream; use pin_project_lite::pin_project; use tokio::sync::oneshot; +use tokio_util::codec::{Decoder as _, Encoder as _}; /// Builder for Websocket session response. /// diff --git a/actix-web/src/http/header/accept.rs b/actix-web/src/http/header/accept.rs index 744c9b6e8..1be136b19 100644 --- a/actix-web/src/http/header/accept.rs +++ b/actix-web/src/http/header/accept.rs @@ -6,8 +6,7 @@ use super::{common_header, QualityItem}; use crate::http::header; common_header! { - /// `Accept` header, defined - /// in [RFC 7231 §5.3.2](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2) + /// `Accept` header, defined in [RFC 7231 §5.3.2]. /// /// The `Accept` header field can be used by user agents to specify /// response media types that are acceptable. Accept header fields can @@ -71,6 +70,8 @@ common_header! { /// ]) /// ); /// ``` + /// + /// [RFC 7231 §5.3.2]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 (Accept, header::ACCEPT) => (QualityItem)* test_parse_and_format { @@ -101,13 +102,12 @@ common_header! { vec![b"text/plain; charset=utf-8"], Some(Accept(vec![ QualityItem::max(mime::TEXT_PLAIN_UTF_8), - ]))); + ]))); crate::http::header::common_header_test!( test4, vec![b"text/plain; charset=utf-8; q=0.5"], Some(Accept(vec![ - QualityItem::new(mime::TEXT_PLAIN_UTF_8, - q(0.5)), + QualityItem::new(mime::TEXT_PLAIN_UTF_8, q(0.5)), ]))); #[test] From 45b77c68195eb933231290a09e9f6a0cca56aad8 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 4 Nov 2022 02:42:22 +0200 Subject: [PATCH 15/15] GitHub Workflows security hardening (#2923) --- .github/workflows/bench.yml | 3 +++ .github/workflows/ci-post-merge.yml | 3 +++ .github/workflows/ci.yml | 3 +++ .github/workflows/upload-doc.yml | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index a4b54ca7a..008c33f89 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -5,6 +5,9 @@ on: branches: - master +permissions: + contents: read # to fetch code (actions/checkout) + jobs: check_benchmark: runs-on: ubuntu-latest diff --git a/.github/workflows/ci-post-merge.yml b/.github/workflows/ci-post-merge.yml index 1ee97b591..6d76301d8 100644 --- a/.github/workflows/ci-post-merge.yml +++ b/.github/workflows/ci-post-merge.yml @@ -4,6 +4,9 @@ on: push: branches: [master] +permissions: + contents: read # to fetch code (actions/checkout) + jobs: build_and_test_nightly: strategy: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index de1e1fe18..07e21ef43 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: push: branches: [master] +permissions: + contents: read # to fetch code (actions/checkout) + jobs: build_and_test: strategy: diff --git a/.github/workflows/upload-doc.yml b/.github/workflows/upload-doc.yml index c47ea1d70..ac181b3f9 100644 --- a/.github/workflows/upload-doc.yml +++ b/.github/workflows/upload-doc.yml @@ -4,8 +4,12 @@ on: push: branches: [master] +permissions: {} jobs: build: + permissions: + contents: write # to push changes in repo (jamesives/github-pages-deploy-action) + runs-on: ubuntu-latest steps: