From 69b5186582d56c42cec520abbe5454171f9f666b Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Wed, 10 Jun 2026 18:22:34 +0200 Subject: [PATCH] fix(gallery): guest upload honours general_max_files_per_upload + i18n placeholder interpolates (#613) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Zszywany reported on v3.44.0 (Ubuntu, Postgres 15): with Settings → General → "Max Files per Upload" set to 10, an event configured with allow_user_uploads + a guest selecting 16 files in the gallery's "Upload Photos" modal succeeded silently — admin uploads to the same event correctly refused with "Upload limit reached". On top of that, the gallery modal's "fileRequirements" hint literally rendered `{{limit}}` instead of the configured number. Two separate misses for the guest path, both fixed here: 1. **Backend enforcement** — `backend/src/routes/gallery.js:1641` had `limits: { fileSize: 50MB, files: 10 }` and `.array('photos', 10)` hardcoded. The admin path at adminPhotos.js:131 has always resolved files-per-batch via `getMaxFilesPerUpload()` (cached 60s read of `general_max_files_per_upload`); guest path just never used it. Mirror the admin: `const maxFilesPerUpload = await getMaxFilesPerUpload()` and feed multer both `limits.files` AND the `.array(...)` cap. The 50MB per-file size is a separate concern from this issue and stays as-is for now. 2. **i18n interpolation missing on the guest modal** — `UserPhotoUpload.tsx:203` called `t('upload.fileRequirements')` with no arguments. The translation string at `en.json:160` is "JPEG, PNG or WebP (max 50MB per file, {{limit}} files per upload)" — `{{limit}}` is unbound, so i18next emits it literally. The admin variant `PhotoUpload.tsx:414` correctly passes `{ limit: maxFilesPerUpload }`. Also wired up the same client-side count guard the admin component uses: addFiles refuses additions past the limit (`upload.limitReached`) and warns on partial-truncate (`upload.someFilesSkipped`). Backend enforces too, but the client guard saves a 4MB+ multipart POST when the user is clearly over. To surface the setting on the guest side, `general_max_files_per_upload` joins the public-settings whitelist + projection (publicSettings.js) and the `PublicSettings` TS interface gets the new field. Default fallback (500, matching `uploadSettings.js` DEFAULT_MAX_FILES_PER_UPLOAD) in both backend projection and frontend reader so an install that's never set the value renders a sensible number rather than "undefined". --- backend/src/routes/gallery.js | 23 +++++++++++--- backend/src/routes/publicSettings.js | 21 ++++++++++++- .../components/gallery/UserPhotoUpload.tsx | 30 ++++++++++++++++++- .../src/services/publicSettings.service.ts | 4 +++ 4 files changed, 72 insertions(+), 6 deletions(-) diff --git a/backend/src/routes/gallery.js b/backend/src/routes/gallery.js index 444712fc..31bd918b 100644 --- a/backend/src/routes/gallery.js +++ b/backend/src/routes/gallery.js @@ -1627,7 +1627,7 @@ router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => { // Import multer and photo processing const multer = require('multer'); - const { getAllowedMimeTypes } = require('../services/uploadSettings'); + const { getAllowedMimeTypes, getMaxFilesPerUpload } = require('../services/uploadSettings'); const { validateFileType } = require('../utils/fileSecurityUtils'); // Resolve allowed MIME types from settings @@ -1638,11 +1638,26 @@ router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => { allowedMimeTypes = ['image/jpeg', 'image/png', 'image/webp']; } + // #613 — per-batch file count was hardcoded to 10 here, so the admin's + // Settings → General → "Max Files per Upload" value silently didn't + // apply to guest uploads (only admin uploads honoured it via + // adminPhotos.js:131). Zszywany reported uploading 16 files succeeded + // even with the limit set to 10. Mirror the admin path: resolve from + // settings (cached for 60s in the service) and feed multer both + // `limits.files` and the `.array(...)` cap. Fall back to the service's + // default if the read fails. + let maxFilesPerUpload; + try { + maxFilesPerUpload = await getMaxFilesPerUpload(); + } catch { + maxFilesPerUpload = 500; + } + const upload = multer({ dest: tempUploadDir, limits: { - fileSize: 50 * 1024 * 1024, // 50MB - files: 10 // Max 10 files at once + fileSize: 50 * 1024 * 1024, // 50MB per file (separate concern from #613) + files: maxFilesPerUpload }, fileFilter: (req, file, cb) => { if (validateFileType(file.originalname, file.mimetype, allowedMimeTypes)) { @@ -1651,7 +1666,7 @@ router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => { cb(new Error('Invalid file type')); } } - }).array('photos', 10); + }).array('photos', maxFilesPerUpload); // Handle upload upload(req, res, async (err) => { diff --git a/backend/src/routes/publicSettings.js b/backend/src/routes/publicSettings.js index 050adc9e..9137807e 100644 --- a/backend/src/routes/publicSettings.js +++ b/backend/src/routes/publicSettings.js @@ -18,7 +18,15 @@ router.get('/', async (req, res) => { 'event_default_require_password', 'event_default_feedback_enabled', 'gallery_show_filter_bar', - 'event_phone_field_enabled' + 'event_phone_field_enabled', + // #613 — guest upload UI needs to know the per-batch file + // count limit so it can render a real number in the + // "fileRequirements" hint (`{{limit}}` was showing literal) + // and short-circuit before posting too many files. The + // backend route also enforces it via getMaxFilesPerUpload, + // but a client-side guard saves a 4MB+ round-trip when the + // limit is small. + 'general_max_files_per_upload' ]); }) .select('setting_key', 'setting_value'); @@ -145,6 +153,17 @@ router.get('/', async (req, res) => { gallery_show_filter_bar: settingsObject.gallery_show_filter_bar !== false, // Upload settings (safe to expose - needed for client-side validation) allowed_file_types: settingsObject.general_allowed_file_types || 'jpg,jpeg,png,webp', + // #613 — per-batch file count limit. The guest UserPhotoUpload modal + // reads this both to render the "{{limit}} files per upload" hint + // (showed `{{limit}}` literal before this) and to refuse a batch + // before posting it. Backend route enforces the same value via + // getMaxFilesPerUpload(), so the client-side check is purely a UX + // optimisation. Default mirrors uploadSettings.js + // DEFAULT_MAX_FILES_PER_UPLOAD so the UI shows a sensible number + // even on installs that have never explicitly set the value. + general_max_files_per_upload: Number.isFinite(Number(settingsObject.general_max_files_per_upload)) + ? Number(settingsObject.general_max_files_per_upload) + : 500, // SEO meta tag flags (safe to expose - these are intended for crawlers) seo_meta_noindex: settingsObject.seo_meta_noindex === true, seo_meta_nofollow: settingsObject.seo_meta_nofollow === true, diff --git a/frontend/src/components/gallery/UserPhotoUpload.tsx b/frontend/src/components/gallery/UserPhotoUpload.tsx index 791aa624..955f3b18 100644 --- a/frontend/src/components/gallery/UserPhotoUpload.tsx +++ b/frontend/src/components/gallery/UserPhotoUpload.tsx @@ -32,6 +32,17 @@ export const UserPhotoUpload: React.FC = ({ const { data: publicSettings } = usePublicSettings(); + // #613 — guest upload UI was hardcoded to behave as if the limit was + // unlimited (no client-side guard) and the fileRequirements hint + // rendered `{{limit}}` literally because t() was called with no + // interpolation argument. publicSettings now surfaces + // general_max_files_per_upload (default 500 from publicSettings.js) + // so we can render a real number and refuse oversized batches before + // they hit the backend. Backend route enforces the same value too. + const maxFilesPerUpload = Number.isFinite(Number(publicSettings?.general_max_files_per_upload)) + ? Number(publicSettings?.general_max_files_per_upload) + : 500; + const allowedMimeTypes = useMemo( () => extensionsToMimeTypes(publicSettings?.allowed_file_types), [publicSettings?.allowed_file_types] @@ -57,6 +68,20 @@ export const UserPhotoUpload: React.FC = ({ return true; }); if (validFiles.length === 0) return; + + // #613 — per-batch file count guard. Mirrors what the admin's + // PhotoUpload component does. Backend also enforces, so this is + // purely UX (saves a multi-MB POST when the user clearly went over). + const remaining = Math.max(0, maxFilesPerUpload - files.length); + if (remaining === 0) { + toast.error(t('upload.limitReached', { limit: maxFilesPerUpload })); + return; + } + if (validFiles.length > remaining) { + toast.warning(t('upload.someFilesSkipped', { allowed: remaining, limit: maxFilesPerUpload })); + setFiles((prev) => [...prev, ...validFiles.slice(0, remaining)]); + return; + } setFiles((prev) => [...prev, ...validFiles]); }; @@ -200,7 +225,10 @@ export const UserPhotoUpload: React.FC = ({ {t('upload.clickToUpload')}

- {t('upload.fileRequirements')} + {/* #613 — pass { limit } so `{{limit}}` interpolates + with the real number from settings instead of + rendering literally. */} + {t('upload.fileRequirements', { limit: maxFilesPerUpload })}