From cd2f082e77ff80b02f1511009b4cd55990e43836 Mon Sep 17 00:00:00 2001 From: valaphee <32491319+valaphee@users.noreply.github.com> Date: Sat, 13 Jul 2024 16:56:13 +0200 Subject: [PATCH 1/3] Remove the usage of content-encoding identity header, disallow compression for partial content in general --- actix-files/src/named.rs | 21 --------------------- actix-files/tests/encoding.rs | 28 ---------------------------- actix-http/src/encoding/encoder.rs | 1 + 3 files changed, 1 insertion(+), 49 deletions(-) diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 9e4a377373b..52abb12f43d 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -534,27 +534,6 @@ impl NamedFile { length = ranges[0].length; offset = ranges[0].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()), diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs index 3c8bdb59b33..aee4094788b 100644 --- a/actix-files/tests/encoding.rs +++ b/actix-files/tests/encoding.rs @@ -35,31 +35,3 @@ 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" - ); -} diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 180927ac64d..5177cfe4099 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -70,6 +70,7 @@ impl Encoder { 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() { From b227167d49ba92910cc3a9a8b823f9f7a7e2b69b Mon Sep 17 00:00:00 2001 From: valaphee <32491319+valaphee@users.noreply.github.com> Date: Sat, 13 Jul 2024 17:11:05 +0200 Subject: [PATCH 2/3] Update CHANGELOG --- actix-files/CHANGES.md | 2 ++ actix-http/CHANGES.md | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index e94f43907ba..d69ebaff83d 100644 --- a/actix-files/CHANGES.md +++ b/actix-files/CHANGES.md @@ -2,6 +2,8 @@ ## Unreleased +- Do not set content-encoding to identity for range requests + ## 0.6.6 - Update `tokio-uring` dependency to `0.4`. diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index b2e5af79f45..cb5b04329a6 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -6,6 +6,10 @@ - Implement `FromIterator<(HeaderName, HeaderValue)>` for `HeaderMap`. +### Changed + +- Prevent compression of Partial Content. + ## 3.8.0 ### Added From 9fa392de97fb1b01014d202b91bab179504b7370 Mon Sep 17 00:00:00 2001 From: valaphee <32491319+valaphee@users.noreply.github.com> Date: Sat, 13 Jul 2024 17:21:20 +0200 Subject: [PATCH 3/3] Rustfmt --- actix-files/src/named.rs | 2 +- actix-files/tests/encoding.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 52abb12f43d..57a620f0b7e 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -14,7 +14,7 @@ use actix_web::{ http::{ header::{ self, Charset, ContentDisposition, ContentEncoding, DispositionParam, DispositionType, - ExtendedValue, HeaderValue, + ExtendedValue, }, StatusCode, }, diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs index aee4094788b..2fc36db9781 100644 --- a/actix-files/tests/encoding.rs +++ b/actix-files/tests/encoding.rs @@ -1,11 +1,11 @@ -use actix_files::{Files, NamedFile}; +use actix_files::Files; use actix_web::{ http::{ header::{self, HeaderValue}, StatusCode, }, test::{self, TestRequest}, - web, App, + App, }; #[actix_web::test]