From c491b6ed1f2acf37709e5eb23bfeb3917db8eede Mon Sep 17 00:00:00 2001 From: eduardo Date: Sun, 12 Apr 2026 16:30:36 -0400 Subject: [PATCH 01/14] Add Content-Disposition header for proxied images --- api_tests/src/image.spec.ts | 25 +++ crates/routes/src/images/download.rs | 283 +++++++++++++++++++++++++-- 2 files changed, 292 insertions(+), 16 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index ea8eb75d98..3e2763058c 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -40,6 +40,14 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); +function inlineContentDisposition(filename: string): string { + return `inline; filename="${filename}"`; +} + +function filenameFromUrl(url: string): string { + return decodeURIComponent(new URL(url).pathname.split("/").pop() ?? ""); +} + test("Upload image and delete it", async () => { const health = await alpha.imageHealth(); expect(health.success).toBeTruthy(); @@ -166,6 +174,8 @@ test("Purge post, linked image removed", async () => { }); test("Images in remote image post are proxied if setting enabled", async () => { + const expectedFilename = filenameFromUrl(sampleImage); + let community = await createCommunity(gamma); let postRes = await createPost( gamma, @@ -192,6 +202,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) { + const proxyResponse = await fetch(post.thumbnail_url); + const cd = proxyResponse.headers.get("content-disposition"); + expect(cd).not.toBeNull(); + expect(cd).toBe(inlineContentDisposition(expectedFilename)); + } + let epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post); expect(epsilonPostRes?.post).toBeDefined(); @@ -216,6 +234,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) { + const proxyResponse = await fetch(epsilonPost.thumbnail_url); + const cd = proxyResponse.headers.get("content-disposition"); + expect(cd).not.toBeNull(); + expect(cd).toBe(inlineContentDisposition(expectedFilename)); + } }); test("Thumbnail of remote image link is proxied if setting enabled", async () => { diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index fa9b32a4c6..c14d838ceb 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -1,8 +1,6 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ - HttpRequest, - HttpResponse, - Responder, + HttpRequest, HttpResponse, Responder, body::{BodyStream, BoxBody}, http::StatusCode, web::{Data, *}, @@ -12,7 +10,7 @@ use lemmy_db_schema::source::images::RemoteImage; use lemmy_db_views_local_image::api::{ImageGetParams, ImageProxyParams}; use lemmy_db_views_site::SiteView; use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; -use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode}; +use percent_encoding::{NON_ALPHANUMERIC, percent_decode_str, utf8_percent_encode}; use std::str::FromStr; use strum::{Display, EnumString}; use url::Url; @@ -40,7 +38,7 @@ pub async fn get_image( url }; - do_get_image(processed_url, req, &context).await + do_get_image(processed_url, req, &context, None).await } pub async fn image_proxy( @@ -56,11 +54,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 @@ -70,6 +67,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()) @@ -82,13 +81,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?, )) } } @@ -97,6 +98,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); @@ -104,10 +106,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 { @@ -120,10 +118,17 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } + if let Some(disposition) = download_filename + .as_deref() + .and_then(inline_content_disposition) + { + client_res.insert_header((actix_web::http::header::CONTENT_DISPOSITION, disposition)); + } + Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } -#[derive(EnumString, Display, PartialEq, Debug, Default)] +#[derive(EnumString, Display, PartialEq, Debug, Default, Clone, Copy)] #[strum(ascii_case_insensitive, serialize_all = "snake_case")] enum PictrsFileType { Apng, @@ -136,6 +141,92 @@ enum PictrsFileType { Webp, } +/// Format a `Content-Disposition: inline` header value for the given filename. +fn inline_content_disposition(name: &str) -> Option { + let sanitized = sanitize_download_filename(name)?; + + if sanitized.is_ascii() { + Some(format!("inline; filename=\"{}\"", sanitized)) + } else { + // use filename* for non-ASCII names. + // Also provide a plain ASCII fallback (non-ASCII chars replaced with '_') + // for older clients that don't understand filename*. + let ascii_fallback: String = sanitized + .chars() + .map(|c| if c.is_ascii() { c } else { '_' }) + .collect(); + let encoded = utf8_percent_encode(&sanitized, NON_ALPHANUMERIC).to_string(); + Some(format!( + "inline; filename=\"{}\"; filename*=UTF-8''{}", + ascii_fallback, encoded + )) + } +} + +fn sanitize_download_filename(name: &str) -> Option { + let mut sanitized = name.to_string(); + sanitized.retain(|c| !is_unsafe_download_filename_char(c)); + + if sanitized.is_empty() { + None + } else { + Some(sanitized) + } +} + +fn is_unsafe_download_filename_char(c: char) -> bool { + c.is_control() + || matches!( + c, + '"' + | '\\' + | '/' + | '\u{00AD}' + | '\u{061C}' + | '\u{180E}' + | '\u{200B}'..='\u{200F}' + | '\u{202A}'..='\u{202E}' + | '\u{2060}'..='\u{2064}' + | '\u{2066}'..='\u{206F}' + | '\u{FEFF}' + ) +} + +/// Extract the last path segment from a URL path string for use as a download filename +fn filename_from_url(url: &str) -> Option { + let raw = url + .rsplit('/') + .next() + .filter(|s| !s.is_empty()) + // Strip any query-string that may be present + .map(|s| s.split_once('?').map_or(s, |(before, _)| before))?; + + let decoded = percent_decode_str(raw).decode_utf8_lossy(); + sanitize_download_filename(decoded.as_ref()) +} + +fn download_filename_from_url( + url: &str, + output_file_type: Option, +) -> Option { + let filename = filename_from_url(url)?; + + if let Some(file_type) = output_file_type { + Some(filename_with_extension(&filename, file_type)) + } else { + Some(filename) + } +} + +fn filename_with_extension(filename: &str, file_type: PictrsFileType) -> String { + let stem = match filename.rsplit_once('.') { + Some((stem, _)) if !stem.is_empty() => stem, + _ => filename, + }; + + format!("{stem}.{}", file_type) +} + /// 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 @@ -147,7 +238,11 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Sat, 9 May 2026 23:18:57 -0400 Subject: [PATCH 02/14] address PR feedback, simplify code --- api_tests/src/image.spec.ts | 19 ++- crates/routes/src/images/download.rs | 239 ++++----------------------- 2 files changed, 53 insertions(+), 205 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 3e2763058c..81a0728679 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -40,8 +40,25 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); +function percentEncodeNonAlphanumeric(str: string): string { + const bytes = new TextEncoder().encode(str); + let result = ""; + for (const byte of bytes) { + if ( + (byte >= 0x41 && byte <= 0x5a) || // A-Z + (byte >= 0x61 && byte <= 0x7a) || // a-z + (byte >= 0x30 && byte <= 0x39) // 0-9 + ) { + result += String.fromCharCode(byte); + } else { + result += "%" + byte.toString(16).toUpperCase().padStart(2, "0"); + } + } + return result; +} + function inlineContentDisposition(filename: string): string { - return `inline; filename="${filename}"`; + return `inline; filename="${percentEncodeNonAlphanumeric(filename)}"`; } function filenameFromUrl(url: string): string { diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index c14d838ceb..088806ac36 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -118,11 +118,11 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } - if let Some(disposition) = download_filename - .as_deref() - .and_then(inline_content_disposition) - { - client_res.insert_header((actix_web::http::header::CONTENT_DISPOSITION, disposition)); + if let Some(filename) = download_filename { + client_res.insert_header(( + actix_web::http::header::CONTENT_DISPOSITION, + inline_content_disposition(&filename), + )); } Ok(client_res.body(BodyStream::new(res.bytes_stream()))) @@ -142,89 +142,29 @@ enum PictrsFileType { } /// Format a `Content-Disposition: inline` header value for the given filename. -fn inline_content_disposition(name: &str) -> Option { - let sanitized = sanitize_download_filename(name)?; - - if sanitized.is_ascii() { - Some(format!("inline; filename=\"{}\"", sanitized)) - } else { - // use filename* for non-ASCII names. - // Also provide a plain ASCII fallback (non-ASCII chars replaced with '_') - // for older clients that don't understand filename*. - let ascii_fallback: String = sanitized - .chars() - .map(|c| if c.is_ascii() { c } else { '_' }) - .collect(); - let encoded = utf8_percent_encode(&sanitized, NON_ALPHANUMERIC).to_string(); - Some(format!( - "inline; filename=\"{}\"; filename*=UTF-8''{}", - ascii_fallback, encoded - )) - } -} - -fn sanitize_download_filename(name: &str) -> Option { - let mut sanitized = name.to_string(); - sanitized.retain(|c| !is_unsafe_download_filename_char(c)); - - if sanitized.is_empty() { - None - } else { - Some(sanitized) - } -} - -fn is_unsafe_download_filename_char(c: char) -> bool { - c.is_control() - || matches!( - c, - '"' - | '\\' - | '/' - | '\u{00AD}' - | '\u{061C}' - | '\u{180E}' - | '\u{200B}'..='\u{200F}' - | '\u{202A}'..='\u{202E}' - | '\u{2060}'..='\u{2064}' - | '\u{2066}'..='\u{206F}' - | '\u{FEFF}' - ) -} - -/// Extract the last path segment from a URL path string for use as a download filename -fn filename_from_url(url: &str) -> Option { - let raw = url - .rsplit('/') - .next() - .filter(|s| !s.is_empty()) - // Strip any query-string that may be present - .map(|s| s.split_once('?').map_or(s, |(before, _)| before))?; - - let decoded = percent_decode_str(raw).decode_utf8_lossy(); - sanitize_download_filename(decoded.as_ref()) +fn inline_content_disposition(name: &str) -> String { + let encoded = utf8_percent_encode(name, NON_ALPHANUMERIC).to_string(); + format!("inline; filename=\"{}\"", encoded) } fn download_filename_from_url( - url: &str, + path: &str, output_file_type: Option, ) -> Option { - let filename = filename_from_url(url)?; - - if let Some(file_type) = output_file_type { - Some(filename_with_extension(&filename, file_type)) - } else { - Some(filename) - } -} - -fn filename_with_extension(filename: &str, file_type: PictrsFileType) -> String { - let stem = match filename.rsplit_once('.') { - Some((stem, _)) if !stem.is_empty() => stem, - _ => filename, - }; - - format!("{stem}.{}", file_type) + let raw = path.rsplit('/').next()?.split('?').next()?; + let decoded = percent_decode_str(raw).decode_utf8_lossy(); + let name = decoded.as_ref(); + + output_file_type.map_or_else( + || Some(name.to_string()), + |ft| { + let stem = match name.rsplit_once('.') { + Some((stem, _)) if !stem.is_empty() => stem, + _ => name, + }; + Some(format!("{stem}.{ft}")) + }, + ) } /// Take file type from param, name, or use jpg if nothing is given @@ -238,10 +178,7 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Sat, 9 May 2026 23:38:44 -0400 Subject: [PATCH 03/14] address additional pr comments --- crates/routes/src/images/download.rs | 100 +++++++++++++++++++-------- 1 file changed, 73 insertions(+), 27 deletions(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 3519dd18fe..99e2b49ccf 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -1,6 +1,6 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ - HttpRequest, HttpResponse, Responder, + HttpRequest, HttpResponse, HttpResponseBuilder, Responder, body::{BodyStream, BoxBody}, http::StatusCode, web::{Data, *}, @@ -70,7 +70,7 @@ pub async fn image_proxy( 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 processed_url = if let Some(ref file_type) = output_file_type { let mut url = format!( "{}image/process.{}?proxy={}", pictrs_config.url, file_type, encoded_url @@ -131,12 +131,7 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } - if let Some(filename) = download_filename { - client_res.insert_header(( - actix_web::http::header::CONTENT_DISPOSITION, - inline_content_disposition(&filename), - )); - } + set_content_disposition(&mut client_res, download_filename.as_deref()); Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } @@ -159,22 +154,42 @@ enum PictrsFileType { Webp, } -/// Format a `Content-Disposition: inline` header value for the given filename. -fn inline_content_disposition(name: &str) -> String { - let encoded = utf8_percent_encode(name, NON_ALPHANUMERIC).to_string(); - format!("inline; filename=\"{}\"", encoded) +fn set_content_disposition(client_res: &mut HttpResponseBuilder, filename: Option<&str>) { + if let Some(name) = filename { + let encoded = utf8_percent_encode(name, NON_ALPHANUMERIC).to_string(); + client_res.insert_header(( + actix_web::http::header::CONTENT_DISPOSITION, + format!("inline; filename=\"{}\"", encoded), + )); + } } fn download_filename_from_url( path: &str, output_file_type: Option, ) -> Option { - let raw = path.rsplit('/').next()?.split('?').next()?; + let raw = path + .rsplit('/') + .next()? + .split('?') + .next() + .filter(|s| !s.is_empty())?; let decoded = percent_decode_str(raw).decode_utf8_lossy(); let name = decoded.as_ref(); output_file_type.map_or_else( - || Some(name.to_string()), + || { + let has_extension = name + .rsplit_once('.') + .map(|(stem, _)| !stem.is_empty()) + .unwrap_or(false); + let filename = if has_extension { + name.to_string() + } else { + format!("{name}.jpg") + }; + Some(filename) + }, |ft| { let stem = match name.rsplit_once('.') { Some((stem, _)) if !stem.is_empty() => stem, @@ -196,7 +211,8 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Sun, 10 May 2026 20:03:24 -0400 Subject: [PATCH 04/14] format --- api_tests/src/image.spec.ts | 2 +- crates/routes/src/images/download.rs | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 3b472beaf4..228edd7fa1 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -47,7 +47,7 @@ function percentEncodeNonAlphanumeric(str: string): string { if ( (byte >= 0x41 && byte <= 0x5a) || // A-Z (byte >= 0x61 && byte <= 0x7a) || // a-z - (byte >= 0x30 && byte <= 0x39) // 0-9 + (byte >= 0x30 && byte <= 0x39) // 0-9 ) { result += String.fromCharCode(byte); } else { diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 99e2b49ccf..d8f01d0e63 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -1,6 +1,9 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ - HttpRequest, HttpResponse, HttpResponseBuilder, Responder, + HttpRequest, + HttpResponse, + HttpResponseBuilder, + Responder, body::{BodyStream, BoxBody}, http::StatusCode, web::{Data, *}, @@ -212,8 +215,8 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Mon, 11 May 2026 21:27:31 -0400 Subject: [PATCH 05/14] address pr feedback --- Cargo.lock | 1 + api_tests/src/image.spec.ts | 34 +++---------- crates/routes/Cargo.toml | 1 + crates/routes/src/images/download.rs | 75 ++++++++++++---------------- 4 files changed, 43 insertions(+), 68 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78f5e41450..481c461d31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4233,6 +4233,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 228edd7fa1..343c3b237d 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -40,30 +40,12 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); -function percentEncodeNonAlphanumeric(str: string): string { - const bytes = new TextEncoder().encode(str); - let result = ""; - for (const byte of bytes) { - if ( - (byte >= 0x41 && byte <= 0x5a) || // A-Z - (byte >= 0x61 && byte <= 0x7a) || // a-z - (byte >= 0x30 && byte <= 0x39) // 0-9 - ) { - result += String.fromCharCode(byte); - } else { - result += "%" + byte.toString(16).toUpperCase().padStart(2, "0"); - } - } - return result; -} +const percentEncodeNonAlphanumeric = (str: string): string => encodeURIComponent(str); function inlineContentDisposition(filename: string): string { return `inline; filename="${percentEncodeNonAlphanumeric(filename)}"`; } -function filenameFromUrl(url: string): string { - return decodeURIComponent(new URL(url).pathname.split("/").pop() ?? ""); -} test("Upload image and delete it", async () => { const health = await alpha.imageHealth(); @@ -193,7 +175,7 @@ test("Purge post, linked image removed", async () => { }); test("Images in remote image post are proxied if setting enabled", async () => { - const expectedFilename = filenameFromUrl(sampleImage); + const expectedFilename = decodeURIComponent(new URL(sampleImage).pathname.split("/").pop() ?? ""); let community = await createCommunity(gamma); let postRes = await createPost( @@ -224,9 +206,9 @@ test("Images in remote image post are proxied if setting enabled", async () => { // Proxied image should include a Content-Disposition: inline header if (post.thumbnail_url) { const proxyResponse = await fetch(post.thumbnail_url); - const cd = proxyResponse.headers.get("content-disposition"); - expect(cd).not.toBeNull(); - expect(cd).toBe(inlineContentDisposition(expectedFilename)); + const contentDisposition = proxyResponse.headers.get("content-disposition"); + expect(contentDisposition).not.toBeNull(); + expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); } let epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post); @@ -256,9 +238,9 @@ test("Images in remote image post are proxied if setting enabled", async () => { if (epsilonPost.thumbnail_url) { const proxyResponse = await fetch(epsilonPost.thumbnail_url); - const cd = proxyResponse.headers.get("content-disposition"); - expect(cd).not.toBeNull(); - expect(cd).toBe(inlineContentDisposition(expectedFilename)); + const contentDisposition = proxyResponse.headers.get("content-disposition"); + expect(contentDisposition).not.toBeNull(); + expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); } }); diff --git a/crates/routes/Cargo.toml b/crates/routes/Cargo.toml index e5240255a6..159fdd4f0b 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 = "0.10.0" 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 d8f01d0e63..5fde1788a0 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -1,11 +1,8 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ - HttpRequest, - HttpResponse, - HttpResponseBuilder, - Responder, + HttpRequest, HttpResponse, HttpResponseBuilder, Responder, body::{BodyStream, BoxBody}, - http::StatusCode, + http::{header::CONTENT_DISPOSITION, StatusCode}, web::{Data, *}, }; use lemmy_api_utils::context::LemmyContext; @@ -14,7 +11,7 @@ use lemmy_db_views_local_image::api::{ImageGetParams, ImageProxyParams}; use lemmy_db_views_local_user::LocalUserView; use lemmy_db_views_site::SiteView; use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; -use percent_encoding::{NON_ALPHANUMERIC, percent_decode_str, utf8_percent_encode}; +use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode}; use std::str::FromStr; use strum::{Display, EnumString}; use url::Url; @@ -31,14 +28,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); @@ -48,7 +45,7 @@ pub async fn get_image( url }; - do_get_image(processed_url, req, &context, None).await + do_get_image(processed_url, req, &context, Some(name)).await } pub async fn image_proxy( @@ -159,14 +156,19 @@ enum PictrsFileType { fn set_content_disposition(client_res: &mut HttpResponseBuilder, filename: Option<&str>) { if let Some(name) = filename { - let encoded = utf8_percent_encode(name, NON_ALPHANUMERIC).to_string(); + let encoded = urlencoding::encode(name); client_res.insert_header(( - actix_web::http::header::CONTENT_DISPOSITION, + 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, @@ -177,30 +179,19 @@ fn download_filename_from_url( .split('?') .next() .filter(|s| !s.is_empty())?; - let decoded = percent_decode_str(raw).decode_utf8_lossy(); + let decoded = urlencoding::decode(raw).unwrap_or_else(|_| raw.into()); let name = decoded.as_ref(); - output_file_type.map_or_else( - || { - let has_extension = name - .rsplit_once('.') - .map(|(stem, _)| !stem.is_empty()) - .unwrap_or(false); - let filename = if has_extension { - name.to_string() - } else { - format!("{name}.jpg") - }; - Some(filename) - }, - |ft| { - let stem = match name.rsplit_once('.') { - Some((stem, _)) if !stem.is_empty() => stem, - _ => name, - }; - Some(format!("{stem}.{ft}")) - }, - ) + 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 @@ -216,7 +207,7 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Mon, 11 May 2026 21:34:11 -0400 Subject: [PATCH 06/14] fix failing ci checks --- api_tests/src/image.spec.ts | 8 ++++--- crates/routes/src/images/download.rs | 34 +++++++++++----------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 343c3b237d..d925eec1c8 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -40,13 +40,13 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); -const percentEncodeNonAlphanumeric = (str: string): string => encodeURIComponent(str); +const percentEncodeNonAlphanumeric = (str: string): string => + encodeURIComponent(str); function inlineContentDisposition(filename: string): string { return `inline; filename="${percentEncodeNonAlphanumeric(filename)}"`; } - test("Upload image and delete it", async () => { const health = await alpha.imageHealth(); expect(health.success).toBeTruthy(); @@ -175,7 +175,9 @@ 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 expectedFilename = decodeURIComponent( + new URL(sampleImage).pathname.split("/").pop() ?? "", + ); let community = await createCommunity(gamma); let postRes = await createPost( diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 5fde1788a0..22d6f86e95 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -1,8 +1,11 @@ use super::utils::{adapt_request, convert_header}; use actix_web::{ - HttpRequest, HttpResponse, HttpResponseBuilder, Responder, + HttpRequest, + HttpResponse, + HttpResponseBuilder, + Responder, body::{BodyStream, BoxBody}, - http::{header::CONTENT_DISPOSITION, StatusCode}, + http::{StatusCode, header::CONTENT_DISPOSITION}, web::{Data, *}, }; use lemmy_api_utils::context::LemmyContext; @@ -207,7 +210,10 @@ fn file_type(file_type: Option, name: &str) -> LemmyResult Date: Mon, 11 May 2026 22:00:35 -0400 Subject: [PATCH 07/14] fix cargo clippy --- crates/routes/src/images/download.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 22d6f86e95..ba337744c0 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -73,7 +73,7 @@ pub async fn image_proxy( 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(ref file_type) = output_file_type { + 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 @@ -306,21 +306,21 @@ mod tests { // ASCII filename: URL-encoded, preserving characters allowed by urlencoding::encode set_content_disposition(&mut builder, Some("photo.jpg")); let res = builder.finish(); - let header = res.headers().get(CONTENT_DISPOSITION).unwrap(); + let header = res.headers().get(CONTENT_DISPOSITION).expect("header should be set"); assert_eq!(header, "inline; filename=\"photo.jpg\""); // Spaces are encoded let mut builder2 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder2, Some("my photo.jpg")); let res2 = builder2.finish(); - let header2 = res2.headers().get(CONTENT_DISPOSITION).unwrap(); + let header2 = res2.headers().get(CONTENT_DISPOSITION).expect("header should be set"); assert_eq!(header2, "inline; filename=\"my%20photo.jpg\""); // Non-ASCII characters are UTF-8 percent-encoded let mut builder3 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder3, Some("héron.jpg")); let res3 = builder3.finish(); - let header3 = res3.headers().get(CONTENT_DISPOSITION).unwrap(); + let header3 = res3.headers().get(CONTENT_DISPOSITION).expect("header should be set"); assert_eq!(header3, "inline; filename=\"h%C3%A9ron.jpg\""); // None sets no header From 5f46c32add79f1b6ca51e9a6a399f282852b7fcd Mon Sep 17 00:00:00 2001 From: eduardo Date: Mon, 11 May 2026 22:06:16 -0400 Subject: [PATCH 08/14] run rust formatting --- crates/routes/src/images/download.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index ba337744c0..390285168b 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -306,21 +306,30 @@ mod tests { // ASCII filename: URL-encoded, preserving characters allowed by urlencoding::encode set_content_disposition(&mut builder, Some("photo.jpg")); let res = builder.finish(); - let header = res.headers().get(CONTENT_DISPOSITION).expect("header should be set"); + let header = res + .headers() + .get(CONTENT_DISPOSITION) + .expect("header should be set"); assert_eq!(header, "inline; filename=\"photo.jpg\""); // Spaces are encoded let mut builder2 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder2, Some("my photo.jpg")); let res2 = builder2.finish(); - let header2 = res2.headers().get(CONTENT_DISPOSITION).expect("header should be set"); + let header2 = res2 + .headers() + .get(CONTENT_DISPOSITION) + .expect("header should be set"); assert_eq!(header2, "inline; filename=\"my%20photo.jpg\""); // Non-ASCII characters are UTF-8 percent-encoded let mut builder3 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder3, Some("héron.jpg")); let res3 = builder3.finish(); - let header3 = res3.headers().get(CONTENT_DISPOSITION).expect("header should be set"); + let header3 = res3 + .headers() + .get(CONTENT_DISPOSITION) + .expect("header should be set"); assert_eq!(header3, "inline; filename=\"h%C3%A9ron.jpg\""); // None sets no header From a185fe33ad7fde0a6d615412df3e74f1ba3fbaaf Mon Sep 17 00:00:00 2001 From: eduardo Date: Tue, 12 May 2026 22:29:40 -0400 Subject: [PATCH 09/14] address clippy error with expect(), clean up redundant checks in test. set_content_disposition only called if a download_filename exist. filename is no longer optional in set_content_disposition --- api_tests/src/image.spec.ts | 5 +--- crates/routes/src/images/download.rs | 45 ++++++++++------------------ 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index d925eec1c8..3051997c34 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -40,11 +40,8 @@ afterAll(async () => { await Promise.allSettled([unfollows(), deleteAllMedia(alpha)]); }); -const percentEncodeNonAlphanumeric = (str: string): string => - encodeURIComponent(str); - function inlineContentDisposition(filename: string): string { - return `inline; filename="${percentEncodeNonAlphanumeric(filename)}"`; + return `inline; filename="${encodeURIComponent(filename)}"`; } test("Upload image and delete it", async () => { diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 390285168b..4859185c9c 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -134,7 +134,9 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } - set_content_disposition(&mut client_res, download_filename.as_deref()); + if let Some(ref download_filename) = download_filename { + set_content_disposition(&mut client_res, download_filename); + } Ok(client_res.body(BodyStream::new(res.bytes_stream()))) } @@ -157,14 +159,12 @@ enum PictrsFileType { Webp, } -fn set_content_disposition(client_res: &mut HttpResponseBuilder, filename: Option<&str>) { - if let Some(name) = filename { - let encoded = urlencoding::encode(name); - client_res.insert_header(( - CONTENT_DISPOSITION, - format!("inline; filename=\"{}\"", encoded), - )); - } +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 @@ -304,38 +304,23 @@ mod tests { let mut builder = HttpResponse::build(StatusCode::OK); // ASCII filename: URL-encoded, preserving characters allowed by urlencoding::encode - set_content_disposition(&mut builder, Some("photo.jpg")); + set_content_disposition(&mut builder, "photo.jpg"); let res = builder.finish(); - let header = res - .headers() - .get(CONTENT_DISPOSITION) - .expect("header should be set"); + let header = res.headers().get(CONTENT_DISPOSITION).unwrap(); assert_eq!(header, "inline; filename=\"photo.jpg\""); // Spaces are encoded let mut builder2 = HttpResponse::build(StatusCode::OK); - set_content_disposition(&mut builder2, Some("my photo.jpg")); + set_content_disposition(&mut builder2, "my photo.jpg"); let res2 = builder2.finish(); - let header2 = res2 - .headers() - .get(CONTENT_DISPOSITION) - .expect("header should be set"); + let header2 = res2.headers().get(CONTENT_DISPOSITION).unwrap(); assert_eq!(header2, "inline; filename=\"my%20photo.jpg\""); // Non-ASCII characters are UTF-8 percent-encoded let mut builder3 = HttpResponse::build(StatusCode::OK); - set_content_disposition(&mut builder3, Some("héron.jpg")); + set_content_disposition(&mut builder3, "héron.jpg"); let res3 = builder3.finish(); - let header3 = res3 - .headers() - .get(CONTENT_DISPOSITION) - .expect("header should be set"); + let header3 = res3.headers().get(CONTENT_DISPOSITION).unwrap(); assert_eq!(header3, "inline; filename=\"h%C3%A9ron.jpg\""); - - // None sets no header - let mut builder4 = HttpResponse::build(StatusCode::OK); - set_content_disposition(&mut builder4, None); - let res4 = builder4.finish(); - assert!(res4.headers().get(CONTENT_DISPOSITION).is_none()); } } From bfa3d12d2e8a45527338f2483af170beba93d9a4 Mon Sep 17 00:00:00 2001 From: eduardo Date: Wed, 13 May 2026 21:15:31 -0400 Subject: [PATCH 10/14] remove ref use & --- crates/routes/src/images/download.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 4859185c9c..4be4ec569e 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -134,7 +134,7 @@ pub(super) async fn do_get_image( client_res.insert_header(convert_header(name, value)); } - if let Some(ref download_filename) = download_filename { + if let Some(download_filename) = &download_filename { set_content_disposition(&mut client_res, download_filename); } From 1c7ab811af67ce51dd33f2361432913722a6b338 Mon Sep 17 00:00:00 2001 From: eduardo Date: Sat, 23 May 2026 21:13:19 -0400 Subject: [PATCH 11/14] remove redundant not.toBeNull assertions in image proxy test --- api_tests/src/image.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 3051997c34..12cbb32c8b 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -206,7 +206,6 @@ test("Images in remote image post are proxied if setting enabled", async () => { if (post.thumbnail_url) { const proxyResponse = await fetch(post.thumbnail_url); const contentDisposition = proxyResponse.headers.get("content-disposition"); - expect(contentDisposition).not.toBeNull(); expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); } @@ -238,7 +237,6 @@ test("Images in remote image post are proxied if setting enabled", async () => { if (epsilonPost.thumbnail_url) { const proxyResponse = await fetch(epsilonPost.thumbnail_url); const contentDisposition = proxyResponse.headers.get("content-disposition"); - expect(contentDisposition).not.toBeNull(); expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); } }); From 5580e7e830543e4296b87bee13e50ab6a1ed2b7c Mon Sep 17 00:00:00 2001 From: eduardo Date: Sat, 23 May 2026 23:41:09 -0400 Subject: [PATCH 12/14] Fix clippy unwrap lint in content disposition test --- crates/routes/src/images/download.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 4be4ec569e..952580eda6 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -306,21 +306,35 @@ mod tests { // ASCII filename: URL-encoded, preserving characters allowed by urlencoding::encode set_content_disposition(&mut builder, "photo.jpg"); let res = builder.finish(); - let header = res.headers().get(CONTENT_DISPOSITION).unwrap(); - assert_eq!(header, "inline; filename=\"photo.jpg\""); + assert_eq!( + res.headers() + .get(CONTENT_DISPOSITION) + .and_then(|header| header.to_str().ok()), + Some("inline; filename=\"photo.jpg\"") + ); // Spaces are encoded let mut builder2 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder2, "my photo.jpg"); let res2 = builder2.finish(); - let header2 = res2.headers().get(CONTENT_DISPOSITION).unwrap(); - assert_eq!(header2, "inline; filename=\"my%20photo.jpg\""); + assert_eq!( + res2 + .headers() + .get(CONTENT_DISPOSITION) + .and_then(|header| header.to_str().ok()), + Some("inline; filename=\"my%20photo.jpg\"") + ); // Non-ASCII characters are UTF-8 percent-encoded let mut builder3 = HttpResponse::build(StatusCode::OK); set_content_disposition(&mut builder3, "héron.jpg"); let res3 = builder3.finish(); - let header3 = res3.headers().get(CONTENT_DISPOSITION).unwrap(); - assert_eq!(header3, "inline; filename=\"h%C3%A9ron.jpg\""); + assert_eq!( + res3 + .headers() + .get(CONTENT_DISPOSITION) + .and_then(|header| header.to_str().ok()), + Some("inline; filename=\"h%C3%A9ron.jpg\"") + ); } } From c7c4083d4e5ef9b82b4d6d8ba98fbf4c2a435602 Mon Sep 17 00:00:00 2001 From: eduardo Date: Sat, 23 May 2026 23:48:04 -0400 Subject: [PATCH 13/14] Format content disposition test assertion --- crates/routes/src/images/download.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/routes/src/images/download.rs b/crates/routes/src/images/download.rs index 952580eda6..4c537952c8 100644 --- a/crates/routes/src/images/download.rs +++ b/crates/routes/src/images/download.rs @@ -307,7 +307,8 @@ mod tests { set_content_disposition(&mut builder, "photo.jpg"); let res = builder.finish(); assert_eq!( - res.headers() + res + .headers() .get(CONTENT_DISPOSITION) .and_then(|header| header.to_str().ok()), Some("inline; filename=\"photo.jpg\"") From 6f25c62cfd69a9f86c3194d31c9d67ec44721d1f Mon Sep 17 00:00:00 2001 From: eduardo Date: Sun, 24 May 2026 22:19:23 -0400 Subject: [PATCH 14/14] Fix image proxy test flakiness by retrying Content-Disposition check The 'Images in remote image post are proxied if setting enabled' test was fetching the proxied thumbnail URL immediately after post creation, before pictrs had finished downloading and caching the remote image. This caused a race condition where the Content-Disposition header was sometimes missing, leading to CI failures. Replace the single-shot fetch with a waitUntilSuccess retry loop that polls the proxied URL until the response is OK and the expected Content-Disposition header is present. This matches the existing pattern used elsewhere in the test suite for waiting on backgrounded pictrs operations. --- api_tests/src/image.spec.ts | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 27d7b2f994..10fa4ee5a1 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -45,6 +45,27 @@ 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(); @@ -218,9 +239,10 @@ test("Images in remote image post are proxied if setting enabled", async () => { // Proxied image should include a Content-Disposition: inline header if (post.thumbnail_url) { - const proxyResponse = await fetch(post.thumbnail_url); - const contentDisposition = proxyResponse.headers.get("content-disposition"); - expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); + await expectProxiedImageContentDisposition( + post.thumbnail_url, + expectedFilename, + ); } const epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post); @@ -249,9 +271,10 @@ test("Images in remote image post are proxied if setting enabled", async () => { expect(epsilonPost.thumbnail_url?.includes(".jpg")).toBeTruthy(); if (epsilonPost.thumbnail_url) { - const proxyResponse = await fetch(epsilonPost.thumbnail_url); - const contentDisposition = proxyResponse.headers.get("content-disposition"); - expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename)); + await expectProxiedImageContentDisposition( + epsilonPost.thumbnail_url, + expectedFilename, + ); } });