From 5d0e8138eeab63647e1e36001e6ae8e2a8ac3722 Mon Sep 17 00:00:00 2001 From: e-rhodes <33500135+e-rhodes@users.noreply.github.com> Date: Wed, 22 Jun 2022 14:02:03 -0600 Subject: [PATCH 1/3] Add getters for `&ServiceRequest` (#2786) --- actix-web/CHANGES.md | 3 +++ actix-web/src/service.rs | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index fb27cddfd..3fa2f8f21 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -2,7 +2,10 @@ ## Unreleased - 2022-xx-xx - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +### Added +- Add `ServiceRequest::{parts, request}()` getter methods. [#2786] +[#2786]: https://github.com/actix/actix-web/pull/2786 ## 4.1.0 - 2022-06-11 ### Added diff --git a/actix-web/src/service.rs b/actix-web/src/service.rs index a9e809bf9..0ad92d8a1 100644 --- a/actix-web/src/service.rs +++ b/actix-web/src/service.rs @@ -95,6 +95,18 @@ impl ServiceRequest { (&mut self.req, &mut self.payload) } + /// Returns immutable accessors to inner parts. + #[inline] + pub fn parts(&self) -> (&HttpRequest, &Payload) { + (&self.req, &self.payload) + } + + /// Returns immutable accessor to inner [`HttpRequest`]. + #[inline] + pub fn request(&self) -> &HttpRequest { + &self.req + } + /// Derives a type from this request using an [extractor](crate::FromRequest). /// /// Returns the `T` extractor's `Future` type which can be `await`ed. This is particularly handy From de92b3be2e6894c353816c6bd29d2a4768ccb92f Mon Sep 17 00:00:00 2001 From: oatoam <31235342+oatoam@users.noreply.github.com> Date: Fri, 24 Jun 2022 11:46:17 +0800 Subject: [PATCH 2/3] fix unrecoverable Err(Overflow) in websocket frame parser (#2790) --- actix-http/CHANGES.md | 6 +++++ actix-http/src/ws/frame.rs | 49 +++++++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 209f86cad..dd6051b85 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,8 +1,14 @@ # Changes ## Unreleased - 2022-xx-xx +### Fixed +- Websocket parser no longer throws endless overflow errors after receiving an oversized frame. [#2790] + +### Changed - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency. +[#2790]: https://github.com/actix/actix-web/pull/2790 + ## 3.1.0 - 2022-06-11 ### Changed diff --git a/actix-http/src/ws/frame.rs b/actix-http/src/ws/frame.rs index 17e34e2ba..3659b6c3b 100644 --- a/actix-http/src/ws/frame.rs +++ b/actix-http/src/ws/frame.rs @@ -17,7 +17,6 @@ impl Parser { fn parse_metadata( src: &[u8], server: bool, - max_size: usize, ) -> Result)>, ProtocolError> { let chunk_len = src.len(); @@ -60,20 +59,12 @@ impl Parser { return Ok(None); } let len = u64::from_be_bytes(TryFrom::try_from(&src[idx..idx + 8]).unwrap()); - if len > max_size as u64 { - return Err(ProtocolError::Overflow); - } idx += 8; len as usize } else { len as usize }; - // check for max allowed size - if length > max_size { - return Err(ProtocolError::Overflow); - } - let mask = if server { if chunk_len < idx + 4 { return Ok(None); @@ -98,11 +89,10 @@ impl Parser { max_size: usize, ) -> Result)>, ProtocolError> { // try to parse ws frame metadata - let (idx, finished, opcode, length, mask) = - match Parser::parse_metadata(src, server, max_size)? { - None => return Ok(None), - Some(res) => res, - }; + let (idx, finished, opcode, length, mask) = match Parser::parse_metadata(src, server)? { + None => return Ok(None), + Some(res) => res, + }; // not enough data if src.len() < idx + length { @@ -112,6 +102,13 @@ impl Parser { // remove prefix src.advance(idx); + // check for max allowed size + if length > max_size { + // drop the payload + src.advance(length); + return Err(ProtocolError::Overflow); + } + // no need for body if length == 0 { return Ok(Some((finished, opcode, None))); @@ -339,6 +336,30 @@ mod tests { } } + #[test] + 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]); + // Next binary frame with length == 2 and payload == `[0x1111_1111u8, 0x1111_1111u8]`. + buf.extend(&[0b0000_0010u8, 0b0000_0010u8, 0b1111_1111u8, 0b1111_1111u8]); + + assert_eq!(buf.len(), 8); + assert!(matches!( + Parser::parse(&mut buf, false, 1), + Err(ProtocolError::Overflow) + )); + assert_eq!(buf.len(), 4); + let frame = extract(Parser::parse(&mut buf, false, 2)); + assert!(!frame.finished); + assert_eq!(frame.opcode, OpCode::Binary); + assert_eq!( + frame.payload, + Bytes::from(vec![0b1111_1111u8, 0b1111_1111u8]) + ); + assert_eq!(buf.len(), 0); + } + #[test] fn test_ping_frame() { let mut buf = BytesMut::new(); From 8dbf7da89feb4f2127f72b97b9d51b7e7847a067 Mon Sep 17 00:00:00 2001 From: PeterPierinakos <101414157+PeterPierinakos@users.noreply.github.com> Date: Sat, 25 Jun 2022 14:01:06 +0000 Subject: [PATCH 3/3] Fix common grammar mistakes and add small documentation for AppConfig's Default implementation (#2793) --- actix-web/src/app.rs | 2 +- actix-web/src/app_service.rs | 2 +- actix-web/src/config.rs | 10 ++++++++++ actix-web/src/guard.rs | 4 ++-- actix-web/src/handler.rs | 2 +- actix-web/src/response/response.rs | 2 +- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/actix-web/src/app.rs b/actix-web/src/app.rs index 119980a03..213c8beff 100644 --- a/actix-web/src/app.rs +++ b/actix-web/src/app.rs @@ -60,7 +60,7 @@ where /// [`HttpRequest::app_data`](crate::HttpRequest::app_data) method at runtime. /// /// # [`Data`] - /// Any [`Data`] type added here can utilize it's extractor implementation in handlers. + /// Any [`Data`] type added here can utilize its extractor implementation in handlers. /// Types not wrapped in `Data` cannot use this extractor. See [its docs](Data) for more /// about its usage and patterns. /// diff --git a/actix-web/src/app_service.rs b/actix-web/src/app_service.rs index 3ef31ac75..28ff8c614 100644 --- a/actix-web/src/app_service.rs +++ b/actix-web/src/app_service.rs @@ -257,7 +257,7 @@ impl ServiceFactory for AppRoutingFactory { type Future = LocalBoxFuture<'static, Result>; fn new_service(&self, _: ()) -> Self::Future { - // construct all services factory future with it's resource def and guards. + // construct all services factory future with its resource def and guards. let factory_fut = join_all(self.services.iter().map(|(path, factory, guards)| { let path = path.clone(); let guards = guards.borrow_mut().take().unwrap_or_default(); diff --git a/actix-web/src/config.rs b/actix-web/src/config.rs index dab309175..58a099c75 100644 --- a/actix-web/src/config.rs +++ b/actix-web/src/config.rs @@ -153,6 +153,16 @@ impl AppConfig { } impl Default for AppConfig { + /// Returns the default AppConfig. + /// Note: The included socket address is "127.0.0.1". + /// + /// 127.0.0.1: non-routable meta address that denotes an unknown, invalid or non-applicable target. + /// If you need a service only accessed by itself, use a loopback address. + /// A loopback address for IPv4 is any loopback address that begins with "127". + /// Loopback addresses should be only used to test your application locally. + /// The default configuration provides a loopback address. + /// + /// 0.0.0.0: if configured to use this special address, the application will listen to any IP address configured on the machine. fn default() -> Self { AppConfig::new( false, diff --git a/actix-web/src/guard.rs b/actix-web/src/guard.rs index 9f7514644..ef1301075 100644 --- a/actix-web/src/guard.rs +++ b/actix-web/src/guard.rs @@ -254,7 +254,7 @@ impl Guard for AllGuard { } } -/// Wraps a guard and inverts the outcome of it's `Guard` implementation. +/// Wraps a guard and inverts the outcome of its `Guard` implementation. /// /// # Examples /// The handler below will be called for any request method apart from `GET`. @@ -459,7 +459,7 @@ impl Guard for HostGuard { return scheme == req_host_uri_scheme; } - // TODO: is the the correct behavior? + // TODO: is this the correct behavior? // falls through if scheme cannot be determined } diff --git a/actix-web/src/handler.rs b/actix-web/src/handler.rs index cf86cb38b..522a48b82 100644 --- a/actix-web/src/handler.rs +++ b/actix-web/src/handler.rs @@ -37,7 +37,7 @@ use crate::{ /// Thanks to Rust's type system, Actix Web can infer the function parameter types. During the /// extraction step, the parameter types are described as a tuple type, [`from_request`] is run on /// that tuple, and the `Handler::call` implementation for that particular function arity -/// destructures the tuple into it's component types and calls your handler function with them. +/// destructures the tuple into its component types and calls your handler function with them. /// /// In pseudo-code the process looks something like this: /// ```ignore diff --git a/actix-web/src/response/response.rs b/actix-web/src/response/response.rs index 630acc3f2..ead8badba 100644 --- a/actix-web/src/response/response.rs +++ b/actix-web/src/response/response.rs @@ -343,7 +343,7 @@ mod response_fut_impl { // Future is only implemented for BoxBody payload type because it's the most useful for making // simple handlers without async blocks. Making it generic over all MessageBody types requires a - // future impl on Response which would cause it's body field to be, undesirably, Option. + // future impl on Response which would cause its body field to be, undesirably, Option. // // This impl is not particularly efficient due to the Response construction and should probably // not be invoked if performance is important. Prefer an async fn/block in such cases.