diff --git a/Cargo.lock b/Cargo.lock index 7b8c58353c..60f3a7f702 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4256,6 +4256,7 @@ dependencies = [ "tracing", "ts-rs", "url", + "urlencoding", ] [[package]] diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 3516d8c90b..10fa4ee5a1 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -41,6 +41,31 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); +function inlineContentDisposition(filename: string): string { + return `inline; filename="${encodeURIComponent(filename)}"`; +} + +async function expectProxiedImageContentDisposition( + url: string, + filename: string, +) { + const expectedContentDisposition = inlineContentDisposition(filename); + const proxyResponse = await waitUntilSuccess( + async () => ({ + state: "success" as const, + data: await fetch(url), + }), + response => + response.ok && + response.headers.get("content-disposition") === + expectedContentDisposition, + ); + + expect(proxyResponse.headers.get("content-disposition")).toBe( + expectedContentDisposition, + ); +} + test("Upload image and delete it", async () => { const health = await alpha.imageHealth().then(expectSuccess); expect(health.success).toBeTruthy(); @@ -182,6 +207,10 @@ test("Purge post, linked image removed", async () => { }); test("Images in remote image post are proxied if setting enabled", async () => { + const expectedFilename = decodeURIComponent( + new URL(sampleImage).pathname.split("/").pop() ?? "", + ); + const community = await createCommunity(gamma).then(expectSuccess); const postRes = await createPost( gamma, @@ -208,6 +237,14 @@ test("Images in remote image post are proxied if setting enabled", async () => { // Make sure that it contains `jpg`, to be sure its an image expect(post.thumbnail_url?.includes(".jpg")).toBeTruthy(); + // Proxied image should include a Content-Disposition: inline header + if (post.thumbnail_url) { + await expectProxiedImageContentDisposition( + post.thumbnail_url, + expectedFilename, + ); + } + const epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post); expect(epsilonPostRes?.post).toBeDefined(); @@ -232,6 +269,13 @@ test("Images in remote image post are proxied if setting enabled", async () => { // Make sure that it contains `jpg`, to be sure its an image expect(epsilonPost.thumbnail_url?.includes(".jpg")).toBeTruthy(); + + if (epsilonPost.thumbnail_url) { + await expectProxiedImageContentDisposition( + epsilonPost.thumbnail_url, + expectedFilename, + ); + } }); test("Thumbnail of remote image link is proxied if setting enabled", async () => { diff --git a/crates/routes/Cargo.toml b/crates/routes/Cargo.toml index 0feee30574..9cd2bb2b45 100644 --- a/crates/routes/Cargo.toml +++ b/crates/routes/Cargo.toml @@ -60,6 +60,7 @@ actix-web-prom = "0.10.0" actix-cors = "0.7.1" rand = { workspace = true } percent-encoding = "2.3.2" +urlencoding = { workspace = true } diesel-uplete.workspace = true lemmy_diesel_utils = { workspace = true } rosetta-i18n = { workspace = true } diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index cb8666636e..4c537952c8 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -2,9 +2,10 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ HttpRequest, HttpResponse, + HttpResponseBuilder, Responder, body::{BodyStream, BoxBody}, - http::StatusCode, + http::{StatusCode, header::CONTENT_DISPOSITION}, web::{Data, *}, }; use lemmy_api_utils::context::LemmyContext; @@ -30,14 +31,14 @@ pub async fn get_image( return Ok(HttpResponse::Unauthorized().finish()); } - let name = &filename.into_inner(); + let name = filename.into_inner(); // If there are no query params, the URL is original let pictrs_url = context.settings().pictrs()?.url; let processed_url = if params.file_type.is_none() && params.max_size.is_none() { format!("{}image/original/{}", pictrs_url, name) } else { - let file_type = file_type(params.file_type, name).unwrap_or_default(); + let file_type = file_type(params.file_type, &name).unwrap_or_default(); let mut url = format!("{}image/process.{}?src={}", pictrs_url, file_type, name); @@ -47,7 +48,7 @@ pub async fn get_image( url }; - do_get_image(processed_url, req, &context).await + do_get_image(processed_url, req, &context, Some(name)).await } pub async fn image_proxy( @@ -69,11 +70,10 @@ pub async fn image_proxy( RemoteImage::validate(&mut context.pool(), url.clone().into()).await?; let pictrs_config = context.settings().pictrs()?; - let processed_url = if params.file_type.is_none() && params.max_size.is_none() { - format!("{}image/original?proxy={}", pictrs_config.url, encoded_url) - } else { - let file_type = file_type(params.file_type, url.path()).unwrap_or_default(); + let output_file_type = (params.file_type.is_some() || params.max_size.is_some()) + .then(|| file_type(params.file_type.clone(), url.path()).unwrap_or_default()); + let processed_url = if let Some(file_type) = &output_file_type { let mut url = format!( "{}image/process.{}?proxy={}", pictrs_config.url, file_type, encoded_url @@ -83,6 +83,8 @@ pub async fn image_proxy( url = format!("{url}&thumbnail={size}",); } url + } else { + format!("{}image/original?proxy={}", pictrs_config.url, encoded_url) }; let proxy_bypass_domains = SiteView::read_local(&mut context.pool()) @@ -95,13 +97,15 @@ pub async fn image_proxy( let bypass_proxy = proxy_bypass_domains .iter() .any(|s| url.domain().is_some_and(|d| d == s)); + if bypass_proxy { // Bypass proxy and redirect user to original image Ok(Either::Left(Redirect::to(url.to_string()).respond_to(&req))) } else { // Proxy the image data through Lemmy + let download_filename = download_filename_from_url(url.path(), output_file_type); Ok(Either::Right( - do_get_image(processed_url, req, &context).await?, + do_get_image(processed_url, req, &context, download_filename).await?, )) } } @@ -110,6 +114,7 @@ pub(super) async fn do_get_image( url: String, req: HttpRequest, context: &LemmyContext, + download_filename: Option, ) -> LemmyResult { let mut client_req = adapt_request(&req, url, context); @@ -117,10 +122,6 @@ pub(super) async fn do_get_image( client_req = client_req.header("X-Forwarded-For", addr.to_string()); } - if let Some(addr) = req.head().peer_addr { - client_req = client_req.header("X-Forwarded-For", addr.to_string()); - } - let res = client_req.send().await?; if res.status() == http::StatusCode::NOT_FOUND { @@ -133,6 +134,10 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } + if let Some(download_filename) = &download_filename { + set_content_disposition(&mut client_res, download_filename); + } + Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } @@ -154,6 +159,44 @@ enum PictrsFileType { Webp, } +fn set_content_disposition(client_res: &mut HttpResponseBuilder, filename: &str) { + let encoded = urlencoding::encode(filename); + client_res.insert_header(( + CONTENT_DISPOSITION, + format!("inline; filename=\"{}\"", encoded), + )); +} + +/// Extracts the final path segment from a URL, percent-decodes it, and returns a +/// download filename. +/// +/// If `output_file_type` is set, the extension is replaced with that type. +/// Otherwise the original extension is preserved, or `.jpg` is added when none exists. +fn download_filename_from_url( + path: &str, + output_file_type: Option, +) -> Option { + let raw = path + .rsplit('/') + .next()? + .split('?') + .next() + .filter(|s| !s.is_empty())?; + let decoded = urlencoding::decode(raw).unwrap_or_else(|_| raw.into()); + let name = decoded.as_ref(); + + let has_ext = name.rsplit_once('.').is_some_and(|(s, _)| !s.is_empty()); + let stem = name + .rsplit_once('.') + .filter(|(s, _)| !s.is_empty()) + .map_or(name, |(s, _)| s); + match output_file_type { + None if has_ext => Some(name.into()), + None => Some(format!("{name}.jpg")), + Some(ft) => Some(format!("{stem}.{ft}")), + } +} + /// Take file type from param, name, or use jpg if nothing is given fn file_type(file_type: Option, name: &str) -> LemmyResult { let type_str = file_type @@ -165,7 +208,12 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult