Merge branch 'main' into feat/encoding-as-extension

actix-files/CHANGES.md

@@ -3,6 +3,9 @@
 ## Unreleased
 
 - Minimum supported Rust version (MSRV) is now 1.88.
+- `PathBufWrap` & `UriSegmentError` made public. [#3694]
+
+[#3694]: https://github.com/actix/actix-web/pull/3694
 
 ## 0.6.9
 

actix-files/src/error.rs

@@ -21,6 +21,7 @@ impl ResponseError for FilesError {
     }
 }
 
+/// Error which can occur with parsing/validating a request-uri path
 #[derive(Debug, PartialEq, Eq, Display)]
 #[non_exhaustive]
 pub enum UriSegmentError {

actix-files/src/lib.rs

@@ -37,13 +37,12 @@ mod range;
 mod service;
 
 pub use self::{
-    chunked::ChunkedReadFile, directory::Directory, files::Files, named::NamedFile,
+    chunked::ChunkedReadFile, directory::Directory, error::UriSegmentError, files::Files,
-    range::HttpRange, service::FilesService,
+    named::NamedFile, path_buf::PathBufWrap, range::HttpRange, service::FilesService,
 };
 use self::{
     directory::{directory_listing, DirectoryRenderer},
     error::FilesError,
-    path_buf::PathBufWrap,
 };
 
 type HttpService = BoxService<ServiceRequest, ServiceResponse, Error>;

actix-files/src/path_buf.rs

@@ -8,8 +8,11 @@ use actix_web::{dev::Payload, FromRequest, HttpRequest};
 
 use crate::error::UriSegmentError;
 
+/// Secure Path Traversal Guard
+///
+/// This struct parses a request-uri [`PathBuf`](std::path::PathBuf)
 #[derive(Debug, PartialEq, Eq)]
-pub(crate) struct PathBufWrap(PathBuf);
+pub struct PathBufWrap(PathBuf);
 
 impl FromStr for PathBufWrap {
     type Err = UriSegmentError;
@@ -20,6 +23,37 @@ impl FromStr for PathBufWrap {
 }
 
 impl PathBufWrap {
+    /// Parse a safe path from the unprocessed tail of a supplied
+    /// [`HttpRequest`](actix_web::HttpRequest), given the choice of allowing hidden files to be
+    /// considered valid segments.
+    ///
+    /// This uses [`HttpRequest::match_info`](actix_web::HttpRequest::match_info) and
+    /// [`Path::unprocessed`](actix_web::dev::Path::unprocessed), which returns the part of the
+    /// path not matched by route patterns. This is useful for mounted services (eg. `Files`),
+    /// where only the tail should be parsed.
+    ///
+    /// Path traversal is guarded by this method.
+    #[inline]
+    pub fn parse_unprocessed_req(
+        req: &HttpRequest,
+        hidden_files: bool,
+    ) -> Result<Self, UriSegmentError> {
+        Self::parse_path(req.match_info().unprocessed(), hidden_files)
+    }
+
+    /// Parse a safe path from the full request path of a supplied
+    /// [`HttpRequest`](actix_web::HttpRequest), given the choice of allowing hidden files to be
+    /// considered valid segments.
+    ///
+    /// This uses [`HttpRequest::path`](actix_web::HttpRequest::path), and is more appropriate
+    /// for non-mounted handlers that want the entire request path.
+    ///
+    /// Path traversal is guarded by this method.
+    #[inline]
+    pub fn parse_req_path(req: &HttpRequest, hidden_files: bool) -> Result<Self, UriSegmentError> {
+        Self::parse_path(req.path(), hidden_files)
+    }
+
     /// Parse a path, giving the choice of allowing hidden files to be considered valid segments.
     ///
     /// Path traversal is guarded by this method.
@@ -91,6 +125,7 @@ impl FromRequest for PathBufWrap {
     type Future = Ready<Result<Self, Self::Error>>;
 
     fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future {
+        // Uses the unprocessed tail of the request path and disallows hidden files.
         ready(req.match_info().unprocessed().parse())
     }
 }

actix-http/CHANGES.md

@@ -3,6 +3,9 @@
 ## Unreleased
 
 - Minimum supported Rust version (MSRV) is now 1.88.
+- Fix truncated body ending without error when connection closed abnormally. [#3067]
+
+[#3067]: https://github.com/actix/actix-web/pull/3067
 
 ## 3.11.2
 

actix-http/src/h1/dispatcher.rs

@@ -1156,6 +1156,7 @@ where
                         let inner = inner.as_mut().project();
                         inner.flags.insert(Flags::READ_DISCONNECT);
                         if let Some(mut payload) = inner.payload.take() {
+                            payload.set_error(PayloadError::Incomplete(None));
                             payload.feed_eof();
                         }
                     };

actix-http/tests/test_rustls.rs

@@ -51,7 +51,7 @@ where
     Ok(buf)
 }
 
-fn tls_config() -> RustlsServerConfig {
+fn tls_config_with_alpn(protocols: &[&[u8]]) -> RustlsServerConfig {
     let rcgen::CertifiedKey { cert, key_pair } =
         rcgen::generate_simple_self_signed(["localhost".to_owned()]).unwrap();
     let cert_chain = vec![cert.der().clone()];
@@ -62,12 +62,23 @@ fn tls_config() -> RustlsServerConfig {
         .with_single_cert(cert_chain, key_der)
         .unwrap();
 
-    config.alpn_protocols.push(HTTP1_1_ALPN_PROTOCOL.to_vec());
+    config.alpn_protocols = protocols.iter().map(|proto| proto.to_vec()).collect();
-    config.alpn_protocols.push(H2_ALPN_PROTOCOL.to_vec());
 
     config
 }
 
+fn tls_config() -> RustlsServerConfig {
+    tls_config_with_alpn(&[HTTP1_1_ALPN_PROTOCOL, H2_ALPN_PROTOCOL])
+}
+
+fn tls_config_h1() -> RustlsServerConfig {
+    tls_config_with_alpn(&[HTTP1_1_ALPN_PROTOCOL])
+}
+
+fn tls_config_h2() -> RustlsServerConfig {
+    tls_config_with_alpn(&[H2_ALPN_PROTOCOL])
+}
+
 pub fn get_negotiated_alpn_protocol(
     addr: SocketAddr,
     client_alpn_protocol: &[u8],
@@ -98,7 +109,7 @@ async fn h1() -> io::Result<()> {
     let srv = test_server(move || {
         HttpService::build()
             .h1(|_| ok::<_, Error>(Response::ok()))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h1())
     })
     .await;
 
@@ -112,7 +123,7 @@ async fn h2() -> io::Result<()> {
     let srv = test_server(move || {
         HttpService::build()
             .h2(|_| ok::<_, Error>(Response::ok()))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -130,7 +141,7 @@ async fn h1_1() -> io::Result<()> {
                 assert_eq!(req.version(), Version::HTTP_11);
                 ok::<_, Error>(Response::ok())
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h1())
     })
     .await;
 
@@ -149,7 +160,7 @@ async fn h2_1() -> io::Result<()> {
                 ok::<_, Error>(Response::ok())
             })
             .rustls_0_23_with_config(
-                tls_config(),
+                tls_config_h2(),
                 TlsAcceptorConfig::default().handshake_timeout(Duration::from_secs(5)),
             )
     })
@@ -169,7 +180,7 @@ async fn h2_body1() -> io::Result<()> {
                 let body = load_body(req.take_payload()).await?;
                 Ok::<_, Error>(Response::ok().set_body(body))
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -195,7 +206,7 @@ async fn h2_content_length() {
                 ];
                 ok::<_, Infallible>(Response::new(statuses[indx]))
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -267,7 +278,7 @@ async fn h2_headers() {
                 }
                 ok::<_, Infallible>(config.body(data.clone()))
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -306,7 +317,7 @@ async fn h2_body2() {
     let mut srv = test_server(move || {
         HttpService::build()
             .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR)))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -323,7 +334,7 @@ async fn h2_head_empty() {
     let mut srv = test_server(move || {
         HttpService::build()
             .finish(|_| ok::<_, Infallible>(Response::ok().set_body(STR)))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -349,7 +360,7 @@ async fn h2_head_binary() {
     let mut srv = test_server(move || {
         HttpService::build()
             .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR)))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -374,7 +385,7 @@ async fn h2_head_binary2() {
     let srv = test_server(move || {
         HttpService::build()
             .h2(|_| ok::<_, Infallible>(Response::ok().set_body(STR)))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -400,7 +411,7 @@ async fn h2_body_length() {
                     Response::ok().set_body(SizedStream::new(STR.len() as u64, body)),
                 )
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -424,7 +435,7 @@ async fn h2_body_chunked_explicit() {
                         .body(BodyStream::new(body)),
                 )
             })
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -453,7 +464,7 @@ async fn h2_response_http_error_handling() {
                     )
                 }))
             }))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -483,7 +494,7 @@ async fn h2_service_error() {
     let mut srv = test_server(move || {
         HttpService::build()
             .h2(|_| err::<Response<BoxBody>, _>(BadRequest))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h2())
     })
     .await;
 
@@ -500,7 +511,7 @@ async fn h1_service_error() {
     let mut srv = test_server(move || {
         HttpService::build()
             .h1(|_| err::<Response<BoxBody>, _>(BadRequest))
-            .rustls_0_23(tls_config())
+            .rustls_0_23(tls_config_h1())
     })
     .await;
 
@@ -519,7 +530,7 @@ const CUSTOM_ALPN_PROTOCOL: &[u8] = b"custom";
 #[actix_rt::test]
 async fn alpn_h1() -> io::Result<()> {
     let srv = test_server(move || {
-        let mut config = tls_config();
+        let mut config = tls_config_h1();
         config.alpn_protocols.push(CUSTOM_ALPN_PROTOCOL.to_vec());
         HttpService::build()
             .h1(|_| ok::<_, Error>(Response::ok()))
@@ -541,7 +552,7 @@ async fn alpn_h1() -> io::Result<()> {
 #[actix_rt::test]
 async fn alpn_h2() -> io::Result<()> {
     let srv = test_server(move || {
-        let mut config = tls_config();
+        let mut config = tls_config_h2();
         config.alpn_protocols.push(CUSTOM_ALPN_PROTOCOL.to_vec());
         HttpService::build()
             .h2(|_| ok::<_, Error>(Response::ok()))

actix-http/tests/test_server.rs

@@ -443,6 +443,60 @@ async fn content_length() {
     srv.stop().await;
 }
 
+#[actix_rt::test]
+async fn content_length_truncated() {
+    use tokio::io::{AsyncReadExt, AsyncWriteExt};
+
+    let mut srv = test_server(|| {
+        HttpService::build()
+            .h1(|mut req: Request| async move {
+                let expected_length: usize = req.uri().path()[1..].parse().unwrap();
+                let mut payload = req.take_payload();
+
+                let mut length = 0;
+                let mut seen_error = false;
+                while let Some(chunk) = payload.next().await {
+                    match chunk {
+                        Ok(b) => length += b.len(),
+                        Err(_) => {
+                            seen_error = true;
+                            break;
+                        }
+                    }
+                }
+                if seen_error {
+                    return Result::<_, Infallible>::Ok(Response::bad_request());
+                }
+
+                assert_eq!(length, expected_length, "length must match when no error");
+                Result::<_, Infallible>::Ok(Response::ok())
+            })
+            .tcp()
+    })
+    .await;
+
+    let addr = srv.addr();
+    let mut buf = [0; 12];
+
+    let mut conn = TcpStream::connect(&addr).await.unwrap();
+    conn.write_all(b"POST /10000 HTTP/1.1\r\nContent-Length: 10000\r\n\r\ndata_truncated")
+        .await
+        .unwrap();
+    conn.shutdown().await.unwrap();
+    conn.read_exact(&mut buf).await.unwrap();
+    assert_eq!(&buf, b"HTTP/1.1 400");
+
+    let mut conn = TcpStream::connect(&addr).await.unwrap();
+    conn.write_all(b"POST /4 HTTP/1.1\r\nContent-Length: 4\r\n\r\ndata")
+        .await
+        .unwrap();
+    conn.shutdown().await.unwrap();
+    conn.read_exact(&mut buf).await.unwrap();
+    assert_eq!(&buf, b"HTTP/1.1 200");
+
+    srv.stop().await;
+}
+
 #[actix_rt::test]
 async fn h1_headers() {
     let data = STR.repeat(10);