From 2231ccf11829d6400ef72ed0ad754c6909bcbf94 Mon Sep 17 00:00:00 2001 From: strawberry Date: Tue, 7 May 2024 16:36:22 -0400 Subject: [PATCH] return `inline` Content-Disposition based on the detected file type (e.g. image/video) Signed-off-by: strawberry --- Cargo.lock | 7 ++++ Cargo.toml | 2 ++ src/api/client_server/media.rs | 62 ++++++++++++++++++++++++++++------ 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d91f30d0..9935aa21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -594,6 +594,7 @@ dependencies = [ "hyper 1.3.1", "hyper-util", "image", + "infer", "ipaddress", "itertools", "jsonwebtoken", @@ -1605,6 +1606,12 @@ dependencies = [ "serde", ] +[[package]] +name = "infer" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "865e8a58ae8e24d2c4412c31344afa1d302a3740ad67528c10f50d6876cdcf55" + [[package]] name = "inlinable_string" version = "0.1.15" diff --git a/Cargo.toml b/Cargo.toml index ea2d1770..681057a0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,8 @@ rust-version = "1.77.0" [dependencies] console-subscriber = { version = "0.2", optional = true } +infer = { version = "0.3", default-features = false } + # for hot lib reload hot-lib-reloader = { version = "^0.7", optional = true } diff --git a/src/api/client_server/media.rs b/src/api/client_server/media.rs index 12899b39..53091d3a 100644 --- a/src/api/client_server/media.rs +++ b/src/api/client_server/media.rs @@ -1,6 +1,7 @@ use std::{io::Cursor, sync::Arc, time::Duration}; use image::io::Reader as ImgReader; +use infer::MatcherType; use ipaddress::IPAddress; use reqwest::Url; use ruma::api::client::{ @@ -178,11 +179,13 @@ pub(crate) async fn get_content_route(body: Ruma) -> R .. }) = services().media.get(mxc.clone()).await? { + let content_disposition = Some(String::from(content_disposition_type(&file, &content_type))); + // TODO: safely sanitise filename to be included in the content-disposition Ok(get_content::v3::Response { file, content_type, - content_disposition: Some("attachment".to_owned()), + content_disposition, cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) @@ -241,10 +244,12 @@ pub(crate) async fn get_content_as_filename_route( .. }) = services().media.get(mxc.clone()).await? { + let content_disposition = Some(String::from(content_disposition_type(&file, &content_type))); + Ok(get_content_as_filename::v3::Response { file, content_type, - content_disposition: Some("attachment".to_owned()), + content_disposition, cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) @@ -258,13 +263,20 @@ pub(crate) async fn get_content_as_filename_route( ) .await { - Ok(remote_content_response) => Ok(get_content_as_filename::v3::Response { - content_disposition: Some("attachment".to_owned()), - content_type: remote_content_response.content_type, - file: remote_content_response.file, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), - cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), - }), + Ok(remote_content_response) => { + let content_disposition = Some(String::from(content_disposition_type( + &remote_content_response.file, + &remote_content_response.content_type, + ))); + + Ok(get_content_as_filename::v3::Response { + content_disposition, + content_type: remote_content_response.content_type, + file: remote_content_response.file, + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), + cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), + }) + }, Err(e) => { debug_warn!("Fetching media `{}` failed: {:?}", mxc, e); Err(Error::BadRequest(ErrorKind::NotFound, "Remote media error.")) @@ -323,12 +335,14 @@ pub(crate) async fn get_content_thumbnail_route( ) .await? { + let content_disposition = Some(String::from(content_disposition_type(&file, &content_type))); + Ok(get_content_thumbnail::v3::Response { file, content_type, cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), - content_disposition: Some("attachment".to_owned()), + content_disposition, }) } else if !server_is_ours(&body.server_name) && body.allow_remote { if services() @@ -373,12 +387,17 @@ pub(crate) async fn get_content_thumbnail_route( ) .await?; + let content_disposition = Some(String::from(content_disposition_type( + &get_thumbnail_response.file, + &get_thumbnail_response.content_type, + ))); + Ok(get_content_thumbnail::v3::Response { file: get_thumbnail_response.file, content_type: get_thumbnail_response.content_type, cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_owned()), - content_disposition: Some("attachment".to_owned()), + content_disposition, }) }, Err(e) => { @@ -688,3 +707,24 @@ fn url_preview_allowed(url_str: &str) -> bool { false } + +/// Returns a Content-Disposition of `attachment` or `inline`, depending on the +/// *parsed* contents of the file uploaded via format magic keys using `infer` +/// crate (basically libmagic without needing libmagic). +/// +/// This forbids trusting what the client or remote server says the file is from +/// their `Content-Type` and we try to detect it ourselves. Also returns +/// `attachment` if the Content-Type does not match what we detected. +/// +/// TODO: add a "strict" function for comparing the Content-Type with what we +/// detected: `file_type.mime_type() != content_type` +fn content_disposition_type(buf: &[u8], _content_type: &Option) -> &'static str { + let Some(file_type) = infer::get(buf) else { + return "attachment"; + }; + + match file_type.matcher_type() { + MatcherType::IMAGE | MatcherType::AUDIO | MatcherType::TEXT | MatcherType::VIDEO => "inline", + _ => "attachment", + } +}