fix(files,http,web): do not compress 206 at all (#3923)

actix-files/CHANGES.md

@@ -5,9 +5,11 @@
 - Add `Files::try_compressed()` to support serving pre-compressed static files [#2615]
 - Fix handling of `bytes=0-`
 - Fix `NamedFile` panic when serving files with pre-UNIX epoch modification times. [#2748]
+- Fix invalid `Content-Encoding: identity` header in `NamedFile` range responses. [#3191]
 
 [#2615]: https://github.com/actix/actix-web/pull/2615
 [#2748]: https://github.com/actix/actix-web/issues/2748
+[#3191]: https://github.com/actix/actix-web/issues/3191
 
 ## 0.6.10
 

actix-files/src/named.rs

@@ -14,7 +14,7 @@ use actix_web::{
     http::{
         header::{
             self, Charset, ContentDisposition, ContentEncoding, DispositionParam, DispositionType,
-            ExtendedValue, HeaderValue,
+            ExtendedValue,
         },
         StatusCode,
     },
@@ -593,27 +593,6 @@ impl NamedFile {
                     length = range.length;
                     offset = range.start;
 
-                    // 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,
                         format!("bytes {}-{}/{}", offset, offset + length - 1, self.md.len()),

actix-files/tests/encoding.rs

@@ -181,15 +181,12 @@ async fn partial_range_response_encoding() {
     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
+    // range request with accept-encoding still returns no content-encoding header
     let req = TestRequest::with_uri("/")
         .append_header((header::RANGE, "bytes=10-20"))
-        .append_header((header::ACCEPT_ENCODING, "identity"))
+        .append_header((header::ACCEPT_ENCODING, "gzip"))
         .to_request();
     let res = test::call_service(&srv, req).await;
     assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT);
-    assert_eq!(
+    assert!(!res.headers().contains_key(header::CONTENT_ENCODING));
-        res.headers().get(header::CONTENT_ENCODING).unwrap(),
-        "identity"
-    );
 }

actix-http/CHANGES.md

@@ -5,9 +5,11 @@
 - Minimum supported Rust version (MSRV) is now 1.88.
 - Fix truncated body ending without error when connection closed abnormally. [#3067]
 - Add config/method for `TCP_NODELAY`. [#3918]
+- Do not compress 206 Partial Content responses. [#3191]
 
 [#3067]: https://github.com/actix/actix-web/pull/3067
 [#3918]: https://github.com/actix/actix-web/pull/3918
+[#3191]: https://github.com/actix/actix-web/issues/3191
 
 ## 3.11.2
 

actix-http/src/encoding/encoder.rs

@@ -70,6 +70,7 @@ impl<B: MessageBody> Encoder<B> {
         let should_encode = !(head.headers().contains_key(&CONTENT_ENCODING)
             || head.status == StatusCode::SWITCHING_PROTOCOLS
             || head.status == StatusCode::NO_CONTENT
+            || head.status == StatusCode::PARTIAL_CONTENT
             || encoding == ContentEncoding::Identity);
 
         let body = match body.try_into_bytes() {

actix-web/CHANGES.md

@@ -8,11 +8,13 @@
 - Add `experimental-introspection` feature to report configured routes [#3594]
 - Add config/method for `TCP_NODELAY`. [#3918]
 - Fix panic when `NormalizePath` rewrites a scoped dynamic path before extraction (e.g., `scope("{tail:.*}")` + `Path<String>`). [#3562]
+- Do not compress 206 Partial Content responses. [#3191]
 
 [#3895]: https://github.com/actix/actix-web/pull/3895
 [#3594]: https://github.com/actix/actix-web/pull/3594
 [#3918]: https://github.com/actix/actix-web/pull/3918
 [#3562]: https://github.com/actix/actix-web/issues/3562
+[#3191]: https://github.com/actix/actix-web/issues/3191
 
 ## 4.12.1
 

actix-web/src/middleware/compress.rs

@@ -449,6 +449,29 @@ mod tests {
         assert!(!res.headers().contains_key(header::CONTENT_ENCODING));
         assert!(test::read_body(res).await.is_empty());
     }
+
+    #[actix_rt::test]
+    async fn skips_compression_partial_content() {
+        let app = test::init_service({
+            App::new()
+                .wrap(Compress::default())
+                .default_service(web::to(|| {
+                    HttpResponse::PartialContent()
+                        .insert_header((header::CONTENT_TYPE, "text/plain"))
+                        .insert_header((header::CONTENT_RANGE, "bytes 0-10/100"))
+                        .body(TEXT_DATA)
+                }))
+        })
+        .await;
+
+        let req = test::TestRequest::default()
+            .insert_header((header::ACCEPT_ENCODING, "gzip"))
+            .to_request();
+        let res = test::call_service(&app, req).await;
+        assert_eq!(res.status(), StatusCode::PARTIAL_CONTENT);
+        assert!(!res.headers().contains_key(header::CONTENT_ENCODING));
+        assert_eq!(test::read_body(res).await, TEXT_DATA.as_bytes());
+    }
 }
 
 #[cfg(feature = "compress-brotli")]