From d0ccadbc99e510d124814701faa410b3099792ed Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 17 Jul 2026 22:50:24 +0200 Subject: [PATCH] fix(uploads): RAW derivative key collision, watermark skip, dev exiftool (codex review of #833 round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Derivative key collision: processUploadedPhotos/replacePhoto passed the client-supplied original filename as the RAW output basename, but thumbnails/ heroes/previews are global keys — two galleries uploading IMG_0001.dng would overwrite each other's derivative. Use the unique stored newFilename instead. (processPhoto already used the unique photo.filename.) - Watermark: the watermark path opens the original with sharp, which can't decode RAW, so it fell back to the original bytes and recorded the copy as watermarked. Skip RAW in generateForPhoto (like videos) so the watermark state stays honest until RAW watermarking is properly supported. - exiftool added to Dockerfile.dev so dev/native runtimes don't accept a DNG then fail it with ENOENT. --- backend/Dockerfile.dev | 5 ++++- backend/src/services/photoProcessor.js | 4 +++- backend/src/services/photoReplacementService.js | 4 +++- backend/src/services/watermarkGeneratorService.js | 10 +++++++++- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/backend/Dockerfile.dev b/backend/Dockerfile.dev index 71d62c7f..a2ef40fc 100644 --- a/backend/Dockerfile.dev +++ b/backend/Dockerfile.dev @@ -8,7 +8,10 @@ RUN apk upgrade --no-cache # Install dumb-init for proper signal handling and ffmpeg for video uploads. # Alpine's ffmpeg ships both ffmpeg + ffprobe built natively against musl; # the npm-bundled binary doesn't run reliably on Alpine. Match production. -RUN apk add --no-cache dumb-init ffmpeg +# exiftool: extract embedded JPEG previews from RAW/DNG uploads (#821) — kept in +# sync with the production Dockerfile so dev/native runtimes don't accept a DNG +# and then fail it with ENOENT. +RUN apk add --no-cache dumb-init ffmpeg exiftool # Copy package files COPY package*.json ./ diff --git a/backend/src/services/photoProcessor.js b/backend/src/services/photoProcessor.js index 986df36a..5820cb1f 100644 --- a/backend/src/services/photoProcessor.js +++ b/backend/src/services/photoProcessor.js @@ -148,7 +148,9 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ // RAW/DNG can't be fed to sharp directly (no raw loader), so extract the // embedded JPEG preview first and thumbnail/measure THAT. Pass-through // for ordinary images. The stored original stays the RAW (download). - const proc = await withProcessableImage(tempPath, file.originalname); + // Use the unique stored filename (not the client-supplied original) so + // the RAW-derived thumbnail's global key can't collide across galleries. + const proc = await withProcessableImage(tempPath, newFilename); try { thumbnailPath = await generateThumbnail(proc.path, { outputBasename: proc.outputBasename }); try { diff --git a/backend/src/services/photoReplacementService.js b/backend/src/services/photoReplacementService.js index 8799825c..1944aeae 100644 --- a/backend/src/services/photoReplacementService.js +++ b/backend/src/services/photoReplacementService.js @@ -69,7 +69,9 @@ async function replacePhoto(existingPhoto, newFileTempPath, { originalFilename, let width = null; let height = null; let thumbnailPath = null; - const proc = await withProcessableImage(newFileTempPath, originalFilename); + // Detect/name by the unique stored filename (newFilename), not the + // client-supplied original, so RAW derivative keys can't collide. + const proc = await withProcessableImage(newFileTempPath, newFilename); try { try { const metadata = await sharp(proc.path).metadata(); diff --git a/backend/src/services/watermarkGeneratorService.js b/backend/src/services/watermarkGeneratorService.js index 1424de37..746e55aa 100644 --- a/backend/src/services/watermarkGeneratorService.js +++ b/backend/src/services/watermarkGeneratorService.js @@ -11,7 +11,7 @@ const { db } = require('../database/db'); const watermarkService = require('./watermarkService'); const { resolvePhotoStorageKey, resolvePhotoFilePath } = require('./photoResolver'); -const { withLocalCopy } = require('./imageProcessor'); +const { withLocalCopy, isRawFilename } = require('./imageProcessor'); const logger = require('../utils/logger'); class WatermarkGeneratorService { @@ -52,6 +52,14 @@ class WatermarkGeneratorService { return { success: false, error: 'Videos do not support watermarks' }; } + // Skip RAW/DNG (experimental, #821). The watermark path opens the original + // with sharp, which can't decode RAW — proceeding would fall back to the + // original bytes and falsely record the copy as watermarked. Skipping keeps + // the watermark state honest until RAW watermarking is properly supported. + if (isRawFilename(photo.filename)) { + return { success: false, error: 'RAW/DNG files are not watermarked yet' }; + } + // Get watermark settings const settings = await watermarkService.getWatermarkSettings(); if (!settings || !settings.enabled) {