Add Content-Disposition header for proxied images by EduardoLZevallos · Pull Request #6440 · LemmyNet/lemmy

EduardoLZevallos · 2026-04-12T20:32:40Z

Fix the download filename for proxied images by setting an inline Content-Disposition header on proxy responses.

This change derives the filename from the proxied image URL, sanitizes it, and uses it in the response header so downloads keep a meaningful name.

This is intentionally scoped to proxied images only, matching the behavior requested in #6354. Direct image downloads are left unchanged.

Copilot

Pull request overview

Adds a meaningful download filename for proxied images by deriving a sanitized filename from the original remote image URL and sending it via an inline Content-Disposition header on proxy responses (scoped to /image/proxy only, leaving direct image downloads unchanged as requested in #6354).

Changes:

Extend the proxy response path to compute a download filename (including adjusted extension when processing params are used) and inject a Content-Disposition: inline header.
Add filename extraction + sanitization helpers to prevent header injection/spoofing characters.
Add Rust unit tests for filename/disposition behavior and API tests asserting the new header for proxied thumbnails.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`crates/routes/src/images/download.rs`	Computes a safe filename from proxied URL paths and sets `Content-Disposition` for proxied image responses; adds unit tests.
`api_tests/src/image.spec.ts`	Adds integration assertions that proxied image responses include the expected `Content-Disposition` header.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Nutomic · 2026-04-13T10:07:01Z

+fn sanitize_download_filename(name: &str) -> Option<String> {
+  let mut sanitized = name.to_string();
+  sanitized.retain(|c| !is_unsafe_download_filename_char(c));
+
+  if sanitized.is_empty() {
+    None
+  } else {
+    Some(sanitized)
+  }


Whats with the AI review, did you enable this?

yea. Didn't think it'd just automatically add it to every repo I create a PR. Will turn it off sorry about that. Thanks for feedback will take a look this week.

Nutomic · 2026-04-13T09:56:44Z

+        | '\u{2060}'..='\u{2064}'
+        | '\u{2066}'..='\u{206F}'
+        | '\u{FEFF}'
+    )


Please add a link with details what these chars are.

Nutomic · 2026-04-13T09:59:01Z


-  if let Some(addr) = req.head().peer_addr {
-    client_req = client_req.header("X-Forwarded-For", addr.to_string());
-  }


Dont remove this

Still needs restoring

Nevermind this was actually duplicated, so good to remove it.

Nutomic · 2026-04-13T10:00:22Z

 }

