Add Content-Disposition header for proxied images

api_tests/src/image.spec.ts

@@ -40,6 +40,31 @@ 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="${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();
   expect(health.success).toBeTruthy();
@@ -168,6 +193,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,
@@ -194,6 +221,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();
 
@@ -218,6 +253,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 () => {

crates/routes/src/images/download.rs

@@ -2,6 +2,7 @@ use super::utils::{adapt_request, convert_header};
 use actix_web::{
   HttpRequest,
   HttpResponse,
+  HttpResponseBuilder,
   Responder,
   body::{BodyStream, BoxBody},
   http::StatusCode,
@@ -13,7 +14,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, 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;
@@ -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, None).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(ref 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,52 @@ enum PictrsFileType {
   Webp,
 }
 
+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<PictrsFileType>,
+) -> Option<String> {
+  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(
+    || {
+      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}"))
+    },
+  )
+}
+
 /// 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 +214,9 @@ 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};
   use lemmy_utils::error::LemmyResult;
 
   #[tokio::test]
@@ -215,4 +266,84 @@ 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: all non-alphanumeric characters are percent-encoded
+    set_content_disposition(&mut builder, Some("photo.jpg"));
+    let res = builder.finish();
+    let header = res
+      .headers()
+      .get(actix_web::http::header::CONTENT_DISPOSITION)
+      .unwrap();
+    assert_eq!(header, "inline; filename=\"photo%2Ejpg\"");
+
+    // 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(actix_web::http::header::CONTENT_DISPOSITION)
+      .unwrap();
+    assert_eq!(header2, "inline; filename=\"my%20photo%2Ejpg\"");
+
+    // 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(actix_web::http::header::CONTENT_DISPOSITION)
+      .unwrap();
+    assert_eq!(header3, "inline; filename=\"h%C3%A9ron%2Ejpg\"");
+
+    // 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(actix_web::http::header::CONTENT_DISPOSITION)
+        .is_none()
+    );
+  }
 }