From 38343e62ded3d65efcea5c74b7c1e169807754d6 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sun, 17 May 2026 00:25:12 +0200 Subject: [PATCH] fix(downloads): apply original-filename toggle to individual downloads too (#507) Follow-up to #498. The toggle reached zip downloads but single-photo downloads still landed on disk with the renamed `event_individual_NNN.jpg` even when the admin had flipped the setting on. Two reasons, fixed in lockstep: - Frontend overrode the server's Content-Disposition with a hardcoded `` attribute (`gallery.service.ts`, `photos.service.ts`) where X was the sanitized `photo.filename` known to the client. So the backend's correctly-formed `Content-Disposition` never reached the disk write. Added `parseContentDispositionFilename` (RFC 5987 + plain `filename=` fallback) and let the server name win when present. - `secureImages.js` (enhanced/maximum protection's secure-download route) was missed in #498 and still emitted a hardcoded `filename="${photo.filename}"` regardless of the toggle. Wired it through `getUseOriginalFilenames` + `buildContentDisposition` so it matches the regular gallery download path. Also exposed `Content-Disposition` via CORS so split (cross-origin) frontend deployments can still read it from JavaScript. Same-origin Docker deploys already had access; this is a defensive addition for the split case. --- backend/server.js | 7 +++- backend/src/routes/secureImages.js | 14 ++++++- frontend/src/services/gallery.service.ts | 48 +++++++++++++----------- frontend/src/services/photos.service.ts | 12 +++++- frontend/src/utils/contentDisposition.ts | 45 ++++++++++++++++++++++ 5 files changed, 100 insertions(+), 26 deletions(-) create mode 100644 frontend/src/utils/contentDisposition.ts diff --git a/backend/server.js b/backend/server.js index 7746487b..832ce91c 100644 --- a/backend/server.js +++ b/backend/server.js @@ -161,7 +161,12 @@ const corsOptions = { callback(null, false); } }, - credentials: true + credentials: true, + // Expose Content-Disposition so split (cross-origin) frontend + // deployments can read the server's chosen download filename. Used + // by the gallery/admin download flows to honour the #493 "original + // camera filename" toggle on individual photo downloads (#507). + exposedHeaders: ['Content-Disposition'], }; // Only attach CORS to API endpoints, not static assets diff --git a/backend/src/routes/secureImages.js b/backend/src/routes/secureImages.js index 81689e59..79093eee 100644 --- a/backend/src/routes/secureImages.js +++ b/backend/src/routes/secureImages.js @@ -8,6 +8,11 @@ const { formatBoolean } = require('../utils/dbCompat'); const { resolvePhotoFilePath, resolvePhotoStorageKey } = require('../services/photoResolver'); const { withLocalCopy } = require('../services/imageProcessor'); const { getStorage } = require('../services/storage'); +const { + getUseOriginalFilenames, + pickRawDownloadName, +} = require('../services/downloadFilenameService'); +const { buildContentDisposition } = require('../utils/filenameSanitizer'); const router = express.Router(); @@ -339,9 +344,16 @@ router.get('/:slug/secure-download/:photoId/:token', 'download' ); + // #493/#507: respect the original-filename toggle here too. The + // regular `/gallery/:slug/download/:photoId` route already does + // this — secure-images was missed in the original PR and ran + // even when the admin had opted into original camera filenames. + const useOriginal = await getUseOriginalFilenames(); + const downloadName = pickRawDownloadName(photo, useOriginal); + res.set({ 'Content-Type': photo.mime_type || 'image/jpeg', - 'Content-Disposition': `attachment; filename="${photo.filename}"`, + 'Content-Disposition': buildContentDisposition(downloadName), 'Content-Length': fileBuffer.length, 'X-Download-Protected': 'true' }); diff --git a/frontend/src/services/gallery.service.ts b/frontend/src/services/gallery.service.ts index 9535ab91..061e3e52 100644 --- a/frontend/src/services/gallery.service.ts +++ b/frontend/src/services/gallery.service.ts @@ -1,6 +1,7 @@ import { api } from '../config/api'; import type { GalleryInfo, GalleryData, GalleryStats, ResolvedGalleryIdentifier } from '../types'; import { normalizeRequirePassword } from '../utils/accessControl'; +import { parseContentDispositionFilename } from '../utils/contentDisposition'; export const galleryService = { // Verify share token @@ -49,35 +50,38 @@ export const galleryService = { // Download single photo async downloadPhoto(slug: string, photoId: number, filename: string): Promise { - try { - const response = await api.get(`/gallery/${slug}/download/${photoId}`, { - responseType: 'blob', - }); + // Honour the server's Content-Disposition filename so the #493 + // "use original camera filename" toggle reaches disk for single + // downloads (it already worked for zips because those skip the + // `` attribute). Falls back to the caller-provided + // sanitized filename if the header is unreadable. + const downloadFromResponse = (response: { data: Blob; headers: Record }) => { + const headerName = + response.headers['content-disposition'] || response.headers['Content-Disposition']; + const serverFilename = parseContentDispositionFilename(headerName); const url = window.URL.createObjectURL(new Blob([response.data])); const link = document.createElement('a'); link.href = url; - link.setAttribute('download', filename); + link.setAttribute('download', serverFilename || filename); document.body.appendChild(link); link.click(); link.remove(); window.URL.revokeObjectURL(url); - } catch (err) { - // Fallback: use the view endpoint if direct download fails (e.g., missing original) - try { - const response = await api.get(`/gallery/${slug}/photo/${photoId}`, { - responseType: 'blob', - }); - const url = window.URL.createObjectURL(new Blob([response.data])); - const link = document.createElement('a'); - link.href = url; - link.setAttribute('download', filename); - document.body.appendChild(link); - link.click(); - link.remove(); - window.URL.revokeObjectURL(url); - } catch (fallbackErr) { - throw fallbackErr; - } + }; + + try { + const response = await api.get(`/gallery/${slug}/download/${photoId}`, { + responseType: 'blob', + }); + downloadFromResponse(response); + } catch { + // Fallback: use the view endpoint if direct download fails (e.g., missing original). + // The view endpoint doesn't emit a download-oriented Content-Disposition, + // so we expect the caller-supplied filename to win here. + const response = await api.get(`/gallery/${slug}/photo/${photoId}`, { + responseType: 'blob', + }); + downloadFromResponse(response); } }, diff --git a/frontend/src/services/photos.service.ts b/frontend/src/services/photos.service.ts index 7fcbb340..004f92d7 100644 --- a/frontend/src/services/photos.service.ts +++ b/frontend/src/services/photos.service.ts @@ -1,4 +1,5 @@ import { api } from '../config/api'; +import { parseContentDispositionFilename } from '../utils/contentDisposition'; export interface AdminPhoto { id: number; @@ -102,11 +103,18 @@ class PhotosService { const response = await api.get(`/admin/events/${eventId}/photos/${photoId}/download`, { responseType: 'blob' }); - + + // Read the filename from the server's Content-Disposition so the + // #493 original-filename toggle reaches disk for admin downloads + // too (see contentDisposition.ts). + const headerName = + response.headers['content-disposition'] || response.headers['Content-Disposition']; + const serverFilename = parseContentDispositionFilename(headerName); + const url = window.URL.createObjectURL(new Blob([response.data])); const link = document.createElement('a'); link.href = url; - link.download = filename; + link.download = serverFilename || filename; document.body.appendChild(link); link.click(); document.body.removeChild(link); diff --git a/frontend/src/utils/contentDisposition.ts b/frontend/src/utils/contentDisposition.ts new file mode 100644 index 00000000..b6c7a19b --- /dev/null +++ b/frontend/src/utils/contentDisposition.ts @@ -0,0 +1,45 @@ +/** + * Parse the `filename` out of a `Content-Disposition` response header. + * + * Prefers the RFC 5987 form (`filename*=UTF-8''`) so unicode + * camera filenames round-trip correctly, and falls back to the plain + * `filename="..."` token. Returns null when the header is missing or + * unparseable so the caller can choose its own fallback (typically the + * client-side photo.filename). + * + * Why this exists: backend download routes emit a Content-Disposition with + * the user-facing filename (which may be the original camera name when the + * #493 toggle is on). The frontend used to override that with a hardcoded + * `` attribute, so the server's filename never reached + * disk. Reading it back from the response means the server stays the + * single source of truth. + */ +export function parseContentDispositionFilename(header: string | null | undefined): string | null { + if (!header) return null; + + // RFC 5987: filename*=UTF-8'' + // The optional language tag (e.g. UTF-8'en'foo.jpg) is rarely emitted by + // servers; we accept the empty case only since that's what we produce. + const star = /filename\*\s*=\s*UTF-8''([^;]+)/i.exec(header); + if (star && star[1]) { + try { + return decodeURIComponent(star[1].trim()); + } catch { + // Malformed percent-encoding — fall through to the plain form. + } + } + + // Plain quoted form: filename="..." + const quoted = /filename\s*=\s*"([^"]+)"/i.exec(header); + if (quoted && quoted[1]) { + return quoted[1]; + } + + // Plain unquoted form: filename=... (terminated by ; or end-of-string) + const unquoted = /filename\s*=\s*([^;]+)/i.exec(header); + if (unquoted && unquoted[1]) { + return unquoted[1].trim(); + } + + return null; +}