Add Content-Disposition header for proxied images

api_tests/src/image.spec.ts

@@ -40,6 +40,13 @@ 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)}"`;
+}
+
 test("Upload image and delete it", async () => {
   const health = await alpha.imageHealth();
   expect(health.success).toBeTruthy();
@@ -168,6 +175,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() ?? "",
+  );
+
   let community = await createCommunity(gamma);
   let postRes = await createPost(
     gamma,
@@ -194,6 +205,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 contentDisposition = proxyResponse.headers.get("content-disposition");
+    expect(contentDisposition).not.toBeNull();
+    expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename));
+  }
+
   let epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post);
   expect(epsilonPostRes?.post).toBeDefined();
 
@@ -218,6 +237,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 contentDisposition = proxyResponse.headers.get("content-disposition");
+    expect(contentDisposition).not.toBeNull();
+    expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename));
+  }
 });
 
 test("Thumbnail of remote image link is proxied if setting enabled", async () => {

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 }

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,17 +114,14 @@ pub(super) async fn do_get_image(
   url: String,
   req: HttpRequest,
   context: &LemmyContext,
+  download_filename: Option<String>,
 ) -> LemmyResult<HttpResponse> {
   let mut client_req = adapt_request(&req, url, context);
 
   if let Some(addr) = req.head().peer_addr {
     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,8 @@ 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());
+
   Ok(client_res.body(BodyStream::new(res.bytes_stream())))
 }
 
@@ -154,6 +157,46 @@ 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),
+    ));
+  }
+}
+
+/// 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<PictrsFileType>,
+) -> Option<String> {
+  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<String>, name: &str) -> LemmyResult<PictrsFileType> {
   let type_str = file_type
@@ -165,7 +208,12 @@ fn file_type(file_type: Option<String>, name: &str) -> LemmyResult<PictrsFileTyp
 
 #[cfg(test)]
 mod tests {
-  use crate::images::download::{PictrsFileType, file_type};
+  use super::{PictrsFileType, download_filename_from_url, set_content_disposition};
+  use crate::images::download::file_type;
+  use actix_web::{
+    HttpResponse,
+    http::{StatusCode, header::CONTENT_DISPOSITION},
+  };
   use lemmy_utils::error::LemmyResult;
 
   #[tokio::test]
@@ -215,4 +263,79 @@ mod tests {
 
     Ok(())
   }
+
+  #[test]
+  fn test_download_filename_from_url() {
+    assert_eq!(
+      download_filename_from_url("/images/photo.png", Some(PictrsFileType::Avif)),
+      Some("photo.avif".to_string())
+    );
+
+    assert_eq!(
+      download_filename_from_url("/images/archive", Some(PictrsFileType::Webp)),
+      Some("archive.webp".to_string())
+    );
+
+    assert_eq!(
+      download_filename_from_url("/images/photo.tar.gz", Some(PictrsFileType::Jpg)),
+      Some("photo.tar.jpg".to_string())
+    );
+
+    assert_eq!(
+      download_filename_from_url("/images/%C3%A9l%C3%A9phant.png", Some(PictrsFileType::Jxl)),
+      Some("éléphant.jxl".to_string())
+    );
+
+    // Without output file type, original extension is preserved
+    assert_eq!(
+      download_filename_from_url("/images/photo.png", None),
+      Some("photo.png".to_string())
+    );
+
+    // Without output file type and no extension, falls back to .jpg
+    assert_eq!(
+      download_filename_from_url("/images/noextension", None),
+      Some("noextension.jpg".to_string())
+    );
+  }
+
+  #[test]
+  fn test_set_content_disposition() {
+    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"));
+    let res = builder.finish();
+    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");
+    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");
+    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());
+  }
 }