Repository: actix/actix-web (language: rust)

Issue:
actix-files: Can't handle files with newlines in them
It seems that weirdly enough no one hit this yet until it was report here: https://github.com/svenstaro/miniserve/issues/1294
Newlines in files are legal in some filesystems and actix-files should handle it which it currently doesn't do.

## Expected Behavior

A file with a newline (for instance created like `touch test$'\n'newline.binary`) should be servable by actix-files.

## Current Behavior

It results in a 500 with `failed to parse header value`.

## Possible Solution

<!--- Not obligatory, but suggest a fix/reason for the bug, -->
<!--- or ideas how to implement the addition or change -->

## Steps to Reproduce (for bugs)

I added a failing test case here: https://github.com/svenstaro/actix-web/tree/actix-files-newline

## Context

It was reported in miniserve here: https://github.com/svenstaro/miniserve/issues/1294


Discussion / hints:

Fix patch:
diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md
index 15c2958f0fb..81e361a2120 100644
--- a/actix-files/CHANGES.md
+++ b/actix-files/CHANGES.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 - Minimum supported Rust version (MSRV) is now 1.68 due to transitive `time` dependency.
+- Properly handle newlines in filenames. [#3235]
 
 ## 0.6.3
 
diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs
index 943130e16fa..87914c1cef9 100644
--- a/actix-files/src/lib.rs
+++ b/actix-files/src/lib.rs
@@ -568,6 +568,26 @@ mod tests {
         assert_eq!(bytes, data);
     }
 
+    #[actix_rt::test]
+    async fn test_static_files_with_newlines() {
+        // Create the file we want to test against ad-hoc. We can't check it in as otherwise
+        // Windows can't even checkout this repository.
+        let tmpdir = tempfile::tempdir().unwrap();
+        let file_with_newlines = tmpdir.path().join("test\nnewline.text");
+        fs::write(&file_with_newlines, "Look at my newlines").unwrap();
+        let srv = test::init_service(
+            App::new().service(Files::new("", tmpdir.path()).index_file("Cargo.toml")),
+        )
+        .await;
+        let request = TestRequest::get().uri("/test%0Anewline.text").to_request();
+        let response = test::call_service(&srv, request).await;
+        assert_eq!(response.status(), StatusCode::OK);
+
+        let bytes = test::read_body(response).await;
+        let data = web::Bytes::from(fs::read(file_with_newlines).unwrap());
+        assert_eq!(bytes, data);
+    }
+
     #[actix_rt::test]
     async fn test_files_not_allowed() {
         let srv = test::init_service(App::new().service(Files::new("/", "."))).await;
@@ -842,7 +862,7 @@ mod tests {
     async fn test_percent_encoding_2() {
         let tmpdir = tempfile::tempdir().unwrap();
         let filename = match cfg!(unix) {
-            true => "ض:?#[]{}<>()@!$&'`|*+,;= %20.test",
+            true => "ض:?#[]{}<>()@!$&'`|*+,;= %20\n.test",
             false => "ض#[]{}()@!$&'`+,;= %20.test",
         };
         let filename_encoded = filename
diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs
index d7795ba73e7..02dc701ea03 100644
--- a/actix-files/src/named.rs
+++ b/actix-files/src/named.rs
@@ -24,7 +24,6 @@ use bitflags::bitflags;
 use derive_more::{Deref, DerefMut};
 use futures_core::future::LocalBoxFuture;
 use mime::Mime;
-use mime_guess::from_path;
 
 use crate::{encoding::equiv_utf8_text, range::HttpRange};
 
@@ -128,7 +127,7 @@ impl NamedFile {
                 }
             };
 
-            let ct = from_path(&path).first_or_octet_stream();
+            let ct = mime_guess::from_path(&path).first_or_octet_stream();
 
             let disposition = match ct.type_() {
                 mime::IMAGE | mime::TEXT | mime::AUDIO | mime::VIDEO => DispositionType::Inline,
@@ -140,7 +139,9 @@ impl NamedFile {
                 _ => DispositionType::Attachment,
             };
 
-            let mut parameters = vec![DispositionParam::Filename(String::from(filename.as_ref()))];
+            // Replace newlines in filenames which could occur on some filesystems.
+            let filename_s = filename.replace('\n', "%0A");
+            let mut parameters = vec![DispositionParam::Filename(filename_s)];
 
             if !filename.is_ascii() {
                 parameters.push(DispositionParam::FilenameExt(ExtendedValue {


Test patch: