diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md index f25a56228..6f4056f57 100644 --- a/actix-files/CHANGES.md +++ b/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 diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 09e3706e1..f1a4642d7 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, }, @@ -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()), diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs index 019abfb57..72e72b913 100644 --- a/actix-files/tests/encoding.rs +++ b/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!( - res.headers().get(header::CONTENT_ENCODING).unwrap(), - "identity" - ); + assert!(!res.headers().contains_key(header::CONTENT_ENCODING)); } diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index b52aa5c58..89ad54596 100644 --- a/actix-http/CHANGES.md +++ b/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 diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 0da95c462..f40338689 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() { diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index bebc1d3eb..a89060b57 100644 --- a/actix-web/CHANGES.md +++ b/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`). [#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 diff --git a/actix-web/src/middleware/compress.rs b/actix-web/src/middleware/compress.rs index 7f0d8a4fb..38bb909fd 100644 --- a/actix-web/src/middleware/compress.rs +++ b/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")]