+/// Format a `Content-Disposition: inline` header value for the given filename.
+fn inline_content_disposition(name: &str) -> Option<String> {


Rename to set_content_disposition, pass in &mut HttpResponse and set the header directly in this method. Then its not necessary to have an if statement in do_get_image().

Nutomic · 2026-04-13T10:03:20Z

+}
+
+/// Extract the last path segment from a URL path string for use as a download filename
+fn filename_from_url(url: &str) -> Option<String> {


This method is only called once so move the code inline. Some other methods also look unnecessary.

Nutomic · 2026-04-13T10:06:39Z

+  if let Some(file_type) = output_file_type {
+    Some(filename_with_extension(&filename, file_type))
+  } else {
+    Some(filename)


This means the file will be saved without extension and users wont be able to open it. If theres no better option at least fallback to .jpg so it can be opend with an image viewer (which will also be able to handle other image types).

There's also already a file_type function that's handles this, so this entire function isn't necessary.

Nutomic · 2026-04-13T10:07:01Z

+fn sanitize_download_filename(name: &str) -> Option<String> {
+  let mut sanitized = name.to_string();
+  sanitized.retain(|c| !is_unsafe_download_filename_char(c));
+
+  if sanitized.is_empty() {
+    None
+  } else {
+    Some(sanitized)
+  }


Whats with the AI review, did you enable this?

dessalines

Remove all this manual sanitization, and use utf8_percent_encode like the other image downloads are doing.

Also there are a bunch of duped functions that makes it seem like this was AI coded.

dessalines · 2026-04-16T17:00:05Z

+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)


What is this for? The file_type function already exists.

dessalines · 2026-04-16T17:01:20Z

+  if let Some(file_type) = output_file_type {
+    Some(filename_with_extension(&filename, file_type))
+  } else {
+    Some(filename)


There's also already a file_type function that's handles this, so this entire function isn't necessary.

dessalines · 2026-05-05T14:11:04Z

bump @EduardoLZevallos

EduardoLZevallos · 2026-05-05T14:42:10Z

bump @EduardoLZevallos

Apologies. I’m going to pick this back up later in the week / this weekend.

dessalines · 2026-05-05T16:22:55Z

No probs, thx.

Nutomic · 2026-05-12T12:35:11Z

+  if (post.thumbnail_url) {
+    const proxyResponse = await fetch(post.thumbnail_url);
+    const contentDisposition = proxyResponse.headers.get("content-disposition");
+    expect(contentDisposition).not.toBeNull();


This line is redundant as you are checking against a concrete value below. Same in line 244.

This is not resolved.

@EduardoLZevallos this one too.

…. set_content_disposition only called if a download_filename exist. filename is no longer optional in set_content_disposition

Nutomic · 2026-05-13T08:14:28Z

CI error from clippy:

error: usage of ref pattern
   --> crates/routes/src/images/download.rs:137:15
    |
137 |   if let Some(ref download_filename) = download_filename {
    |               ^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider using `&` for clarity instead
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#ref_patterns
    = note: requested on the command line with `-D clippy::ref-patterns`

dessalines

Other than the one comment above from nutomic, everything looks good to me.

…ilename-proxied-images # Conflicts: # api_tests/src/image.spec.ts

EduardoLZevallos · 2026-05-24T22:00:19Z

run_federation_tests are failing.

FAIL src/image.spec.ts (49.512 s)
  ✓ Upload image and delete it (184 ms)
  ✓ Purge user, uploaded image removed (1585 ms)
  ✓ Purge post, linked image removed (2321 ms)
  ✕ Images in remote image post are proxied if setting enabled (11012 ms)
  ✓ Thumbnail of remote image link is proxied if setting enabled (2223 ms)
  ✓ No image proxying if setting is disabled (5792 ms)
  ✓ Make regular post, and give it a custom thumbnail (663 ms)
  ✓ Create an image post, and make sure a custom thumbnail doesn't overwrite it (628 ms)

  ● Images in remote image post are proxied if setting enabled

    expect(received).toBe(expected) // Object.is equality

    Expected: "inline; filename=\"df5f5b1b174a2b4b6026cc6c8f9395c1.jpg\""
    Received: null

      221 |     const proxyResponse = await fetch(post.thumbnail_url);
      222 |     const contentDisposition = proxyResponse.headers.get("content-disposition");
    > 223 |     expect(contentDisposition).toBe(inlineContentDisposition(expectedFilename));
          |                                ^
      224 |   }
      225 |
      226 |   const epsilonPostRes = await resolvePost(epsilon, postRes.post_view.post);

      at Object.<anonymous> (src/image.spec.ts:223:32)

Test Suites: 1 failed, 5 passed, 6 of 10 total
Tests:       1 failed, 2 skipped, 83 passed, 86 total

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.

Nutomic · 2026-05-25T10:44:44Z

Looks like content disposition is not included in the response. Add dbg!() statements to the Rust code to see where its going wrong.

EduardoLZevallos · 2026-05-31T02:07:30Z

Looks like content disposition is not included in the response. Add dbg!() statements to the Rust code to see where its going wrong.

will do

Add Content-Disposition header for proxied images

c491b6e

EduardoLZevallos requested review from Nutomic, dessalines, dullbananas and phiresky as code owners April 12, 2026 20:32

Copilot AI review requested due to automatic review settings April 12, 2026 20:32

Copilot started reviewing on behalf of EduardoLZevallos April 12, 2026 20:33 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

EduardoLZevallos mentioned this pull request Apr 12, 2026

Download filename for proxied images #6354

Open

Nutomic reviewed Apr 13, 2026

View reviewed changes

Comment thread crates/routes/src/images/download.rs

Nutomic reviewed Apr 13, 2026

View reviewed changes

LemmyNet deleted a comment from Copilot AI Apr 16, 2026

dessalines reviewed Apr 16, 2026

View reviewed changes

eduardo and others added 4 commits May 9, 2026 23:18

address PR feedback, simplify code

e190303

Merge branch 'main' into fix/6354-download-filename-proxied-images

6b7ef23

address additional pr comments

a88c6bf

format

efcae76