do not compress media types (#3075)

* misc: add temporary nix file

* Add test to check content type image/*

* misc: add unit test for expected behaviour jpeg

* feat(compress): add compress function to middleware

* feat(compress): use response content type to decide compress

* feat(compress): give more control to the user

* misc: improve default compress function

* add Compress::with_predicate

* remove predicate options

* assert auto traits on Compress

* fix changelog

---------

Co-authored-by: William R. Arellano <arellanowr@gmail.com>

actix-web/CHANGES.md

@@ -10,6 +10,7 @@
 ### Changed
 
 - Handler functions can now receive up to 16 extractor parameters.
+- The `Compress` middleware no longer compresses image or video content.
 - Hide sensitive header values in `HttpRequest`'s `Debug` output.
 - Minimum supported Rust version (MSRV) is now 1.65 due to transitive `time` dependency.
 

actix-web/src/http/header/content_type.rs

@@ -3,109 +3,102 @@ use mime::Mime;
 use super::CONTENT_TYPE;
 
 crate::http::header::common_header! {
-    /// `Content-Type` header, defined
+    /// `Content-Type` header, defined in [RFC 9110 §8.3].
-    /// in [RFC 7231 §3.1.1.5](https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.5)
     ///
-    /// The `Content-Type` header field indicates the media type of the
+    /// The `Content-Type` header field indicates the media type of the associated representation:
-    /// associated representation: either the representation enclosed in the
+    /// either the representation enclosed in the message payload or the selected representation,
-    /// message payload or the selected representation, as determined by the
+    /// as determined by the message semantics. The indicated media type defines both the data
-    /// message semantics.  The indicated media type defines both the data
+    /// format and how that data is intended to be processed by a recipient, within the scope of the
-    /// format and how that data is intended to be processed by a recipient,
+    /// received message semantics, after any content codings indicated by Content-Encoding are
-    /// within the scope of the received message semantics, after any content
+    /// decoded.
-    /// codings indicated by Content-Encoding are decoded.
     ///
-    /// Although the `mime` crate allows the mime options to be any slice, this crate
+    /// Although the `mime` crate allows the mime options to be any slice, this crate forces the use
-    /// forces the use of Vec. This is to make sure the same header can't have more than 1 type. If
+    /// of Vec. This is to make sure the same header can't have more than 1 type. If this is an
-    /// this is an issue, it's possible to implement `Header` on a custom struct.
+    /// issue, it's possible to implement `Header` on a custom struct.
     ///
     /// # ABNF
+    ///
     /// ```plain
     /// Content-Type = media-type
     /// ```
     ///
     /// # Example Values
-    /// * `text/html; charset=utf-8`
+    ///
-    /// * `application/json`
+    /// - `text/html; charset=utf-8`
+    /// - `application/json`
     ///
     /// # Examples
-    /// ```
-    /// use actix_web::HttpResponse;
-    /// use actix_web::http::header::ContentType;
-    ///
-    /// let mut builder = HttpResponse::Ok();
-    /// builder.insert_header(
-    ///     ContentType::json()
-    /// );
-    /// ```
     ///
     /// ```
-    /// use actix_web::HttpResponse;
+    /// use actix_web::{http::header::ContentType, HttpResponse};
-    /// use actix_web::http::header::ContentType;
     ///
-    /// let mut builder = HttpResponse::Ok();
+    /// let res_json = HttpResponse::Ok()
-    /// builder.insert_header(
+    ///     .insert_header(ContentType::json());
-    ///     ContentType(mime::TEXT_HTML)
+    ///
-    /// );
+    /// let res_html = HttpResponse::Ok()
+    ///     .insert_header(ContentType(mime::TEXT_HTML));
     /// ```
+    ///
+    /// [RFC 9110 §8.3]: https://datatracker.ietf.org/doc/html/rfc9110#section-8.3
     (ContentType, CONTENT_TYPE) => [Mime]
 
     test_parse_and_format {
         crate::http::header::common_header_test!(
-            test1,
+            test_text_html,
             vec![b"text/html"],
             Some(HeaderField(mime::TEXT_HTML)));
+        crate::http::header::common_header_test!(
+            test_image_star,
+            vec![b"image/*"],
+            Some(HeaderField(mime::IMAGE_STAR)));
+
     }
 }
 
 impl ContentType {
-    /// A constructor to easily create a `Content-Type: application/json`
+    /// Constructs a `Content-Type: application/json` header.
-    /// header.
     #[inline]
     pub fn json() -> ContentType {
         ContentType(mime::APPLICATION_JSON)
     }
 
-    /// A constructor to easily create a `Content-Type: text/plain;
+    /// Constructs a `Content-Type: text/plain; charset=utf-8` header.
-    /// charset=utf-8` header.
     #[inline]
     pub fn plaintext() -> ContentType {
         ContentType(mime::TEXT_PLAIN_UTF_8)
     }
 
-    /// A constructor to easily create a `Content-Type: text/html; charset=utf-8`
+    /// Constructs a `Content-Type: text/html; charset=utf-8` header.
-    /// header.
     #[inline]
     pub fn html() -> ContentType {
         ContentType(mime::TEXT_HTML_UTF_8)
     }
 
-    /// A constructor to easily create a `Content-Type: text/xml` header.
+    /// Constructs a `Content-Type: text/xml` header.
     #[inline]
     pub fn xml() -> ContentType {
         ContentType(mime::TEXT_XML)
     }
 
-    /// A constructor to easily create a `Content-Type:
+    /// Constructs a `Content-Type: application/www-form-url-encoded` header.
-    /// application/www-form-url-encoded` header.
     #[inline]
     pub fn form_url_encoded() -> ContentType {
         ContentType(mime::APPLICATION_WWW_FORM_URLENCODED)
     }
 
-    /// A constructor to easily create a `Content-Type: image/jpeg` header.
+    /// Constructs a `Content-Type: image/jpeg` header.
     #[inline]
     pub fn jpeg() -> ContentType {
         ContentType(mime::IMAGE_JPEG)
     }
 
-    /// A constructor to easily create a `Content-Type: image/png` header.
+    /// Constructs a `Content-Type: image/png` header.
     #[inline]
     pub fn png() -> ContentType {
         ContentType(mime::IMAGE_PNG)
     }
 
-    /// A constructor to easily create a `Content-Type:
+    /// Constructs a `Content-Type: application/octet-stream` header.
-    /// application/octet-stream` header.
     #[inline]
     pub fn octet_stream() -> ContentType {
         ContentType(mime::APPLICATION_OCTET_STREAM)

actix-web/src/middleware/compress.rs

@@ -11,13 +11,14 @@ use actix_http::encoding::Encoder;
 use actix_service::{Service, Transform};
 use actix_utils::future::{ok, Either, Ready};
 use futures_core::ready;
+use mime::Mime;
 use once_cell::sync::Lazy;
 use pin_project_lite::pin_project;
 
 use crate::{
     body::{EitherBody, MessageBody},
     http::{
-        header::{self, AcceptEncoding, Encoding, HeaderValue},
+        header::{self, AcceptEncoding, ContentEncoding, Encoding, HeaderValue},
         StatusCode,
     },
     service::{ServiceRequest, ServiceResponse},
@@ -170,19 +171,40 @@ where
 {
     type Output = Result<ServiceResponse<EitherBody<Encoder<B>>>, Error>;
 
-    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
+    fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
-        let this = self.project();
+        let this = self.as_mut().project();
 
         match ready!(this.fut.poll(cx)) {
             Ok(resp) => {
                 let enc = match this.encoding {
                     Encoding::Known(enc) => *enc,
                     Encoding::Unknown(enc) => {
-                        unimplemented!("encoding {} should not be here", enc);
+                        unimplemented!("encoding '{enc}' should not be here");
                     }
                 };
 
                 Poll::Ready(Ok(resp.map_body(move |head, body| {
+                    let content_type = head.headers.get(header::CONTENT_TYPE);
+
+                    fn default_compress_predicate(content_type: Option<&HeaderValue>) -> bool {
+                        match content_type {
+                            None => true,
+                            Some(hdr) => {
+                                match hdr.to_str().ok().and_then(|hdr| hdr.parse::<Mime>().ok()) {
+                                    Some(mime) if mime.type_().as_str() == "image" => false,
+                                    Some(mime) if mime.type_().as_str() == "video" => false,
+                                    _ => true,
+                                }
+                            }
+                        }
+                    }
+
+                    let enc = if default_compress_predicate(content_type) {
+                        enc
+                    } else {
+                        ContentEncoding::Identity
+                    };
+
                     EitherBody::left(Encoder::response(enc, head, body))
                 })))
             }
@@ -246,8 +268,18 @@ static SUPPORTED_ENCODINGS: &[Encoding] = &[
 mod tests {
     use std::collections::HashSet;
 
+    use static_assertions::assert_impl_all;
+
     use super::*;
-    use crate::{middleware::DefaultHeaders, test, web, App};
+    use crate::{http::header::ContentType, middleware::DefaultHeaders, test, web, App};
+
+    const HTML_DATA_PART: &str = "<html><h1>hello world</h1></html";
+    const HTML_DATA: &str = const_str::repeat!(HTML_DATA_PART, 100);
+
+    const TEXT_DATA_PART: &str = "hello world ";
+    const TEXT_DATA: &str = const_str::repeat!(TEXT_DATA_PART, 100);
+
+    assert_impl_all!(Compress: Send, Sync);
 
     pub fn gzip_decode(bytes: impl AsRef<[u8]>) -> Vec<u8> {
         use std::io::Read as _;
@@ -257,23 +289,55 @@ mod tests {
         buf
     }
 
+    #[track_caller]
+    fn assert_successful_res_with_content_type<B>(res: &ServiceResponse<B>, ct: &str) {
+        assert!(res.status().is_success());
+        assert!(
+            res.headers()
+                .get(header::CONTENT_TYPE)
+                .expect("content-type header should be present")
+                .to_str()
+                .expect("content-type header should be utf-8")
+                .contains(ct),
+            "response's content-type did not match {}",
+            ct
+        );
+    }
+
+    #[track_caller]
+    fn assert_successful_gzip_res_with_content_type<B>(res: &ServiceResponse<B>, ct: &str) {
+        assert_successful_res_with_content_type(res, ct);
+        assert_eq!(
+            res.headers()
+                .get(header::CONTENT_ENCODING)
+                .expect("response should be gzip compressed"),
+            "gzip",
+        );
+    }
+
+    #[track_caller]
+    fn assert_successful_identity_res_with_content_type<B>(res: &ServiceResponse<B>, ct: &str) {
+        assert_successful_res_with_content_type(res, ct);
+        assert!(
+            res.headers().get(header::CONTENT_ENCODING).is_none(),
+            "response should not be compressed",
+        );
+    }
+
     #[actix_rt::test]
     async fn prevents_double_compressing() {
-        const D: &str = "hello world ";
-        const DATA: &str = const_str::repeat!(D, 100);
-
         let app = test::init_service({
             App::new()
                 .wrap(Compress::default())
                 .route(
                     "/single",
-                    web::get().to(move || HttpResponse::Ok().body(DATA)),
+                    web::get().to(move || HttpResponse::Ok().body(TEXT_DATA)),
                 )
                 .service(
                     web::resource("/double")
                         .wrap(Compress::default())
                         .wrap(DefaultHeaders::new().add(("x-double", "true")))
-                        .route(web::get().to(move || HttpResponse::Ok().body(DATA))),
+                        .route(web::get().to(move || HttpResponse::Ok().body(TEXT_DATA))),
                 )
         })
         .await;
@@ -287,7 +351,7 @@ mod tests {
         assert_eq!(res.headers().get("x-double"), None);
         assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip");
         let bytes = test::read_body(res).await;
-        assert_eq!(gzip_decode(bytes), DATA.as_bytes());
+        assert_eq!(gzip_decode(bytes), TEXT_DATA.as_bytes());
 
         let req = test::TestRequest::default()
             .uri("/double")
@@ -298,7 +362,7 @@ mod tests {
         assert_eq!(res.headers().get("x-double").unwrap(), "true");
         assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip");
         let bytes = test::read_body(res).await;
-        assert_eq!(gzip_decode(bytes), DATA.as_bytes());
+        assert_eq!(gzip_decode(bytes), TEXT_DATA.as_bytes());
     }
 
     #[actix_rt::test]
@@ -324,4 +388,45 @@ mod tests {
         assert!(vary_headers.contains(&HeaderValue::from_static("x-test")));
         assert!(vary_headers.contains(&HeaderValue::from_static("accept-encoding")));
     }
+
+    fn configure_predicate_test(cfg: &mut web::ServiceConfig) {
+        cfg.route(
+            "/html",
+            web::get().to(|| {
+                HttpResponse::Ok()
+                    .content_type(ContentType::html())
+                    .body(HTML_DATA)
+            }),
+        )
+        .route(
+            "/image",
+            web::get().to(|| {
+                HttpResponse::Ok()
+                    .content_type(ContentType::jpeg())
+                    .body(TEXT_DATA)
+            }),
+        );
+    }
+
+    #[actix_rt::test]
+    async fn prevents_compression_jpeg() {
+        let app = test::init_service(
+            App::new()
+                .wrap(Compress::default())
+                .configure(configure_predicate_test),
+        )
+        .await;
+
+        let req =
+            test::TestRequest::with_uri("/html").insert_header((header::ACCEPT_ENCODING, "gzip"));
+        let res = test::call_service(&app, req.to_request()).await;
+        assert_successful_gzip_res_with_content_type(&res, "text/html");
+        assert_ne!(test::read_body(res).await, HTML_DATA.as_bytes());
+
+        let req =
+            test::TestRequest::with_uri("/image").insert_header((header::ACCEPT_ENCODING, "gzip"));
+        let res = test::call_service(&app, req.to_request()).await;
+        assert_successful_identity_res_with_content_type(&res, "image/jpeg");
+        assert_eq!(test::read_body(res).await, TEXT_DATA.as_bytes());
+    }
 }