diff --git a/backend/__tests__/routes/galleryDownloadStorage.test.js b/backend/__tests__/routes/galleryDownloadStorage.test.js new file mode 100644 index 00000000..b1fb3f9a --- /dev/null +++ b/backend/__tests__/routes/galleryDownloadStorage.test.js @@ -0,0 +1,278 @@ +/** + * Single-photo gallery downloads must go through the storage backend (#1048). + * + * `GET /api/gallery/:slug/download/:photoId` resolved a LOCAL filesystem path + * unconditionally and handed it to res.sendFile. On an S3/R2 deployment + * managed photos never exist on local disk, so every per-photo download 404'd + * with ENOENT — while download-all and secure-images worked fine, because they + * already went through getStorage(). The gallery looks healthy until a guest + * clicks the download button on a single photo. + * + * The local branch is pinned just as hard: sendFile emits Content-Length, + * Accept-Ranges, ETag and Last-Modified and answers Range with a 206. Routing + * local installs through a bare stream.pipe(res) to share one code path would + * silently drop all of that, and a resumed download would append a second full + * body onto the partial file. + */ + +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +process.env.NODE_ENV = 'test'; +process.env.TEST_DATABASE_PATH = path.join( + fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-dl-')), 'db.sqlite', +); +process.env.JWT_SECRET = process.env.JWT_SECRET || 'download-test-secret'; +process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-dl-storage-')); + +const { Readable } = require('stream'); + +const SLUG = 'download-gallery'; +const FILENAME = 'original.jpg'; +// Deliberately not written to disk anywhere: if the route reads the +// filesystem instead of the backend, it cannot produce these bytes. +const mockObjectBody = Buffer.from('S3-ONLY-ORIGINAL-BYTES-not-on-local-disk'); +const mockBackendKind = { value: 's3' }; + +const mockStorage = { + kind: () => mockBackendKind.value, + stat: jest.fn(async () => ({ size: mockObjectBody.length, mtime: new Date('2026-08-20T10:00:00Z') })), + get: jest.fn(async () => Readable.from([mockObjectBody])), + getRange: jest.fn(async (key, start, end) => Readable.from([mockObjectBody.subarray(start, end + 1)])), + delete: jest.fn(async () => undefined), + exists: jest.fn(async () => true), +}; + +jest.mock('../../src/services/storage', () => ({ + getStorage: () => mockStorage, + initStorage: async () => mockStorage, +})); + +const request = require('supertest'); +const express = require('express'); +const cookieParser = require('cookie-parser'); +const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb'); + +describe('single-photo download through the storage backend (#1048)', () => { + let db; let cleanup; let app; let eventId; let photoId; + + beforeAll(async () => { + ({ db, cleanup } = await bootCrmDb()); + await seedMinimal(db); + + const ev = await db('events').insert({ + slug: SLUG, + event_type: 'wedding', + event_name: 'Downloads', + event_date: '2026-08-01', + host_email: 'h@example.com', + admin_email: 'a@example.com', + password_hash: 'x', + share_link: `/gallery/${SLUG}/s`, + share_token: 'download-share', + expires_at: new Date(Date.now() + 7 * 864e5).toISOString(), + is_active: 1, + is_archived: 0, + is_draft: 0, + require_password: 0, + allow_downloads: 1, + created_at: new Date().toISOString(), + }).returning('id'); + eventId = ev[0]?.id ?? ev[0]; + + const row = await db('photos').insert({ + event_id: eventId, + filename: FILENAME, + path: `${SLUG}/${FILENAME}`, + type: 'individual', + source_origin: 'managed', + mime_type: 'image/jpeg', + uploaded_at: new Date().toISOString(), + }).returning('id'); + photoId = row[0]?.id ?? row[0]; + + app = express(); + app.use(express.json()); + app.use(cookieParser()); + app.use('/api/gallery', require('../../src/routes/gallery')); + }, 120000); + + afterAll(async () => { if (cleanup) await cleanup(); }); + + beforeEach(() => { + mockBackendKind.value = 's3'; + mockStorage.get.mockClear(); + mockStorage.getRange.mockClear(); + }); + + it('streams the stored object instead of 404ing on a local path', async () => { + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .buffer(true) + .parse((response, cb) => { + const chunks = []; + response.on('data', (c) => chunks.push(c)); + response.on('end', () => cb(null, Buffer.concat(chunks))); + }); + + expect(res.status).toBe(200); + // The bytes only exist in the backend — proof it did not read the disk. + expect(res.body.equals(mockObjectBody)).toBe(true); + expect(mockStorage.get).toHaveBeenCalledWith(`events/active/${SLUG}/${FILENAME}`); + // Never written locally, so a filesystem read could not have served this. + expect(fs.existsSync(path.join(process.env.STORAGE_PATH, 'events/active', SLUG, FILENAME))).toBe(false); + }); + + it('sends Content-Length so the browser can show download progress', async () => { + const res = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + + expect(res.headers['content-length']).toBe(String(mockObjectBody.length)); + expect(res.headers['accept-ranges']).toBe('bytes'); + expect(res.headers['content-disposition']).toContain(FILENAME); + }); + + it('answers a Range request with 206 and only the requested bytes', async () => { + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=0-9') + .buffer(true) + .parse((response, cb) => { + const chunks = []; + response.on('data', (c) => chunks.push(c)); + response.on('end', () => cb(null, Buffer.concat(chunks))); + }); + + expect(res.status).toBe(206); + expect(res.headers['content-range']).toBe(`bytes 0-9/${mockObjectBody.length}`); + expect(res.headers['content-length']).toBe('10'); + expect(res.body.equals(mockObjectBody.subarray(0, 10))).toBe(true); + expect(mockStorage.getRange).toHaveBeenCalledWith(`events/active/${SLUG}/${FILENAME}`, 0, 9); + }); + + it('ignores a malformed Range rather than emitting a nonsense 206', async () => { + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=abc-def'); + + expect(res.status).toBe(200); + expect(res.headers['content-range']).toBeUndefined(); + }); + + it('404s cleanly when the object is missing from the backend', async () => { + mockStorage.stat.mockResolvedValueOnce(null); + + const res = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + + expect(res.status).toBe(404); + // The error must not inherit the image headers staged for a successful + // download, or the browser saves a .jpg containing JSON. + expect(res.headers['content-type']).toMatch(/json/); + expect(res.headers['content-disposition']).toBeUndefined(); + }); + + it('keeps res.sendFile on a local backend rather than a bare pipe', async () => { + mockBackendKind.value = 'local'; + const abs = path.join(process.env.STORAGE_PATH, 'events/active', SLUG, FILENAME); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, 'local-disk-bytes'); + + const res = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + + expect(res.status).toBe(200); + expect(mockStorage.get).not.toHaveBeenCalled(); + // sendFile's signature: conditional-request headers a raw pipe never sets. + expect(res.headers.etag).toBeDefined(); + expect(res.headers['last-modified']).toBeDefined(); + + fs.rmSync(abs, { force: true }); + }); + + it('does not serve a partial body when the If-Range validator is stale', async () => { + // The object was replaced since the client's last attempt. Answering 206 + // from the new bytes would let it splice two versions into one file. + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=0-9') + .set('If-Range', new Date('2020-01-01T00:00:00Z').toUTCString()); + + expect(res.status).toBe(200); + expect(res.headers['content-range']).toBeUndefined(); + expect(res.headers['content-length']).toBe(String(mockObjectBody.length)); + }); + + it('still serves 206 when the If-Range validator matches', async () => { + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=0-9') + .set('If-Range', new Date('2026-08-20T10:00:00Z').toUTCString()); + + expect(res.status).toBe(206); + expect(res.headers['content-range']).toBe(`bytes 0-9/${mockObjectBody.length}`); + }); + + it('errors cleanly when the object vanishes between stat and get', async () => { + // HeadObject succeeding does not mean GetObject will — a concurrent + // delete lands here. The staged image headers must not escape with it. + const gone = new Error('NoSuchKey'); + gone.name = 'NoSuchKey'; + mockStorage.get.mockRejectedValueOnce(gone); + + const res = await request(app).get(`/api/gallery/${SLUG}/download/${photoId}`); + + expect(res.status).toBe(404); + expect(res.headers['content-type']).toMatch(/json/); + expect(res.headers['content-disposition']).toBeUndefined(); + }); + + it('does not send 206 headers before the range fetch can fail', async () => { + // writeHead(206) before the await would make this ERR_HTTP_HEADERS_SENT. + mockStorage.getRange.mockRejectedValueOnce(new Error('connection reset')); + + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=0-9'); + + expect(res.status).toBe(500); + expect(res.headers['content-type']).toMatch(/json/); + expect(res.headers['content-range']).toBeUndefined(); + }); + + it('answers HEAD from stat instead of draining the object out of S3', async () => { + const before = (await db('photos').where('id', photoId).first()).download_count || 0; + const logsBefore = (await db('access_logs').where({ photo_id: photoId, action: 'download' })).length; + + const res = await request(app).head(`/api/gallery/${SLUG}/download/${photoId}`); + + expect(res.status).toBe(200); + expect(res.headers['content-length']).toBe(String(mockObjectBody.length)); + expect(res.headers['accept-ranges']).toBe('bytes'); + // The whole point: no egress for a metadata probe. + expect(mockStorage.get).not.toHaveBeenCalled(); + expect(mockStorage.getRange).not.toHaveBeenCalled(); + + // And no side effects: a probe is not a download. + const after = (await db('photos').where('id', photoId).first()).download_count || 0; + expect(after).toBe(before); + const logsAfter = (await db('access_logs').where({ photo_id: photoId, action: 'download' })).length; + expect(logsAfter).toBe(logsBefore); + }); + + it('returns a clean error when the range stream dies before its first chunk', async () => { + // Resolves, then errors — writeHead would already have committed the 206, + // leaving a connection reset as the only possible outcome. + const { Readable: R } = require('stream'); + mockStorage.getRange.mockImplementationOnce(async () => { + const dead = new R({ read() { this.destroy(new Error('socket hang up')); } }); + return dead; + }); + + const res = await request(app) + .get(`/api/gallery/${SLUG}/download/${photoId}`) + .set('Range', 'bytes=0-9'); + + expect(res.status).toBe(500); + expect(res.headers['content-type']).toMatch(/json/); + expect(res.headers['content-range']).toBeUndefined(); + }); +}); diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 3e36e0ef..dab40a64 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -35,16 +35,16 @@ const { COLOR_LABELS, SHARED_COLOR_LABEL_IDENTITY } = require('../constants/colo const secureImageService = require('../services/secureImageService'); const logger = require('../utils/logger'); const { pipeStreamToResponse } = require('../utils/streamResponse'); -const { resolvePhotoFilePath } = require('../services/photoResolver'); +const { resolvePhotoFilePath, resolvePhotoStorageKey } = require('../services/photoResolver'); const { getEventCategoriesOrdered } = require('../utils/categoryOrder'); const { getEventShareToken, resolveShareIdentifier, buildShareLinkVariants } = require('../services/shareLinkService'); const { handleAsync, errorResponse } = require('../utils/routeHelpers'); const { isGalleryHidden, guestBlockedByReveal, blockHiddenGallery } = require('../utils/revealMode'); const { toIso } = require('../utils/dateNormalize'); const { NotFoundError } = require('../utils/errors'); -const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy, resizeToBox } = require('../services/imageProcessor'); +const { ensureThumbnail, ensureHeroImage, ensurePreviewImage, withLocalCopy } = require('../services/imageProcessor'); const downloadZipService = require('../services/downloadZipService'); -const { renderPhotoForDownload } = require('../services/downloadRendition'); +const { renderPhotoForDownload, resolveWatermarkSettings } = require('../services/downloadRendition'); const downloadJobService = require('../services/downloadJobService'); // Download resolutions (#858) — the standard size a gallery hands out, plus // validation of any guest-picked override. @@ -89,6 +89,42 @@ const fs = require('fs'); // Get storage path from environment or default const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); +/** + * Parse a single-range `Range: bytes=` header against a known size. + * + * Returns null for absent, malformed, multi-range or unsatisfiable headers — + * every one of which the caller answers with a normal 200 full body, which is + * what a client that sent an unparseable range would get today anyway. + * Validating matters because an unchecked parse yields NaN bounds and a 206 + * with a nonsense Content-Range, which corrupts a resumed download rather + * than merely failing it. + */ +function parseByteRange(header, size) { + if (!header || typeof header !== 'string' || !size) return null; + const match = /^bytes=(\d*)-(\d*)$/.exec(header.trim()); + if (!match) return null; + + const [, rawStart, rawEnd] = match; + if (rawStart === '' && rawEnd === '') return null; + + let start; + let end; + if (rawStart === '') { + // Suffix form: the last N bytes. + const suffix = parseInt(rawEnd, 10); + if (!suffix) return null; + start = Math.max(0, size - suffix); + end = size - 1; + } else { + start = parseInt(rawStart, 10); + end = rawEnd === '' ? size - 1 : parseInt(rawEnd, 10); + } + + if (!Number.isFinite(start) || !Number.isFinite(end)) return null; + if (start > end || start >= size) return null; + return { start, end: Math.min(end, size - 1) }; +} + // "Gallery opened" for the admin notification bell (#746). The photo-list // endpoint fires on every gallery page load, so notifying per hit would spam // the bell — debounce to at most one notification per event per window. The @@ -1548,6 +1584,44 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, } const box = isVideo ? null : parseResolution(requested); + // A HEAD is a metadata probe, not a download. Answering it below the + // counters recorded every probe as a real download, and answering it below + // renderPhotoForDownload fetched and watermarked an image whose body Node + // then discards. Both happen before this point in a GET, so HEAD leaves + // here — with no side effects and no bytes read. + if (req.method === 'HEAD') { + const headUseOriginal = await getUseOriginalFilenames(); + const headHeaders = { + 'Content-Type': photo.mime_type || 'image/jpeg', + 'Content-Disposition': buildContentDisposition(pickRawDownloadName(photo, headUseOriginal)), + 'Accept-Ranges': 'bytes', + }; + + // Content-Length only when the bytes ship untransformed AND the size can + // be read without fetching them. A watermark or resize changes the + // length, and the only way to learn the new one is to do the work this + // branch exists to avoid — HEAD is allowed to omit it. + const headWatermark = await resolveWatermarkSettings(req.event); + if (!box && !headWatermark) { + try { + const headKey = resolvePhotoStorageKey(req.event, photo); + const headStorage = getStorage(); + if (headKey && headStorage.kind() !== 'local') { + const headStat = await headStorage.stat(headKey); + if (!headStat) return res.status(404).json({ error: 'Photo file not found' }); + headHeaders['Content-Length'] = headStat.size; + if (headStat.mtime) headHeaders['Last-Modified'] = new Date(headStat.mtime).toUTCString(); + } + } catch (headErr) { + // No length is a valid HEAD; not worth failing the probe over. + logger.debug('HEAD probe could not stat the object', { photoId, error: headErr.message }); + } + } + + res.set(headHeaders); + return res.end(); + } + // Admin preview (#868) downloads are excluded from the download count + // guest analytics — kept out of client-facing stats. if (!req.isAdminPreview) { @@ -1571,6 +1645,174 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, if (res.statusCode < 400 && !req.isAdminPreview) notifySinglePhotoDownload(req.event, req); }); + // #493: if the admin enabled "use original filenames", surface the + // pre-rename camera filename in Content-Disposition. Storage path is + // unchanged — only the user-visible download name is swapped. + const useOriginal = await getUseOriginalFilenames(); + const downloadName = pickRawDownloadName(photo, useOriginal); + const contentDisposition = buildContentDisposition(downloadName); + + // The gallery's standard applies to EVERY ordinary download, single photos + // included — otherwise a lowered standard is trivially bypassed by + // downloading photos one at a time. `box` was resolved above, before the + // counters. Videos have no resize path and always ship as-is. + // + // renderPhotoForDownload (#858) owns the resize-then-watermark ordering + // and the storage fetch, and is what the zip builders below already use. + // It returns null when the photo needs no transformation at all, which is + // the default gallery's common case and lets us ship the stored bytes + // without buffering a full-size original into memory. + const effectiveSettings = await resolveWatermarkSettings(req.event); + + let rendered; + try { + rendered = await renderPhotoForDownload(req.event, photo, box, effectiveSettings); + } catch (renderError) { + // Classify, the same way the pass-through branch below does. This can + // reject because the source object is gone, but equally because + // getToFile timed out, the tmp filesystem filled up, or sharp failed — + // and reporting an operational failure as 404 tells the guest their + // photo does not exist and tells us nothing. + const gone = renderError.code === 'ENOENT' + || renderError.name === 'NoSuchKey' + || renderError.name === 'NotFound' + || renderError.$metadata?.httpStatusCode === 404; + logger.error('Failed to render photo for download', { + slug: req.params.slug, + photoId, + eventId: req.event.id, + error: renderError.message, + }); + return gone + ? res.status(404).json({ error: 'Photo file not found' }) + : res.status(500).json({ error: 'Failed to download photo' }); + } + + if (rendered) { + res.set({ + 'Content-Type': photo.mime_type || 'image/jpeg', + 'Content-Disposition': contentDisposition, + 'Content-Length': rendered.length + }); + + return res.send(rendered); + } + + // Untransformed: ship the stored bytes. + // + // Managed photos live behind the storage abstraction and on an S3/R2 + // deployment are not on local disk at all — resolving a filesystem path + // unconditionally here is what made every single-photo download 404 with + // ENOENT in S3 mode (#1048), while download-all and secure-images worked + // because they already went through getStorage(). + // + // resolvePhotoStorageKey returns null for external/reference photos: those + // live on a local mount and keep the sendFile path. + let storageKey = null; + try { + storageKey = resolvePhotoStorageKey(req.event, photo); + } catch (resolveError) { + logger.error('Failed to resolve photo storage key for download', { + slug: req.params.slug, + photoId, + eventId: req.event.id, + error: resolveError.message, + }); + return res.status(404).json({ error: 'Photo file not found' }); + } + + const storage = getStorage(); + if (storageKey && storage.kind() !== 'local') { + // Deliberately NOT the local path: res.sendFile emits Content-Length, + // Accept-Ranges, ETag and Last-Modified and answers Range requests with + // a 206, and a bare stream.pipe(res) has none of that. On local disk + // sendFile stays the better implementation, so it stays the branch. + // + // On S3 we reproduce the parts that matter for a download: the length + // (browsers need it for the progress indicator, which matters most on + // exactly the large files this route serves) and Range, so an + // interrupted download resumes instead of appending a second full body + // onto the partial file. Conditional requests are not reproduced — + // there is no ETag here, so a client revalidating gets the whole body, + // same as it does today. + const stat = await storage.stat(storageKey); + if (!stat) { + logger.error('Photo not found in storage backend for download', { + slug: req.params.slug, + photoId, + eventId: req.event.id, + storageKey, + }); + return res.status(404).json({ error: 'Photo file not found' }); + } + + const lastModified = stat.mtime ? new Date(stat.mtime).toUTCString() : null; + const headers = { + 'Content-Type': photo.mime_type || 'image/jpeg', + 'Content-Disposition': contentDisposition, + 'Accept-Ranges': 'bytes', + }; + if (lastModified) headers['Last-Modified'] = lastModified; + + // If-Range: a client resuming an interrupted download sends back the + // validator it was given last time. If the object has been replaced + // since — the watcher re-importing a swapped file, an admin re-upload — + // answering 206 from the NEW bytes lets the client splice two different + // versions into one corrupt file. A validator that doesn't match means + // a full 200, which is the whole point of the header. + const ifRange = req.headers['if-range']; + const staleValidator = !!ifRange && (!lastModified || ifRange.trim() !== lastModified); + const range = staleValidator ? null : parseByteRange(req.headers.range, stat.size); + + // Open the stream BEFORE any header is staged or sent. stat() succeeding + // does not mean get() will: a concurrent delete or replace, or a + // transient backend error, lands here. Once writeHead(206) has gone out + // the outer catch can do nothing but throw ERR_HTTP_HEADERS_SENT, and in + // the non-range case it would send its 500 JSON underneath the staged + // image/jpeg attachment headers — a .jpg file full of JSON. + let stream; + try { + stream = range + ? await storage.getRange(storageKey, range.start, range.end) + : await storage.get(storageKey); + } catch (fetchError) { + const gone = fetchError.code === 'ENOENT' + || fetchError.name === 'NoSuchKey' + || fetchError.name === 'NotFound' + || fetchError.$metadata?.httpStatusCode === 404; + logger.error('Failed to open photo stream for download', { + slug: req.params.slug, + photoId, + eventId: req.event.id, + storageKey, + error: fetchError.message, + }); + return gone + ? res.status(404).json({ error: 'Photo file not found' }) + : res.status(500).json({ error: 'Failed to download photo' }); + } + + if (range) { + // status()+set() rather than writeHead(): writeHead commits the + // response immediately, so a stream that resolves and THEN errors + // before its first chunk would leave pipeStreamToResponse able only to + // destroy the connection. Staged headers are flushed by the first body + // write, which means an error at byte zero can still clear them and + // return a clean, retryable status instead of a transport reset. + res.status(206).set({ + ...headers, + 'Content-Range': `bytes ${range.start}-${range.end}/${stat.size}`, + 'Content-Length': (range.end - range.start) + 1, + }); + } else { + res.set({ ...headers, 'Content-Length': stat.size }); + } + pipeStreamToResponse(stream, res, { + context: range ? `download range for photo ${photo.id}` : `download for photo ${photo.id}`, + }); + return; + } + let filePath; try { filePath = resolvePhotoFilePath(req.event, photo); @@ -1583,75 +1825,25 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken, }); return res.status(404).json({ error: 'Photo file not found' }); } - - // Get watermark settings - apply if global setting OR event-level setting is enabled - const watermarkSettings = await watermarkService.getWatermarkSettings(); - const eventWatermarkEnabled = req.event.watermark_downloads === true || req.event.watermark_downloads === 1; - const shouldApplyWatermark = (watermarkSettings && watermarkSettings.enabled) || eventWatermarkEnabled; - // #493: if the admin enabled "use original filenames", surface the - // pre-rename camera filename in Content-Disposition. Storage path is - // unchanged — only the user-visible download name is swapped. - const useOriginal = await getUseOriginalFilenames(); - const downloadName = pickRawDownloadName(photo, useOriginal); - const contentDisposition = buildContentDisposition(downloadName); - - // The gallery's standard applies to EVERY ordinary download, single photos - // included — otherwise a lowered standard is trivially bypassed by - // downloading photos one at a time. `box` was resolved above, before the - // counters. Videos have no resize path and always ship as-is. - if (shouldApplyWatermark || box) { - // Resize BEFORE watermarking: applyWatermark sizes the mark relative to - // its input's width, so watermarking the original and then shrinking - // would resample the mark and waste work on discarded pixels. - // - // With no resize (the default 'original' standard) hand applyWatermark - // the PATH, not a buffer: buffer inputs deliberately skip its cache, so - // buffering here would re-run sharp over the full-size original on every - // download and regress the pre-#858 watermark performance. - const effectiveSettings = shouldApplyWatermark ? { - ...watermarkSettings, - enabled: true, - text: req.event.watermark_text || watermarkSettings?.text || 'Protected' - } : null; - - let buffer; - if (!box) { - buffer = await watermarkService.applyWatermark(filePath, effectiveSettings); - } else { - buffer = await resizeToBox(await fs.promises.readFile(filePath), box); - if (shouldApplyWatermark) { - buffer = await watermarkService.applyWatermark(buffer, effectiveSettings); - } + // res.download() builds Content-Disposition itself but doesn't emit the + // RFC 5987 filename* parameter, so unicode camera filenames would lose + // their bytes on download. Set the header explicitly and stream the + // file with res.sendFile-equivalent semantics. + res.set({ + 'Content-Type': photo.mime_type || 'image/jpeg', + 'Content-Disposition': contentDisposition, + }); + res.sendFile(filePath, (downloadError) => { + if (downloadError) { + logger.error('Error streaming gallery download', { + slug: req.params.slug, + photoId, + eventId: req.event.id, + error: downloadError.message, + }); } - - res.set({ - 'Content-Type': photo.mime_type || 'image/jpeg', - 'Content-Disposition': contentDisposition, - 'Content-Length': buffer.length - }); - - res.send(buffer); - } else { - // res.download() builds Content-Disposition itself but doesn't emit the - // RFC 5987 filename* parameter, so unicode camera filenames would lose - // their bytes on download. Set the header explicitly and stream the - // file with res.sendFile-equivalent semantics. - res.set({ - 'Content-Type': photo.mime_type || 'image/jpeg', - 'Content-Disposition': contentDisposition, - }); - res.sendFile(filePath, (downloadError) => { - if (downloadError) { - logger.error('Error streaming gallery download', { - slug: req.params.slug, - photoId, - eventId: req.event.id, - error: downloadError.message, - }); - } - }); - } + }); } catch (error) { errorResponse(res, error, 500, 'Failed to download photo'); } diff --git a/backend/src/utils/streamResponse.js b/backend/src/utils/streamResponse.js index 2db6b528..0188ea4d 100644 --- a/backend/src/utils/streamResponse.js +++ b/backend/src/utils/streamResponse.js @@ -55,6 +55,11 @@ function pipeStreamToResponse(stream, res, options = {}) { res.removeHeader('ETag'); res.removeHeader('Content-Type'); res.removeHeader('Content-Disposition'); + // Range headers describe the body that is no longer coming. Left behind, + // a 500 goes out still advertising `Content-Range: bytes 0-9/40`, which + // tells a resuming client the error response IS the partial content. + res.removeHeader('Content-Range'); + res.removeHeader('Accept-Ranges'); res.setHeader('Cache-Control', 'no-store'); if (gone) {