From db99da5daf8e52f31513921819047fa07a9cf168 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 19 Jul 2023 20:24:32 +0100 Subject: [PATCH] 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 --- actix-web/CHANGES.md | 1 + actix-web/src/http/header/content_type.rs | 79 ++++++------- actix-web/src/middleware/compress.rs | 129 ++++++++++++++++++++-- 3 files changed, 154 insertions(+), 55 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 7e82d095a..35d695328 100644 --- a/actix-web/CHANGES.md +++ b/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. diff --git a/actix-web/src/http/header/content_type.rs b/actix-web/src/http/header/content_type.rs index 41c0c1fc9..bf86afffa 100644 --- a/actix-web/src/http/header/content_type.rs +++ b/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 - /// in [RFC 7231 §3.1.1.5](https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.5) + /// `Content-Type` header, defined in [RFC 9110 §8.3]. /// - /// The `Content-Type` header field indicates the media type of the - /// associated representation: either the representation enclosed in the - /// message payload or the selected representation, as determined by the - /// 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 received message semantics, after any content - /// codings indicated by Content-Encoding are decoded. + /// The `Content-Type` header field indicates the media type of the associated representation: + /// either the representation enclosed in the message payload or the selected representation, + /// as determined by the 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 + /// received message semantics, after any content codings indicated by Content-Encoding are + /// decoded. /// - /// Although the `mime` crate allows the mime options to be any slice, this crate - /// forces the use of Vec. This is to make sure the same header can't have more than 1 type. If - /// this is an issue, it's possible to implement `Header` on a custom struct. + /// Although the `mime` crate allows the mime options to be any slice, this crate forces the use + /// of Vec. This is to make sure the same header can't have more than 1 type. If this is an + /// 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; + /// use actix_web::{http::header::ContentType, HttpResponse}; /// - /// let mut builder = HttpResponse::Ok(); - /// builder.insert_header( - /// ContentType(mime::TEXT_HTML) - /// ); + /// let res_json = HttpResponse::Ok() + /// .insert_header(ContentType::json()); + /// + /// 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` - /// header. + /// Constructs a `Content-Type: application/json` header. #[inline] pub fn json() -> ContentType { ContentType(mime::APPLICATION_JSON) } - /// A constructor to easily create a `Content-Type: text/plain; - /// charset=utf-8` header. + /// Constructs a `Content-Type: text/plain; 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` - /// header. + /// Constructs a `Content-Type: text/html; charset=utf-8` 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: - /// application/www-form-url-encoded` header. + /// Constructs a `Content-Type: 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: - /// application/octet-stream` header. + /// Constructs a `Content-Type: application/octet-stream` header. #[inline] pub fn octet_stream() -> ContentType { ContentType(mime::APPLICATION_OCTET_STREAM) diff --git a/actix-web/src/middleware/compress.rs b/actix-web/src/middleware/compress.rs index 51b44c6ef..a55b46264 100644 --- a/actix-web/src/middleware/compress.rs +++ b/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>>, Error>; - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = self.project(); + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + 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::().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 = "

hello world

) -> Vec { use std::io::Read as _; @@ -257,23 +289,55 @@ mod tests { buf } + #[track_caller] + fn assert_successful_res_with_content_type(res: &ServiceResponse, 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(res: &ServiceResponse, 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(res: &ServiceResponse, 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()); + } }