diff --git a/src/api/client/media.rs b/src/api/client/media.rs index 712c3fe2..9a7d9eaa 100644 --- a/src/api/client/media.rs +++ b/src/api/client/media.rs @@ -6,11 +6,7 @@ use axum::extract::State; use axum_client_ip::InsecureClientIp; use conduit::{ debug_info, debug_warn, err, info, - utils::{ - self, - content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename}, - math::ruma_from_usize, - }, + utils::{self, content_disposition::make_content_disposition, math::ruma_from_usize}, warn, Err, Error, Result, }; use ruma::api::client::media::{ @@ -118,21 +114,14 @@ pub(crate) async fn create_content_route( let mxc = format!("mxc://{}/{}", services.globals.server_name(), utils::random_string(MXC_LENGTH)); + let content_disposition = make_content_disposition(None, body.content_type.as_deref(), body.filename.as_deref()); + services .media .create( Some(sender_user.clone()), &mxc, - body.filename - .as_ref() - .map(|filename| { - format!( - "{}; filename={}", - content_disposition_type(&body.content_type), - sanitise_filename(filename.to_owned()) - ) - }) - .as_deref(), + Some(&content_disposition), body.content_type.as_deref(), &body.file, ) @@ -185,14 +174,14 @@ pub(crate) async fn get_content_route( content_disposition, }) = services.media.get(&mxc).await? { - let content_disposition = Some(make_content_disposition(&content_type, content_disposition, None)); - let file = content.expect("content"); + let content_disposition = make_content_disposition(content_disposition.as_ref(), content_type.as_deref(), None); + let file = content.expect("content"); Ok(get_content::v3::Response { file, - content_type, - content_disposition, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), + content_type: content_type.map(Into::into), + content_disposition: Some(content_disposition), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) } else if !services.globals.server_is_ours(&body.server_name) && body.allow_remote { @@ -207,18 +196,15 @@ pub(crate) async fn get_content_route( .await .map_err(|e| err!(Request(NotFound(debug_warn!("Fetching media `{mxc}` failed: {e:?}")))))?; - let content_disposition = Some(make_content_disposition( - &response.content_type, - response.content_disposition, - None, - )); + let content_disposition = + make_content_disposition(response.content_disposition.as_ref(), response.content_type.as_deref(), None); Ok(get_content::v3::Response { file: response.file, content_type: response.content_type, - content_disposition, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), - cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_owned()), + content_disposition: Some(content_disposition), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), + cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) } else { Err!(Request(NotFound("Media not found."))) @@ -268,18 +254,15 @@ pub(crate) async fn get_content_as_filename_route( content_disposition, }) = services.media.get(&mxc).await? { - let content_disposition = Some(make_content_disposition( - &content_type, - content_disposition, - Some(body.filename.clone()), - )); + let content_disposition = + make_content_disposition(content_disposition.as_ref(), content_type.as_deref(), Some(&body.filename)); let file = content.expect("content"); Ok(get_content_as_filename::v3::Response { file, - content_type, - content_disposition, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), + content_type: content_type.map(Into::into), + content_disposition: Some(content_disposition), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) } else if !services.globals.server_is_ours(&body.server_name) && body.allow_remote { @@ -294,17 +277,17 @@ pub(crate) async fn get_content_as_filename_route( .await { Ok(remote_content_response) => { - let content_disposition = Some(make_content_disposition( - &remote_content_response.content_type, - remote_content_response.content_disposition, + let content_disposition = make_content_disposition( + remote_content_response.content_disposition.as_ref(), + remote_content_response.content_type.as_deref(), None, - )); + ); Ok(get_content_as_filename::v3::Response { - content_disposition, + content_disposition: Some(content_disposition), content_type: remote_content_response.content_type, file: remote_content_response.file, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) }, @@ -369,15 +352,15 @@ pub(crate) async fn get_content_thumbnail_route( ) .await? { - let content_disposition = Some(make_content_disposition(&content_type, content_disposition, None)); + let content_disposition = make_content_disposition(content_disposition.as_ref(), content_type.as_deref(), None); let file = content.expect("content"); Ok(get_content_thumbnail::v3::Response { file, - content_type, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), + content_type: content_type.map(Into::into), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), - content_disposition, + content_disposition: Some(content_disposition), }) } else if !services.globals.server_is_ours(&body.server_name) && body.allow_remote { if services @@ -423,18 +406,18 @@ pub(crate) async fn get_content_thumbnail_route( ) .await?; - let content_disposition = Some(make_content_disposition( - &get_thumbnail_response.content_type, - get_thumbnail_response.content_disposition, + let content_disposition = make_content_disposition( + get_thumbnail_response.content_disposition.as_ref(), + get_thumbnail_response.content_type.as_deref(), None, - )); + ); 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, + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), + cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), + content_disposition: Some(content_disposition), }) }, Err(e) => Err!(Request(NotFound(debug_warn!("Fetching media `{mxc}` failed: {e:?}")))), @@ -495,18 +478,18 @@ async fn get_remote_content( ) .await?; - let content_disposition = Some(make_content_disposition( - &content_response.content_type, - content_response.content_disposition, + let content_disposition = make_content_disposition( + content_response.content_disposition.as_ref(), + content_response.content_type.as_deref(), None, - )); + ); services .media .create( None, mxc, - content_disposition.as_deref(), + Some(&content_disposition), content_response.content_type.as_deref(), &content_response.file, ) @@ -515,8 +498,8 @@ async fn get_remote_content( Ok(get_content::v3::Response { file: content_response.file, content_type: content_response.content_type, - content_disposition, - cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.to_owned()), - cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_owned()), + content_disposition: Some(content_disposition), + cross_origin_resource_policy: Some(CORP_CROSS_ORIGIN.into()), + cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()), }) } diff --git a/src/core/error/mod.rs b/src/core/error/mod.rs index 824b4913..9797c741 100644 --- a/src/core/error/mod.rs +++ b/src/core/error/mod.rs @@ -87,6 +87,8 @@ pub enum Error { Config(&'static str, Cow<'static, str>), #[error("{0}")] Conflict(&'static str), // This is only needed for when a room alias already exists + #[error(transparent)] + ContentDisposition(#[from] ruma::http_headers::ContentDispositionParseError), #[error("{0}")] Database(Cow<'static, str>), #[error("Remote server {0} responded with: {1}")] diff --git a/src/core/utils/content_disposition.rs b/src/core/utils/content_disposition.rs index be17a731..a2fe923c 100644 --- a/src/core/utils/content_disposition.rs +++ b/src/core/utils/content_disposition.rs @@ -1,7 +1,8 @@ -use crate::debug_info; +use std::borrow::Cow; -const ATTACHMENT: &str = "attachment"; -const INLINE: &str = "inline"; +use ruma::http_headers::{ContentDisposition, ContentDispositionType}; + +use crate::debug_info; /// as defined by MSC2702 const ALLOWED_INLINE_CONTENT_TYPES: [&str; 26] = [ @@ -38,42 +39,44 @@ const ALLOWED_INLINE_CONTENT_TYPES: [&str; 26] = [ /// Content-Type against MSC2702 list of safe inline Content-Types /// (`ALLOWED_INLINE_CONTENT_TYPES`) #[must_use] -pub fn content_disposition_type(content_type: &Option) -> &'static str { +pub fn content_disposition_type(content_type: Option<&str>) -> ContentDispositionType { let Some(content_type) = content_type else { debug_info!("No Content-Type was given, assuming attachment for Content-Disposition"); - return ATTACHMENT; + return ContentDispositionType::Attachment; }; // is_sorted is unstable /* debug_assert!(ALLOWED_INLINE_CONTENT_TYPES.is_sorted(), * "ALLOWED_INLINE_CONTENT_TYPES is not sorted"); */ - let content_type = content_type + let content_type: Cow<'_, str> = content_type .split(';') .next() .unwrap_or(content_type) - .to_ascii_lowercase(); + .to_ascii_lowercase() + .into(); if ALLOWED_INLINE_CONTENT_TYPES - .binary_search(&content_type.as_str()) + .binary_search(&content_type.as_ref()) .is_ok() { - INLINE + ContentDispositionType::Inline } else { - ATTACHMENT + ContentDispositionType::Attachment } } /// sanitises the file name for the Content-Disposition using /// `sanitize_filename` crate #[tracing::instrument(level = "debug")] -pub fn sanitise_filename(filename: String) -> String { - let options = sanitize_filename::Options { - truncate: false, - ..Default::default() - }; - - sanitize_filename::sanitize_with_options(filename, options) +pub fn sanitise_filename(filename: &str) -> String { + sanitize_filename::sanitize_with_options( + filename, + sanitize_filename::Options { + truncate: false, + ..Default::default() + }, + ) } /// creates the final Content-Disposition based on whether the filename exists @@ -85,33 +88,13 @@ pub fn sanitise_filename(filename: String) -> String { /// /// else: `Content-Disposition: attachment/inline` pub fn make_content_disposition( - content_type: &Option, content_disposition: Option, req_filename: Option, -) -> String { - let filename: String; - - if let Some(req_filename) = req_filename { - filename = sanitise_filename(req_filename); - } else { - filename = content_disposition.map_or_else(String::new, |content_disposition| { - let (_, filename) = content_disposition - .split_once("filename=") - .unwrap_or(("", "")); - - if filename.is_empty() { - String::new() - } else { - sanitise_filename(filename.to_owned()) - } - }); - }; - - if !filename.is_empty() { - // Content-Disposition: attachment/inline; filename=filename.ext - format!("{}; filename={}", content_disposition_type(content_type), filename) - } else { - // Content-Disposition: attachment/inline - String::from(content_disposition_type(content_type)) - } + content_disposition: Option<&ContentDisposition>, content_type: Option<&str>, filename: Option<&str>, +) -> ContentDisposition { + ContentDisposition::new(content_disposition_type(content_type)).with_filename( + filename + .or_else(|| content_disposition.and_then(|content_disposition| content_disposition.filename.as_deref())) + .map(sanitise_filename), + ) } #[cfg(test)] @@ -136,4 +119,20 @@ mod tests { assert_eq!(SANITISED, sanitize_filename::sanitize_with_options(SAMPLE, options.clone())); } + + #[test] + fn empty_sanitisation() { + use crate::utils::string::EMPTY; + + let result = sanitize_filename::sanitize_with_options( + EMPTY, + sanitize_filename::Options { + windows: true, + truncate: true, + replacement: "", + }, + ); + + assert_eq!(EMPTY, result); + } } diff --git a/src/service/media/data.rs b/src/service/media/data.rs index 70e010c2..222d28b5 100644 --- a/src/service/media/data.rs +++ b/src/service/media/data.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use conduit::{debug, debug_info, utils::string_from_bytes, Error, Result}; use database::{Database, Map}; -use ruma::api::client::error::ErrorKind; +use ruma::{api::client::error::ErrorKind, http_headers::ContentDisposition}; use super::preview::UrlPreviewData; @@ -14,7 +14,7 @@ pub(crate) struct Data { #[derive(Debug)] pub(super) struct Metadata { - pub(super) content_disposition: Option, + pub(super) content_disposition: Option, pub(super) content_type: Option, pub(super) key: Vec, } @@ -29,8 +29,8 @@ impl Data { } pub(super) fn create_file_metadata( - &self, sender_user: Option<&str>, mxc: &str, width: u32, height: u32, content_disposition: Option<&str>, - content_type: Option<&str>, + &self, sender_user: Option<&str>, mxc: &str, width: u32, height: u32, + content_disposition: Option<&ContentDisposition>, content_type: Option<&str>, ) -> Result> { let mut key = mxc.as_bytes().to_vec(); key.push(0xFF); @@ -39,9 +39,9 @@ impl Data { key.push(0xFF); key.extend_from_slice( content_disposition - .as_ref() - .map(|f| f.as_bytes()) - .unwrap_or_default(), + .map(ToString::to_string) + .unwrap_or_default() + .as_bytes(), ); key.push(0xFF); key.extend_from_slice( @@ -143,7 +143,8 @@ impl Data { } else { Some( string_from_bytes(content_disposition_bytes) - .map_err(|_| Error::bad_database("Content Disposition in mediaid_file is invalid unicode."))?, + .map_err(|_| Error::bad_database("Content Disposition in mediaid_file is invalid unicode."))? + .parse()?, ) }; diff --git a/src/service/media/mod.rs b/src/service/media/mod.rs index ff3f3dc4..62baf548 100644 --- a/src/service/media/mod.rs +++ b/src/service/media/mod.rs @@ -9,7 +9,7 @@ use async_trait::async_trait; use base64::{engine::general_purpose, Engine as _}; use conduit::{debug, debug_error, err, error, trace, utils, utils::MutexMap, Err, Result, Server}; use data::{Data, Metadata}; -use ruma::{OwnedMxcUri, OwnedUserId}; +use ruma::{http_headers::ContentDisposition, OwnedMxcUri, OwnedUserId}; use tokio::{ fs, io::{AsyncReadExt, AsyncWriteExt, BufReader}, @@ -21,7 +21,7 @@ use crate::{client, globals, Dep}; pub struct FileMeta { pub content: Option>, pub content_type: Option, - pub content_disposition: Option, + pub content_disposition: Option, } pub struct Service { @@ -65,7 +65,7 @@ impl crate::Service for Service { impl Service { /// Uploads a file. pub async fn create( - &self, sender_user: Option, mxc: &str, content_disposition: Option<&str>, + &self, sender_user: Option, mxc: &str, content_disposition: Option<&ContentDisposition>, content_type: Option<&str>, file: &[u8], ) -> Result<()> { // Width, Height = 0 if it's not a thumbnail diff --git a/src/service/media/thumbnail.rs b/src/service/media/thumbnail.rs index 01bf73f6..7a1eef6c 100644 --- a/src/service/media/thumbnail.rs +++ b/src/service/media/thumbnail.rs @@ -2,7 +2,7 @@ use std::{cmp, io::Cursor, num::Saturating as Sat}; use conduit::{checked, Result}; use image::{imageops::FilterType, DynamicImage}; -use ruma::OwnedUserId; +use ruma::{http_headers::ContentDisposition, OwnedUserId}; use tokio::{ fs, io::{AsyncReadExt, AsyncWriteExt}, @@ -14,7 +14,7 @@ impl super::Service { /// Uploads or replaces a file thumbnail. #[allow(clippy::too_many_arguments)] pub async fn upload_thumbnail( - &self, sender_user: Option, mxc: &str, content_disposition: Option<&str>, + &self, sender_user: Option, mxc: &str, content_disposition: Option<&ContentDisposition>, content_type: Option<&str>, width: u32, height: u32, file: &[u8], ) -> Result<()> { let key = if let Some(user) = sender_user { @@ -104,7 +104,7 @@ impl super::Service { mxc, width, height, - data.content_disposition.as_deref(), + data.content_disposition.as_ref(), data.content_type.as_deref(), )?;