From 20f4cfe6b58cd409252de906944352ea80f1dab6 Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Sat, 23 Jul 2022 17:27:01 +0200
Subject: [PATCH] fix partial ranges for video content (#2817)

fixes #2815
---
 actix-files/CHANGES.md        |  3 +++
 actix-files/src/named.rs      | 25 ++++++++++++++++++++-----
 actix-files/tests/encoding.rs | 32 ++++++++++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md
index b127cd9ea..dea26269b 100644
--- a/actix-files/CHANGES.md
+++ b/actix-files/CHANGES.md
@@ -1,8 +1,11 @@
 # Changes
 
 ## Unreleased - 2022-xx-xx
+- Allow partial range responses for video content to start streaming sooner. [#2817]
 - Minimum supported Rust version (MSRV) is now 1.57 due to transitive `time` dependency.
 
+[#2817]: https://github.com/actix/actix-web/pull/2817
+
 
 ## 0.6.1 - 2022-06-11
 - Add `NamedFile::{modified, metadata, content_type, content_disposition, encoding}()` getters. [#2021]
diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs
index 5580e6f7e..1213534c2 100644
--- a/actix-files/src/named.rs
+++ b/actix-files/src/named.rs
@@ -528,11 +528,26 @@ impl NamedFile {
                     length = ranges[0].length;
                     offset = ranges[0].start;
 
-                    // don't allow compression middleware to modify partial content
-                    res.insert_header((
-                        header::CONTENT_ENCODING,
-                        HeaderValue::from_static("identity"),
-                    ));
+                    // When a Content-Encoding header is present in a 206 partial content response
+                    // for video content, it prevents browser video players from starting playback
+                    // before loading the whole video and also prevents seeking.
+                    //
+                    // See: https://github.com/actix/actix-web/issues/2815
+                    //
+                    // The assumption of this fix is that the video player knows to not send an
+                    // Accept-Encoding header for this request and that downstream middleware will
+                    // not attempt compression for requests without it.
+                    //
+                    // TODO: Solve question around what to do if self.encoding is set and partial
+                    // range is requested. Reject request? Ignoring self.encoding seems wrong, too.
+                    // In practice, it should not come up.
+                    if req.headers().contains_key(&header::ACCEPT_ENCODING) {
+                        // don't allow compression middleware to modify partial content
+                        res.insert_header((
+                            header::CONTENT_ENCODING,
+                            HeaderValue::from_static("identity"),
+                        ));
+                    }
 
                     res.insert_header((
                         header::CONTENT_RANGE,
diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs
index 080292af5..7aec25ff9 100644
--- a/actix-files/tests/encoding.rs
+++ b/actix-files/tests/encoding.rs
@@ -1,11 +1,11 @@
-use actix_files::Files;
+use actix_files::{Files, NamedFile};
 use actix_web::{
     http::{
         header::{self, HeaderValue},
         StatusCode,
     },
     test::{self, TestRequest},
-    App,
+    web, App,
 };
 
 #[actix_web::test]
@@ -36,3 +36,31 @@ async fn test_utf8_file_contents() {
         Some(&HeaderValue::from_static("text/plain")),
     );
 }
+
+#[actix_web::test]
+async fn partial_range_response_encoding() {
+    let srv = test::init_service(App::new().default_service(web::to(|| async {
+        NamedFile::open_async("./tests/test.binary").await.unwrap()
+    })))
+    .await;
+
+    // range request without accept-encoding returns no content-encoding header
+    let req = TestRequest::with_uri("/")
+        .append_header((header::RANGE, "bytes=10-20"))
+        .to_request();
+    let res = test::call_service(&srv, req).await;
+    assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT);
+    assert!(!res.headers().contains_key(header::CONTENT_ENCODING));
+
+    // range request with accept-encoding returns a content-encoding header
+    let req = TestRequest::with_uri("/")
+        .append_header((header::RANGE, "bytes=10-20"))
+        .append_header((header::ACCEPT_ENCODING, "identity"))
+        .to_request();
+    let res = test::call_service(&srv, req).await;
+    assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT);
+    assert_eq!(
+        res.headers().get(header::CONTENT_ENCODING).unwrap(),
+        "identity"
+    );
+}