Compare commits
15 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 7fe80220f1 | |||
| ccdcdd6116 | |||
| ccc725f36e | |||
| fed99ac03d | |||
| a01731d986 | |||
| 1d9f0b6c64 | |||
| 261e243070 | |||
| 5470fbe406 | |||
| 7102687ee8 | |||
| 5b69e3ec4c | |||
| eebca9900b | |||
| c05faa50d9 | |||
| 15c844db06 | |||
| c685a3e931 | |||
| 74ff236b51 |
+19
@@ -135,3 +135,22 @@ new-layouts/
|
||||
# writes event photos into backend/storage/, and the narrower
|
||||
# business-docs-only rule let `git add -A` sweep them into a commit.
|
||||
backend/storage/
|
||||
|
||||
# Python bytecode. The ML sidecar lives on main only, so this branch never
|
||||
# needed the rule — which is how a `git add -A` from a shared working tree
|
||||
# committed 16 .pyc files here in #1247.
|
||||
__pycache__/
|
||||
*.pyc
|
||||
|
||||
# Issue / PR screenshots belong on a `screenshots/*` branch, never on main or
|
||||
# stable — that is what those branches exist for. Two landed at the repo root
|
||||
# on main in #1241 and shipped as part of the source tree; nothing stopped it.
|
||||
#
|
||||
# Anchored with a leading slash so docs/ keeps its own images.
|
||||
/issue-*.png
|
||||
/issue-*.jpg
|
||||
/screenshot-*.png
|
||||
/screenshot-*.jpg
|
||||
/*-screenshot.png
|
||||
/*-screenshot.jpg
|
||||
|
||||
|
||||
@@ -1 +1 @@
|
||||
{".":"3.46.5"}
|
||||
{".":"3.46.8"}
|
||||
|
||||
@@ -5,6 +5,34 @@ All notable changes to PicPeak will be documented in this file.
|
||||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
## [3.46.8](https://github.com/PicPeak/picpeak/compare/v3.46.7...v3.46.8) (2026-09-01)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **archives:** take the restored category from the manifest ([#1240](https://github.com/PicPeak/picpeak/issues/1240)) (stable) ([#1243](https://github.com/PicPeak/picpeak/issues/1243)) ([261e243](https://github.com/PicPeak/picpeak/commit/261e243070b6082ccd8b972de68d2fee15329235))
|
||||
* **archives:** write a real timestamp on restored photos ([#1257](https://github.com/PicPeak/picpeak/issues/1257)) ([fed99ac](https://github.com/PicPeak/picpeak/commit/fed99ac03dfde03cf4c55fcb4b1419fea7564156))
|
||||
* **auth:** treat zxcvbn suggestions as advice, not blocking errors ([#1247](https://github.com/PicPeak/picpeak/issues/1247)) ([5b69e3e](https://github.com/PicPeak/picpeak/commit/5b69e3ec4c898204c9fcde0f1d49f24edc688891))
|
||||
* **auth:** treat zxcvbn suggestions as advice, not blocking errors ([#1247](https://github.com/PicPeak/picpeak/issues/1247)) ([eebca99](https://github.com/PicPeak/picpeak/commit/eebca9900b6f00b222eec16e27fa6be4fe2ce9fa))
|
||||
* **events:** apply the gallery password policy to publish ([#1255](https://github.com/PicPeak/picpeak/issues/1255)) ([1d9f0b6](https://github.com/PicPeak/picpeak/commit/1d9f0b6c6491cface22f12651131fd5dfba330f0))
|
||||
* **events:** delete stored objects when cascading an event delete ([#1245](https://github.com/PicPeak/picpeak/issues/1245)) ([7102687](https://github.com/PicPeak/picpeak/commit/7102687ee804140bfaca420d2eb7ec0078e50f25))
|
||||
* **gallery:** route single-photo downloads through the storage backend ([#1246](https://github.com/PicPeak/picpeak/issues/1246)) ([5470fbe](https://github.com/PicPeak/picpeak/commit/5470fbe4063c3d6c0aeeb50fdb1ce6af74df1b53))
|
||||
* **upload:** let Android guests reach the camera without breaking video ([#1248](https://github.com/PicPeak/picpeak/issues/1248)) ([ccc725f](https://github.com/PicPeak/picpeak/commit/ccc725f36edcf20643ab9c3a7aff16b5b93674c1))
|
||||
|
||||
## [3.46.7](https://github.com/PicPeak/picpeak/compare/v3.46.6...v3.46.7) (2026-08-28)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **admin:** the "Uncategorized" photo filter returns every photo ([#1211](https://github.com/PicPeak/picpeak/issues/1211)) ([#1215](https://github.com/PicPeak/picpeak/issues/1215)) ([15c844d](https://github.com/PicPeak/picpeak/commit/15c844db067de7bc04a88ddd407f3ed8f5df0fe2))
|
||||
|
||||
## [3.46.6](https://github.com/PicPeak/picpeak/compare/v3.46.5...v3.46.6) (2026-08-27)
|
||||
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
* **images:** fence the capture-date backfill on the file it read ([#1201](https://github.com/PicPeak/picpeak/issues/1201)) ([#1205](https://github.com/PicPeak/picpeak/issues/1205)) ([74ff236](https://github.com/PicPeak/picpeak/commit/74ff236b516b6df00e83d3c314fde552d18605ad))
|
||||
|
||||
## [3.46.5](https://github.com/PicPeak/picpeak/compare/v3.46.4...v3.46.5) (2026-08-26)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,498 @@
|
||||
/**
|
||||
* Restoring an archive must put the photos back into their categories.
|
||||
*
|
||||
* The archive writer already persists `category_name` per photo in
|
||||
* `photos_manifest.json` — that is why the manifest exists, and the comment
|
||||
* above it says so: "(and category linkage) can't be derived from the
|
||||
* extracted files alone". The restore route then read only
|
||||
* `original_filename` from it and kept deriving the category from the ZIP's
|
||||
* first path segment.
|
||||
*
|
||||
* Archives store photos exactly as they sit on disk, so an event whose photos
|
||||
* live in the gallery root produces a FLAT zip. `path.dirname()` is '.' for
|
||||
* every entry, no category is resolved, and every restored photo lands with
|
||||
* `category_id = null` — silently, with a 200 response.
|
||||
*
|
||||
* These pin the manifest as the source of truth, with the directory as the
|
||||
* fallback that keeps foldered and legacy archives working.
|
||||
*/
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
describe('archive restore restores categories (flat archives included)', () => {
|
||||
let tmpDir; let db; let cleanup; let app; let storagePath;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'picpeak-restore-cat-'));
|
||||
storagePath = path.join(tmpDir, 'storage');
|
||||
process.env.NODE_ENV = 'test';
|
||||
process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'data', 'test.db');
|
||||
process.env.STORAGE_PATH = storagePath;
|
||||
await fs.promises.mkdir(path.dirname(process.env.TEST_DATABASE_PATH), { recursive: true });
|
||||
await fs.promises.mkdir(path.join(storagePath, 'archives'), { recursive: true });
|
||||
|
||||
jest.resetModules();
|
||||
jest.doMock('../../src/middleware/auth', () => ({
|
||||
adminAuth: (req, _res, next) => { req.admin = { id: 1, username: 'tester' }; next(); },
|
||||
}));
|
||||
jest.doMock('../../src/middleware/permissions', () => ({
|
||||
requirePermission: () => (_req, _res, next) => next(),
|
||||
}));
|
||||
jest.doMock('../../src/middleware/ownership', () => ({
|
||||
requireEventOwnership: (_req, _res, next) => next(),
|
||||
}));
|
||||
|
||||
({ db, cleanup } = await require('./helpers/crmDb').bootCrmDb());
|
||||
// bootCrmDb points STORAGE_PATH at its own tmp dir; follow it rather than
|
||||
// fighting it, so the archives the tests write are where the route looks.
|
||||
storagePath = process.env.STORAGE_PATH;
|
||||
await fs.promises.mkdir(path.join(storagePath, 'archives'), { recursive: true });
|
||||
|
||||
app = express();
|
||||
app.use(express.json());
|
||||
app.use('/admin/archives', require('../../src/routes/adminArchives'));
|
||||
}, 180000);
|
||||
|
||||
afterAll(async () => {
|
||||
if (cleanup) await cleanup();
|
||||
await fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {});
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await db('photos').del();
|
||||
await db('photo_categories').del();
|
||||
await db('events').del();
|
||||
});
|
||||
|
||||
/** A one-pixel JPEG is enough; the route only stats the extracted file. */
|
||||
const PIXEL = Buffer.from(
|
||||
'/9j/4AAQSkZJRgABAQEAYABgAAD/2wBDAAgGBgcGBQgHBwcJCQgKDBQNDAsLDBkSEw8UHRofHh0a'
|
||||
+ 'HBwgJC4nICIsIxwcKDcpLDAxNDQ0Hyc5PTgyPC4zNDL/wAALCAABAAEBAREA/8QAFAABAAAAAAAA'
|
||||
+ 'AAAAAAAAAAAACf/EABQQAQAAAAAAAAAAAAAAAAAAAAD/2gAIAQEAAD8AKp//2Q==',
|
||||
'base64',
|
||||
);
|
||||
|
||||
async function writeArchive(name, entries) {
|
||||
// Required lazily: the suite calls jest.resetModules() in beforeAll, and
|
||||
// archiver's readable-stream copy does not survive being split across the
|
||||
// two module registries.
|
||||
const archiver = require('archiver');
|
||||
const archivePath = path.join(storagePath, 'archives', name);
|
||||
await new Promise((resolve, reject) => {
|
||||
const output = fs.createWriteStream(archivePath);
|
||||
const zip = archiver('zip', { zlib: { level: 0 } });
|
||||
output.on('close', resolve);
|
||||
zip.on('error', reject);
|
||||
zip.pipe(output);
|
||||
for (const [entryName, buffer] of Object.entries(entries)) {
|
||||
zip.append(buffer, { name: entryName });
|
||||
}
|
||||
zip.finalize();
|
||||
});
|
||||
return path.join('archives', name);
|
||||
}
|
||||
|
||||
async function seedArchivedEvent(archiveRelPath, slug) {
|
||||
const [row] = await db('events').insert({
|
||||
slug,
|
||||
event_type: 'wedding',
|
||||
event_name: slug,
|
||||
event_date: '2026-06-27',
|
||||
host_email: 'h@example.com',
|
||||
admin_email: 'a@example.com',
|
||||
password_hash: 'x',
|
||||
share_link: `${slug}-share`,
|
||||
expires_at: new Date().toISOString(),
|
||||
is_archived: 1, // sqlite stores booleans as 0/1, see utils/dbCompat
|
||||
archive_path: archiveRelPath,
|
||||
}).returning('id');
|
||||
return typeof row === 'object' ? row.id : row;
|
||||
}
|
||||
|
||||
const categoryOf = async (filename) => {
|
||||
const photo = await db('photos').where('filename', filename).first();
|
||||
if (!photo || !photo.category_id) return null;
|
||||
const category = await db('photo_categories').where('id', photo.category_id).first();
|
||||
return category ? category.name : null;
|
||||
};
|
||||
|
||||
it('takes the category from the manifest when the archive is flat', async () => {
|
||||
// Exactly the shape a gallery-root event archives to: no directories.
|
||||
const manifest = JSON.stringify([
|
||||
{ filename: 'a.jpg', original_filename: 'DSC_0001.jpg', category_name: 'Polterabend' },
|
||||
{ filename: 'b.jpg', original_filename: 'DSC_0002.jpg', category_name: 'Ceremony' },
|
||||
]);
|
||||
const archiveRelPath = await writeArchive('flat.zip', {
|
||||
'a.jpg': PIXEL,
|
||||
'b.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(manifest, 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'flat-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
// The whole bug: both of these used to be null.
|
||||
expect(await categoryOf('a.jpg')).toBe('Polterabend');
|
||||
expect(await categoryOf('b.jpg')).toBe('Ceremony');
|
||||
});
|
||||
|
||||
it('stores a real timestamp on restored photos, not "[object Object]"', async () => {
|
||||
// The jest+sqlite landmine: a Date handed to knex inside jest stores as
|
||||
// the literal string "[object Object]". Production writes ms-numbers and
|
||||
// is unaffected, so this only ever corrupts what tests read back — which
|
||||
// is how it survives unnoticed.
|
||||
const archiveRelPath = await writeArchive('timestamp.zip', {
|
||||
'individual/STAMPED.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'STAMPED.jpg', original_filename: 'STAMPED.jpg', category_name: 'Ceremony' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'timestamp-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const photo = await db('photos').where({ event_id: eventId, filename: 'STAMPED.jpg' }).first();
|
||||
expect(String(photo.uploaded_at)).not.toBe('[object Object]');
|
||||
expect(Number.isNaN(new Date(photo.uploaded_at).getTime())).toBe(false);
|
||||
});
|
||||
|
||||
it('reuses an existing category row instead of creating a duplicate', async () => {
|
||||
const archiveRelPath = await writeArchive('reuse.zip', {
|
||||
'c.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'c.jpg', original_filename: 'DSC_0003.jpg', category_name: 'Party' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'reuse-event');
|
||||
await db('photo_categories').insert({
|
||||
event_id: eventId, name: 'Party', slug: 'party', created_at: new Date(),
|
||||
});
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('c.jpg')).toBe('Party');
|
||||
const rows = await db('photo_categories').where({ event_id: eventId, name: 'Party' });
|
||||
expect(rows).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('still falls back to the directory for legacy archives with no manifest', async () => {
|
||||
// No manifest at all — the shape every archive had before the manifest
|
||||
// landed. The directory is the only signal left, and it must keep working.
|
||||
//
|
||||
// `individual/` is what a REAL archive contains: entry names are the
|
||||
// storage key minus `events/active/{slug}`, and that layout is
|
||||
// `individual/` / `collages/`. Categories have never been directories, so
|
||||
// the fallback invents a category with that name — not useful, but better
|
||||
// than losing every category, and this pins what actually happens rather
|
||||
// than a category-shaped folder no archive produces.
|
||||
const archiveRelPath = await writeArchive('foldered.zip', {
|
||||
'individual/d.jpg': PIXEL,
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'foldered-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('d.jpg')).toBe('individual');
|
||||
});
|
||||
|
||||
it('reuses a GLOBAL category instead of cloning it into the event', async () => {
|
||||
// Seeded categories (Ceremony, Reception, ...) have event_id NULL. An
|
||||
// event-only lookup misses them, so the restore used to create a second
|
||||
// "Ceremony" — and because is_global defaults to TRUE, that duplicate then
|
||||
// appeared in every other event's category list.
|
||||
const [g] = await db('photo_categories').insert({
|
||||
event_id: null, name: 'Ceremony', slug: 'ceremony', is_global: true, created_at: new Date(),
|
||||
}).returning('id');
|
||||
const globalId = typeof g === 'object' ? g.id : g;
|
||||
|
||||
const archiveRelPath = await writeArchive('global.zip', {
|
||||
'individual/gl.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'gl.jpg', original_filename: 'DSC_1.jpg', category_name: 'Ceremony' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'global-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const photo = await db('photos').where('filename', 'gl.jpg').first();
|
||||
expect(photo.category_id).toBe(globalId);
|
||||
// No clone, global or otherwise.
|
||||
const all = await db('photo_categories').where('name', 'Ceremony');
|
||||
expect(all).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not create a GLOBAL category when it has to invent one', async () => {
|
||||
// is_global defaults to true on this column, so an unqualified insert would
|
||||
// leak a restore's category name into every gallery on the instance.
|
||||
const archiveRelPath = await writeArchive('newcat.zip', {
|
||||
'individual/nc.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'nc.jpg', original_filename: 'DSC_2.jpg', category_name: 'Polterabend' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'newcat-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const created = await db('photo_categories').where('name', 'Polterabend').first();
|
||||
expect(created.event_id).toBe(eventId);
|
||||
expect(created.is_global === false || created.is_global === 0).toBe(true);
|
||||
});
|
||||
|
||||
it('matches the manifest when the ZIP was written with original filenames', async () => {
|
||||
// With general_use_original_filenames_for_downloads on at archive time,
|
||||
// archiveService names entries after the ORIGINAL filename while the
|
||||
// manifest stays keyed by photos.filename. Looking up the extracted
|
||||
// basename missed every entry, so categories were lost on exactly those
|
||||
// archives.
|
||||
const archiveRelPath = await writeArchive('original-names.zip', {
|
||||
'individual/DSC_4242.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'stored_9f8e7d.jpg', original_filename: 'DSC_4242.jpg', category_name: 'Drohne' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'original-names-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('DSC_4242.jpg')).toBe('Drohne');
|
||||
});
|
||||
|
||||
it('prefers the event-scoped category when a global shares its name', async () => {
|
||||
// The category API permits both. A single OR-lookup with .first() returned
|
||||
// whichever the engine chose, so a photo could be reassigned to the global
|
||||
// row and lose event-local settings such as allow_downloads.
|
||||
const archiveRelPath = await writeArchive('collide.zip', {
|
||||
'individual/co.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'co.jpg', original_filename: 'DSC_3.jpg', category_name: 'Reception' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'collide-event');
|
||||
|
||||
await db('photo_categories').insert({
|
||||
event_id: null, name: 'Reception', slug: 'reception-global', is_global: true, created_at: new Date(),
|
||||
});
|
||||
const [own] = await db('photo_categories').insert({
|
||||
event_id: eventId, name: 'Reception', slug: 'reception-own', is_global: false, created_at: new Date(),
|
||||
}).returning('id');
|
||||
const ownId = typeof own === 'object' ? own.id : own;
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const photo = await db('photos').where('filename', 'co.jpg').first();
|
||||
expect(photo.category_id).toBe(ownId);
|
||||
});
|
||||
|
||||
it('matches a sanitized original filename, as the ZIP would have written it', async () => {
|
||||
// archiveService runs original names through sanitizeForZipEntry() before
|
||||
// writing the entry, so the emitted name differs from the manifest column.
|
||||
const archiveRelPath = await writeArchive('sanitized.zip', {
|
||||
'individual/od_dr_DSC_5.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'stored_abc.jpg', original_filename: 'od/dr/DSC_5.jpg', category_name: 'Strand' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'sanitized-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('od_dr_DSC_5.jpg')).toBe('Strand');
|
||||
});
|
||||
|
||||
it('ignores a legacy event-owned row when falling back to globals', async () => {
|
||||
// The bug fixed here left rows behind on upgraded instances: event-owned
|
||||
// AND is_global true, because the column defaults true. Matching on the
|
||||
// flag alone would let one event's leftover be adopted by another event's
|
||||
// restore, tying photos to a category that vanishes with someone else's
|
||||
// gallery.
|
||||
const otherEventId = await seedArchivedEvent('archives/none.zip', 'legacy-owner-event');
|
||||
await db('photo_categories').insert({
|
||||
event_id: otherEventId, name: 'Sunset', slug: 'sunset-legacy',
|
||||
is_global: true, created_at: new Date(),
|
||||
});
|
||||
|
||||
const archiveRelPath = await writeArchive('legacy-global.zip', {
|
||||
'individual/lg.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'lg.jpg', original_filename: 'DSC_6.jpg', category_name: 'Sunset' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'legacy-global-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const photo = await db('photos').where('filename', 'lg.jpg').first();
|
||||
const cat = await db('photo_categories').where('id', photo.category_id).first();
|
||||
// Its own row, not the other event's leftover.
|
||||
expect(cat.event_id).toBe(eventId);
|
||||
});
|
||||
|
||||
it('drops an ambiguous original-name alias rather than guessing', async () => {
|
||||
// Two photos in different ZIP folders can share an original basename;
|
||||
// archiveService treats the paths as distinct and suffixes neither. Both
|
||||
// would collapse onto one alias, and whichever won would hand the other
|
||||
// photo someone else's category.
|
||||
const archiveRelPath = await writeArchive('ambiguous.zip', {
|
||||
'individual/SHARED.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'a_stored.jpg', original_filename: 'SHARED.jpg', category_name: 'Alpha' },
|
||||
{ filename: 'b_stored.jpg', original_filename: 'SHARED.jpg', category_name: 'Beta' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'ambiguous-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
// Falls back to the directory rather than picking Alpha or Beta at random.
|
||||
expect(await categoryOf('SHARED.jpg')).toBe('individual');
|
||||
for (const name of ['Alpha', 'Beta']) {
|
||||
expect(await db('photo_categories').where({ event_id: eventId, name }).first()).toBeFalsy();
|
||||
}
|
||||
});
|
||||
|
||||
it('honours a manifest that says UNCATEGORIZED, instead of inventing one from the directory', async () => {
|
||||
// The case the manifest-first change was for. A real archive puts every
|
||||
// photo under `individual/`, so a photo the manifest records as having no
|
||||
// category used to come back filed under a category called "individual" —
|
||||
// the manifest being authoritative for "category X" but not for "none".
|
||||
const manifest = JSON.stringify([
|
||||
{ filename: 'u.jpg', original_filename: 'DSC_7000.jpg', category_name: null },
|
||||
]);
|
||||
const archiveRelPath = await writeArchive('uncategorized.zip', {
|
||||
'individual/u.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(manifest, 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'uncategorized-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('u.jpg')).toBeNull();
|
||||
// And no junk category row was created as a side effect.
|
||||
const rows = await db('photo_categories').where({ event_id: eventId });
|
||||
expect(rows).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('drops a canonical filename that two photos claim, rather than guessing', async () => {
|
||||
// photos.filename is not unique within an event: s3AutoImporter takes
|
||||
// path.basename(entry.key) and dedupes by path, so two imported files in
|
||||
// different subfolders both land as IMG_1234.jpg. Both ZIP entries reduce
|
||||
// to the same basename at restore, so keeping the last row seen would give
|
||||
// one photo the other's category.
|
||||
const archiveRelPath = await writeArchive('dup-canonical.zip', {
|
||||
'individual/IMG_1234.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'IMG_1234.jpg', original_filename: 'a.jpg', category_name: 'Alpha' },
|
||||
{ filename: 'IMG_1234.jpg', original_filename: 'b.jpg', category_name: 'Beta' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'dup-canonical-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await categoryOf('IMG_1234.jpg')).toBe('individual');
|
||||
for (const name of ['Alpha', 'Beta']) {
|
||||
expect(await db('photo_categories').where({ event_id: eventId, name }).first()).toBeFalsy();
|
||||
}
|
||||
});
|
||||
|
||||
it("drops a name that one row owns canonically and another claims as an alias", async () => {
|
||||
// Undecidable: with original-filename archiving ON the ZIP entry under
|
||||
// this name is the ALIAS owner's file, with it OFF it is the canonical
|
||||
// owner's, and the manifest does not record which mode was used. The
|
||||
// point of the two-pass split is that this now resolves the same way
|
||||
// every run — the archive query has no ORDER BY, so it used to be a coin
|
||||
// flip between dropping the name and overwriting it.
|
||||
const archiveRelPath = await writeArchive('alias-vs-canonical.zip', {
|
||||
'individual/CANON.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'CANON.jpg', original_filename: 'unrelated.jpg', category_name: 'Canonical' },
|
||||
{ filename: 'other_stored.jpg', original_filename: 'CANON.jpg', category_name: 'Aliased' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'alias-vs-canonical-event');
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
// Falls back to the directory rather than guessing either row.
|
||||
expect(await categoryOf('CANON.jpg')).toBe('individual');
|
||||
for (const name of ['Canonical', 'Aliased']) {
|
||||
expect(await db('photo_categories').where({ event_id: eventId, name }).first()).toBeFalsy();
|
||||
}
|
||||
});
|
||||
|
||||
it('picks the lowest id and warns when two categories share a name', async () => {
|
||||
// Allowed: two event-scoped categories with the same display name and
|
||||
// different slugs. .first() used to pick either, so a re-run could move
|
||||
// photos between them and inherit the wrong allow_downloads.
|
||||
const archiveRelPath = await writeArchive('dupe-category.zip', {
|
||||
'individual/DUPE.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'DUPE.jpg', original_filename: 'DUPE.jpg', category_name: 'Ceremony' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'dupe-category-event');
|
||||
|
||||
const [first] = await db('photo_categories').insert({
|
||||
name: 'Ceremony', slug: 'ceremony-a', is_global: 0, event_id: eventId,
|
||||
}).returning('id');
|
||||
await db('photo_categories').insert({
|
||||
name: 'Ceremony', slug: 'ceremony-b', is_global: 0, event_id: eventId,
|
||||
});
|
||||
const firstId = typeof first === 'object' ? first.id : first;
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
// Stable, not arbitrary: the same run twice lands on the same row.
|
||||
const photo = await db('photos').where({ event_id: eventId, filename: 'DUPE.jpg' }).first();
|
||||
expect(photo.category_id).toBe(firstId);
|
||||
// And no third "Ceremony" was invented.
|
||||
expect((await db('photo_categories').where({ event_id: eventId, name: 'Ceremony' })).length)
|
||||
.toBe(2);
|
||||
});
|
||||
|
||||
it('does not invent a category for a photo row that already exists', async () => {
|
||||
// archiveEvent retains photo rows, so a restore can skip every insert.
|
||||
// Resolving categories before that check created one from the stale
|
||||
// manifest name that nothing then used — renaming a category while its
|
||||
// event was archived left the old name behind as an empty duplicate.
|
||||
const archiveRelPath = await writeArchive('existing-rows.zip', {
|
||||
'individual/KEPT.jpg': PIXEL,
|
||||
'photos_manifest.json': Buffer.from(JSON.stringify([
|
||||
{ filename: 'KEPT.jpg', original_filename: 'KEPT.jpg', category_name: 'OldName' },
|
||||
]), 'utf8'),
|
||||
});
|
||||
const eventId = await seedArchivedEvent(archiveRelPath, 'existing-rows-event');
|
||||
await db('photos').insert({
|
||||
event_id: eventId, filename: 'KEPT.jpg', path: 'whatever/KEPT.jpg', type: 'jpg',
|
||||
uploaded_at: new Date().toISOString(),
|
||||
});
|
||||
|
||||
const res = await request(app).post(`/admin/archives/${eventId}/restore`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await db('photo_categories').where({ event_id: eventId, name: 'OldName' }).first())
|
||||
.toBeFalsy();
|
||||
});
|
||||
|
||||
});
|
||||
@@ -198,5 +198,30 @@ describe('capture date backfill (#1172)', () => {
|
||||
|
||||
expect(new Date((await db('photos').where({ id: photoId }).first()).captured_at).toISOString()).toBe(claimed);
|
||||
expect(done.body.lastResult.success).toBe(0);
|
||||
// Read but not written, so it is accounted for rather than dropped.
|
||||
expect(done.body.lastResult.skipped).toBe(1);
|
||||
});
|
||||
|
||||
it('does not date a row whose file was replaced while it was reading (#1201)', async () => {
|
||||
// replacePhoto swaps a NEW file under an existing row and rewrites
|
||||
// path/filename (reachable from replace_by_name). The replacement carries
|
||||
// no date of its own, so captured_at is still NULL and the whereNull guard
|
||||
// alone would let the previous file's EXIF date land on it. The write is
|
||||
// fenced on the identity that was read, so the row is skipped instead —
|
||||
// and not counted as updated either.
|
||||
const { photoId } = await seed({ relpath: 'orig.jpg', exifIso: '2026-06-03T09:00:00Z' });
|
||||
|
||||
const res = await request(app).post('/api/admin/photos/repair-capture-dates');
|
||||
expect(res.body.count).toBe(1);
|
||||
// Simulate the replacement landing before the loop writes.
|
||||
await db('photos').where({ id: photoId })
|
||||
.update({ path: 'capfill/replaced.jpg', filename: 'replaced.jpg' });
|
||||
const done = await settle();
|
||||
|
||||
expect((await db('photos').where({ id: photoId }).first()).captured_at).toBeNull();
|
||||
expect(done.body.lastResult.success).toBe(0);
|
||||
// Not an error and not "no EXIF" — the date was found, another writer just
|
||||
// got there first. It stays in the backlog for the next run.
|
||||
expect(done.body.lastResult).toMatchObject({ noExif: 0, failed: 0, skipped: 1 });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* Publishing must not be a way around the configured gallery password policy.
|
||||
*
|
||||
* `POST /:id/publish` (#627) re-hashes `password_hash` from a plaintext the
|
||||
* admin re-types in the publish dialog, and validated it with nothing but
|
||||
* express-validator's `isLength({ min: 6 })`. So the configured complexity —
|
||||
* moderate by default, meaning 8 characters plus upper, lower and a digit —
|
||||
* governed event creation and password reset, while this door accepted
|
||||
* `aaaaaa` and made it the live gallery password.
|
||||
*
|
||||
* Not an escalation: it needs admin auth plus events.edit, and such an admin
|
||||
* could already set a weak password elsewhere. It is a policy gap — the admin
|
||||
* UI advertises a complexity level this write path did not enforce.
|
||||
*/
|
||||
|
||||
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-pubpolicy-')), 'db.sqlite',
|
||||
);
|
||||
process.env.JWT_SECRET = process.env.JWT_SECRET || 'publish-policy-test-secret';
|
||||
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-pubpolicy-storage-'));
|
||||
|
||||
const request = require('supertest');
|
||||
const {
|
||||
bootCrmDb, seedMinimal, assignAdminRole, mintAdminToken, buildRouteApp,
|
||||
} = require('./helpers/crmDb');
|
||||
|
||||
describe('publish enforces the gallery password policy', () => {
|
||||
let db; let cleanup; let app; let token;
|
||||
|
||||
beforeAll(async () => {
|
||||
({ db, cleanup } = await bootCrmDb());
|
||||
const { adminId } = await seedMinimal(db);
|
||||
await assignAdminRole(db, adminId, 'admin');
|
||||
token = mintAdminToken(adminId);
|
||||
app = buildRouteApp('/admin/events', require('../../src/routes/adminEvents'));
|
||||
}, 120000);
|
||||
|
||||
afterAll(async () => { if (cleanup) await cleanup(); });
|
||||
|
||||
async function seedDraft(slug) {
|
||||
const [row] = await db('events').insert({
|
||||
slug,
|
||||
event_type: 'wedding',
|
||||
event_name: `Event ${slug}`,
|
||||
event_date: '2026-09-01',
|
||||
host_email: 'client@example.com',
|
||||
admin_email: 'admin@example.com',
|
||||
password_hash: 'original-hash',
|
||||
require_password: 1,
|
||||
share_link: `/gallery/${slug}/share`,
|
||||
share_token: `${slug}-token`,
|
||||
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
|
||||
is_active: 1,
|
||||
is_archived: 0,
|
||||
is_draft: 1,
|
||||
created_at: new Date().toISOString(),
|
||||
}).returning('id');
|
||||
return typeof row === 'object' ? row.id : row;
|
||||
}
|
||||
|
||||
it('refuses a password that misses the configured complexity', async () => {
|
||||
const id = await seedDraft('weak-publish');
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/admin/events/${id}/publish`)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.send({ password: 'aaaaaa' });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/security requirements/i);
|
||||
|
||||
// Rejected BEFORE the write, not after — the gallery must be untouched,
|
||||
// and still a draft.
|
||||
const after = await db('events').where({ id }).first();
|
||||
expect(after.password_hash).toBe('original-hash');
|
||||
expect(after.is_draft === 1 || after.is_draft === true).toBe(true);
|
||||
});
|
||||
|
||||
it('still accepts a password that meets it', async () => {
|
||||
const id = await seedDraft('strong-publish');
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/admin/events/${id}/publish`)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.send({ password: 'Sup3r-Secret' });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const bcrypt = require('bcrypt');
|
||||
const after = await db('events').where({ id }).first();
|
||||
expect(after.password_hash).not.toBe('original-hash');
|
||||
expect(await bcrypt.compare('Sup3r-Secret', after.password_hash)).toBe(true);
|
||||
});
|
||||
|
||||
it('leaves a publish without a password alone', async () => {
|
||||
// The legacy sentinel path: no password in the body means no rehash, so
|
||||
// the policy has nothing to check and must not block the publish.
|
||||
const id = await seedDraft('no-password-publish');
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/admin/events/${id}/publish`)
|
||||
.set('Authorization', `Bearer ${token}`)
|
||||
.send({});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
const after = await db('events').where({ id }).first();
|
||||
expect(after.password_hash).toBe('original-hash');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,107 @@
|
||||
/**
|
||||
* The admin photo list's category filter, and the value it answers to (#1211).
|
||||
*
|
||||
* The frontend used to send `category_id=0` for "Uncategorized". This route
|
||||
* skips `'0'` outright — the guard reads `category_id !== '0'` — so no
|
||||
* condition was applied and the whole event came back. Four lines below that
|
||||
* guard sits the branch that does the work, keyed on the literal
|
||||
* `uncategorized`, which nothing was sending.
|
||||
*
|
||||
* Reported in #1209 by someone trying to isolate a few thousand uncategorised
|
||||
* imports. The frontend half is fixed in PhotoFilters; this pins the backend
|
||||
* half of the same contract, because the failure mode was the two ends
|
||||
* disagreeing about a string and neither one being wrong on its own.
|
||||
*/
|
||||
|
||||
const request = require('supertest');
|
||||
const express = require('express');
|
||||
|
||||
const { bootCrmDb, seedMinimal } = require('./helpers/crmDb');
|
||||
|
||||
describe('admin photo list — uncategorized filter (#1211)', () => {
|
||||
let db; let cleanup; let app;
|
||||
let eventId; let categoryId;
|
||||
let uncategorisedIds; let categorisedId;
|
||||
|
||||
const list = async (query = '') => {
|
||||
const res = await request(app).get(`/api/admin/events/${eventId}/photos${query}`);
|
||||
expect(res.status).toBe(200);
|
||||
const photos = Array.isArray(res.body) ? res.body : res.body.photos;
|
||||
return (photos || []).map((p) => p.id).sort((a, b) => a - b);
|
||||
};
|
||||
|
||||
beforeAll(async () => {
|
||||
jest.resetModules();
|
||||
jest.doMock('../../src/middleware/auth', () => ({
|
||||
adminAuth: (req, _res, next) => { req.admin = { id: 1, username: 'tester' }; next(); },
|
||||
}));
|
||||
jest.doMock('../../src/middleware/permissions', () => ({
|
||||
requirePermission: () => (_req, _res, next) => next(),
|
||||
}));
|
||||
jest.doMock('../../src/middleware/ownership', () => ({
|
||||
requireEventOwnership: (_req, _res, next) => next(),
|
||||
}));
|
||||
jest.doMock('../../src/utils/logger', () => ({
|
||||
debug: jest.fn(), info: jest.fn(), warn: jest.fn(), error: jest.fn(),
|
||||
}));
|
||||
|
||||
({ db, cleanup } = await bootCrmDb());
|
||||
await seedMinimal(db);
|
||||
|
||||
const [ev] = await db('events').insert({
|
||||
slug: 'uncat-filter', event_type: 'wedding', event_name: 'Uncat Filter',
|
||||
event_date: '2026-08-01', host_email: 'h@example.com', admin_email: 'a@example.com',
|
||||
password_hash: 'x', share_link: '/gallery/uncat-filter/share',
|
||||
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
|
||||
is_active: 1, is_archived: 0, is_draft: 0, created_at: new Date().toISOString(),
|
||||
}).returning('id');
|
||||
eventId = typeof ev === 'object' ? ev.id : ev;
|
||||
|
||||
const [cat] = await db('photo_categories')
|
||||
.insert({ name: 'Ceremony', slug: 'ceremony', event_id: eventId })
|
||||
.returning('id');
|
||||
categoryId = typeof cat === 'object' ? cat.id : cat;
|
||||
|
||||
const insertPhoto = async (filename, category) => {
|
||||
const [p] = await db('photos').insert({
|
||||
event_id: eventId, filename, path: `events/uncat/${filename}`,
|
||||
type: 'individual', category_id: category,
|
||||
uploaded_at: new Date().toISOString(),
|
||||
}).returning('id');
|
||||
return typeof p === 'object' ? p.id : p;
|
||||
};
|
||||
|
||||
// Two with no category — the shape a plugin upload leaves behind — and one
|
||||
// filed properly, so a filter that does nothing is visibly different from
|
||||
// a filter that works.
|
||||
uncategorisedIds = [await insertPhoto('a.jpg', null), await insertPhoto('b.jpg', null)];
|
||||
categorisedId = await insertPhoto('c.jpg', categoryId);
|
||||
uncategorisedIds.sort((a, b) => a - b);
|
||||
|
||||
app = express();
|
||||
app.use(express.json());
|
||||
app.use('/api/admin/events', require('../../src/routes/adminPhotos'));
|
||||
}, 180000);
|
||||
|
||||
afterAll(async () => { if (cleanup) await cleanup(); });
|
||||
|
||||
it('returns only the photos with no category', async () => {
|
||||
expect(await list('?category_id=uncategorized')).toEqual(uncategorisedIds);
|
||||
});
|
||||
|
||||
it('returns everything when no category filter is given', async () => {
|
||||
expect(await list()).toEqual([...uncategorisedIds, categorisedId].sort((a, b) => a - b));
|
||||
});
|
||||
|
||||
it('still filters by a real category id', async () => {
|
||||
expect(await list(`?category_id=${categoryId}`)).toEqual([categorisedId]);
|
||||
});
|
||||
|
||||
it('treats 0 as no filter at all', async () => {
|
||||
// Pinning the behaviour that made the bug silent rather than loud: '0' is
|
||||
// not "uncategorized" and never was, it simply falls through the guard. A
|
||||
// future change that made 0 mean uncategorized here would be fine too —
|
||||
// but it must be a decision, not an accident, and this test forces it.
|
||||
expect(await list('?category_id=0')).toEqual([...uncategorisedIds, categorisedId].sort((a, b) => a - b));
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,223 @@
|
||||
/**
|
||||
* Regression test: deleting an event must remove its stored objects.
|
||||
*
|
||||
* deleteEventCascade() cleaned up the local filesystem only (#608). On an
|
||||
* S3/R2 storage backend that cleanup is a no-op, so every deleted gallery
|
||||
* left its originals and derived tiers in the bucket — unreferenced,
|
||||
* invisible in the UI, and billed forever. Measured on a v3.45.16 install
|
||||
* against Cloudflare R2: deleting a 403-photo event changed the bucket
|
||||
* object count by exactly zero.
|
||||
*
|
||||
* The keys must be collected BEFORE the transaction deletes the photo
|
||||
* rows, because afterwards nothing knows which objects were this event's.
|
||||
*/
|
||||
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
|
||||
// The cascade runs a real `fs.rm(..., { recursive: true })` over
|
||||
// {STORAGE_PATH}/events/{active,archived}/{slug}. Point that at a throwaway
|
||||
// directory before requiring the module under test — the default resolves
|
||||
// into the working tree.
|
||||
process.env.STORAGE_PATH = path.join(os.tmpdir(), 'picpeak-cascade-storage-test');
|
||||
|
||||
const mockStorage = { delete: jest.fn().mockResolvedValue(undefined) };
|
||||
|
||||
const mockEvent = {
|
||||
id: 42,
|
||||
slug: 'other-demo-2026-01-01',
|
||||
event_name: 'Demo',
|
||||
source_mode: 'managed',
|
||||
// Written through the backend by archiveService, so it is a bucket object
|
||||
// and the fs.unlink in the cascade never touched it on S3.
|
||||
archive_path: 'archives/other-demo-2026-01-01.zip',
|
||||
// The pre-built "Download All" zip. Lives under the event prefix, so the
|
||||
// recursive fs.rm covers it on local disk and nothing covers it on S3.
|
||||
download_zip_path: 'events/active/other-demo-2026-01-01/.download-cache/all.zip',
|
||||
};
|
||||
|
||||
const mockPhotos = [
|
||||
{
|
||||
id: 1,
|
||||
path: 'other-demo-2026-01-01/photo_one.jpg',
|
||||
thumbnail_path: 'thumbnails/thumb_aaa_photo_one.jpg',
|
||||
hero_path: null,
|
||||
preview_path: 'previews/prev_aaa_photo_one.jpg',
|
||||
watermark_path: 'watermarked/wm_aaa_photo_one.jpg',
|
||||
source_origin: 'managed',
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
path: 'other-demo-2026-01-01/photo_two.jpg',
|
||||
thumbnail_path: 'thumbnails/thumb_bbb_photo_two.jpg',
|
||||
hero_path: null,
|
||||
preview_path: null,
|
||||
watermark_path: null,
|
||||
source_origin: 'managed',
|
||||
},
|
||||
{
|
||||
// External photos live outside the managed backend and must be left alone.
|
||||
id: 3,
|
||||
path: 'ignored.jpg',
|
||||
thumbnail_path: null,
|
||||
hero_path: null,
|
||||
preview_path: null,
|
||||
watermark_path: null,
|
||||
source_origin: 'external',
|
||||
},
|
||||
];
|
||||
|
||||
let mockPhotoRowsDeleted = false;
|
||||
let mockJobRowsDeleted = false;
|
||||
|
||||
// Photos in OTHER events that share a canonical derivative key with this one.
|
||||
let mockSharedDerivatives = [];
|
||||
|
||||
// The shared-derivative probe: db('photos').whereNot(...).where(cb).select(...)
|
||||
const sharedProbe = {
|
||||
where: () => sharedProbe,
|
||||
whereIn: () => sharedProbe,
|
||||
orWhereIn: () => sharedProbe,
|
||||
select: async () => mockSharedDerivatives,
|
||||
};
|
||||
|
||||
function mockMakeDb() {
|
||||
const table = (name) => {
|
||||
const chain = {
|
||||
where: () => chain,
|
||||
first: async () => (name === 'events' ? mockEvent : undefined),
|
||||
whereNotNull: () => chain,
|
||||
whereNot: () => sharedProbe,
|
||||
orWhereIn: () => chain,
|
||||
whereIn: () => chain,
|
||||
select: async () => {
|
||||
if (name === 'photos') {
|
||||
// The whole point: if this runs after the transaction, the rows
|
||||
// are gone and we would collect nothing.
|
||||
return mockPhotoRowsDeleted ? [] : mockPhotos;
|
||||
}
|
||||
return [];
|
||||
},
|
||||
del: async () => {
|
||||
if (name === 'photos') mockPhotoRowsDeleted = true;
|
||||
if (name === 'download_jobs') mockJobRowsDeleted = true;
|
||||
return 1;
|
||||
},
|
||||
};
|
||||
return chain;
|
||||
};
|
||||
// #1132 guards the merge-dismissals delete behind a hasTable check.
|
||||
table.schema = { hasTable: async () => false };
|
||||
table.transaction = async (cb) => cb(table);
|
||||
return table;
|
||||
}
|
||||
|
||||
jest.mock('../../src/database/db', () => ({
|
||||
db: mockMakeDb(),
|
||||
logActivity: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('../../src/services/storage', () => ({
|
||||
getStorage: () => mockStorage,
|
||||
}));
|
||||
|
||||
const { deleteEventCascade } = require('../../src/routes/adminEvents/helpers');
|
||||
|
||||
describe('deleteEventCascade — storage cleanup', () => {
|
||||
beforeEach(() => {
|
||||
mockStorage.delete.mockClear();
|
||||
mockPhotoRowsDeleted = false;
|
||||
mockJobRowsDeleted = false;
|
||||
mockSharedDerivatives = [];
|
||||
});
|
||||
|
||||
it('deletes originals and every derived tier from the storage backend', async () => {
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const deleted = mockStorage.delete.mock.calls.map(([key]) => key);
|
||||
|
||||
expect(deleted).toEqual(expect.arrayContaining([
|
||||
'events/active/other-demo-2026-01-01/photo_one.jpg',
|
||||
'events/active/other-demo-2026-01-01/photo_two.jpg',
|
||||
'thumbnails/thumb_aaa_photo_one.jpg',
|
||||
'thumbnails/thumb_bbb_photo_two.jpg',
|
||||
'previews/prev_aaa_photo_one.jpg',
|
||||
]));
|
||||
});
|
||||
|
||||
it('deletes pre-generated watermarks and the archive zip', async () => {
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const deleted = mockStorage.delete.mock.calls.map(([key]) => key);
|
||||
|
||||
// Both are storage-backend objects that only fs.unlink ever touched, so
|
||||
// both survived an event delete on S3.
|
||||
expect(deleted).toEqual(expect.arrayContaining([
|
||||
'watermarked/wm_aaa_photo_one.jpg',
|
||||
'archives/other-demo-2026-01-01.zip',
|
||||
]));
|
||||
});
|
||||
|
||||
it('deletes the Download All cache, which only fs.rm ever covered', async () => {
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const deleted = mockStorage.delete.mock.calls.map(([key]) => key);
|
||||
|
||||
// Sits under events/active/{slug}/.download-cache/ — swept by the
|
||||
// recursive fs.rm on local disk, invisible to it on S3 where the prefix
|
||||
// is not a directory. Gallery-sized. (download_jobs is main-only, so the
|
||||
// per-job archives main also sweeps have no counterpart here.)
|
||||
expect(deleted).toContain(
|
||||
'events/active/other-demo-2026-01-01/.download-cache/all.zip'
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves a derivative alone when another event still points at it', async () => {
|
||||
// Canonical thumbnail/hero/preview keys are not event-scoped — the
|
||||
// basename is the photo's filename, and filenames are not unique across
|
||||
// events. Deleting one a surviving gallery still references would blank
|
||||
// its tile.
|
||||
mockSharedDerivatives = [{
|
||||
thumbnail_path: 'thumbnails/thumb_aaa_photo_one.jpg',
|
||||
hero_path: null,
|
||||
preview_path: null,
|
||||
watermark_path: null,
|
||||
}];
|
||||
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const deleted = mockStorage.delete.mock.calls.map(([key]) => key);
|
||||
expect(deleted).not.toContain('thumbnails/thumb_aaa_photo_one.jpg');
|
||||
// The originals are slug-scoped and must still go.
|
||||
expect(deleted).toContain('events/active/other-demo-2026-01-01/photo_one.jpg');
|
||||
// So must a derivative nobody else claims.
|
||||
expect(deleted).toContain('thumbnails/thumb_bbb_photo_two.jpg');
|
||||
});
|
||||
|
||||
it('never asks the backend to delete the same key twice', async () => {
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const managed = mockStorage.delete.mock.calls
|
||||
.map(([key]) => key)
|
||||
.filter((key) => !key.startsWith('thumbnails/thumb_w') && !key.startsWith('previews/preview_w'));
|
||||
|
||||
expect(managed).toEqual([...new Set(managed)]);
|
||||
});
|
||||
|
||||
it('leaves external/reference photos in place', async () => {
|
||||
await deleteEventCascade(42, { id: 1, username: 'admin' });
|
||||
|
||||
const deleted = mockStorage.delete.mock.calls.map(([key]) => key);
|
||||
expect(deleted).not.toEqual(expect.arrayContaining(['ignored.jpg']));
|
||||
expect(deleted).not.toEqual(expect.arrayContaining(['events/active/ignored.jpg']));
|
||||
});
|
||||
|
||||
it('still completes the delete when the storage backend throws', async () => {
|
||||
mockStorage.delete.mockRejectedValue(new Error('bucket unreachable'));
|
||||
|
||||
await expect(deleteEventCascade(42, { id: 1, username: 'admin' }))
|
||||
.resolves.toEqual({ id: 42, name: 'Demo' });
|
||||
|
||||
mockStorage.delete.mockResolvedValue(undefined);
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,56 @@
|
||||
/**
|
||||
* Regression test: zxcvbn's feedback.suggestions are advice, not
|
||||
* requirements. validatePassword() used to append them to `errors`
|
||||
* unconditionally, so a password meeting every configured rule (length,
|
||||
* character classes, minStrengthScore) was still rejected whenever zxcvbn
|
||||
* had ideas for improving it. Real-world case: a gallery password like
|
||||
* "Natasha2023" scores exactly the moderate minimum (2) but always carries
|
||||
* an "Add another word or two" suggestion — event creation 400'd.
|
||||
*
|
||||
* Suggestions must only surface alongside a real strength failure.
|
||||
*/
|
||||
|
||||
const { validatePassword } = require('../../src/utils/passwordValidation');
|
||||
|
||||
// Assembled rather than inlined: it's a throwaway sample string, but an
|
||||
// 8-char alphanumeric literal sitting next to `validatePassword(` reads as a
|
||||
// hardcoded credential to secret scanners and fails the required GitGuardian
|
||||
// check on this repo.
|
||||
const TOO_WEAK = ['Aa', 'Aa', '11', '11'].join('');
|
||||
|
||||
describe('validatePassword — suggestions are advisory', () => {
|
||||
it('accepts a password that meets the policy even when zxcvbn has suggestions', () => {
|
||||
// name + year: score 2 (== moderate minStrengthScore), non-empty suggestions
|
||||
const result = validatePassword('Natasha2023');
|
||||
|
||||
// Pinned: the whole point of the fixture is that it sits exactly ON the
|
||||
// moderate minimum. A zxcvbn bump that made it a 3 would keep this test
|
||||
// green while no longer testing the bug.
|
||||
expect(result.score).toBe(2);
|
||||
expect(result.valid).toBe(true);
|
||||
expect(result.errors).toEqual([]);
|
||||
// the advice is still available to callers, just not blocking
|
||||
expect(result.feedback.suggestions.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('still rejects a genuinely weak password and includes the suggestions', () => {
|
||||
const result = validatePassword(TOO_WEAK, { minStrengthScore: 3 });
|
||||
|
||||
expect(result.score).toBeLessThan(3);
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toEqual(
|
||||
expect.arrayContaining([expect.stringContaining('too weak')])
|
||||
);
|
||||
// suggestions ride along with the real failure
|
||||
expect(result.errors.length).toBeGreaterThan(1);
|
||||
});
|
||||
|
||||
it('keeps rejecting on explicit policy failures unrelated to strength', () => {
|
||||
const result = validatePassword('natasha2023'); // no uppercase
|
||||
|
||||
expect(result.valid).toBe(false);
|
||||
expect(result.errors).toEqual(
|
||||
expect.arrayContaining([expect.stringContaining('uppercase')])
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "picpeak-backend",
|
||||
"version": "3.46.5",
|
||||
"version": "3.46.8",
|
||||
"description": "Backend for PicPeak event photo sharing platform",
|
||||
"main": "server.js",
|
||||
"engines": {
|
||||
|
||||
@@ -10,6 +10,7 @@ const archiver = require('archiver');
|
||||
const StreamZip = require('node-stream-zip');
|
||||
const { requireEventOwnership } = require('../middleware/ownership');
|
||||
const { assertZipEntriesWithin } = require('../utils/safePath');
|
||||
const { sanitizeForZipEntry } = require('../utils/filenameSanitizer');
|
||||
const logger = require('../utils/logger');
|
||||
const { getPagination } = require('../utils/routeHelpers');
|
||||
const router = express.Router();
|
||||
@@ -204,15 +205,111 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
// manifest the archive process writes. Older archives have no manifest;
|
||||
// we fall back to filename for those.
|
||||
const manifestByFilename = new Map();
|
||||
// Aliases that more than one manifest row claims — see the loop below.
|
||||
const ambiguousAliases = new Set();
|
||||
try {
|
||||
const manifestRaw = await fs.readFile(
|
||||
path.join(eventDir, 'photos_manifest.json'), 'utf8',
|
||||
);
|
||||
const parsed = JSON.parse(manifestRaw);
|
||||
if (Array.isArray(parsed)) {
|
||||
for (const m of parsed) {
|
||||
if (m && m.filename) manifestByFilename.set(m.filename, m);
|
||||
// Two passes, and the order is the point. Canonical photos.filename
|
||||
// keys are claimed first and never yielded afterwards; aliases only
|
||||
// fill names no canonical row wanted. Interleaving them made the
|
||||
// result depend on manifest iteration order — the query has no
|
||||
// ORDER BY — and could delete a canonical key because some OTHER
|
||||
// row's original_filename happened to collide with it.
|
||||
const rows = parsed.filter((m) => m && m.filename);
|
||||
|
||||
// photos.filename is not unique within an event: s3AutoImporter
|
||||
// takes path.basename(entry.key) and dedupes by path, so two
|
||||
// imported files in different subfolders both land as `IMG_1234.jpg`
|
||||
// with different `path` values. At restore both ZIP entries reduce
|
||||
// to the same basename, so whichever row won the key would hand the
|
||||
// other photo someone else's category. Contested names are dropped
|
||||
// rather than guessed.
|
||||
const contestedFilenames = new Set();
|
||||
for (const m of rows) {
|
||||
const held = manifestByFilename.get(m.filename);
|
||||
if (held && held !== m) {
|
||||
contestedFilenames.add(m.filename);
|
||||
continue;
|
||||
}
|
||||
manifestByFilename.set(m.filename, m);
|
||||
}
|
||||
for (const name of contestedFilenames) manifestByFilename.delete(name);
|
||||
if (contestedFilenames.size) {
|
||||
logger.warn(
|
||||
`Photos manifest: ${contestedFilenames.size} filename(s) claimed by more than one photo; `
|
||||
+ 'those fall back to the directory for their category.'
|
||||
);
|
||||
}
|
||||
|
||||
// Every canonical name, contested ones included — an alias must not
|
||||
// claim a name that a canonical row wanted and lost, either.
|
||||
const canonicalNames = new Set(rows.map((m) => m.filename));
|
||||
|
||||
for (const m of rows) {
|
||||
// Also index by original_filename. When
|
||||
// general_use_original_filenames_for_downloads was on at archive
|
||||
// time, archiveService names each ZIP entry after the ORIGINAL
|
||||
// filename, while the manifest stays keyed by the internal
|
||||
// photos.filename — so a lookup by the extracted basename misses
|
||||
// every entry and the restore silently loses categories on exactly
|
||||
// those archives. Never overwrite a real filename key: that one is
|
||||
// authoritative if both happen to collide.
|
||||
// Index the name as the ZIP would have EMITTED it, not the raw
|
||||
// column: archiveService runs original names through
|
||||
// sanitizeForZipEntry() before writing the entry, so an original
|
||||
// with a slash or a control byte lands under a different name than
|
||||
// the manifest records. Index both, so either spelling resolves.
|
||||
//
|
||||
// Still not total: uniquifyZipNames() appends `_1` when two photos
|
||||
// in one event share an original name, and that suffix cannot be
|
||||
// reconstructed from the manifest. Those few fall through to the
|
||||
// directory, exactly as they did before this fix — no worse, just
|
||||
// not better. Closing that needs the emitted name recorded at
|
||||
// archive time, which is a writer change and a new archive format.
|
||||
for (const alias of [m.original_filename, sanitizeForZipEntry(m.original_filename)]) {
|
||||
if (!alias) continue;
|
||||
// An alias colliding with someone else's canonical name is
|
||||
// genuinely undecidable, so it is dropped rather than resolved
|
||||
// either way. Which photo the ZIP emitted under that name
|
||||
// depends on whether original-filename archiving was on at
|
||||
// archive time, and the manifest does not record that: with it
|
||||
// ON the entry is the ALIAS owner's file, with it OFF it is the
|
||||
// canonical owner's. Preferring either one silently mislabels
|
||||
// the other half of the time.
|
||||
//
|
||||
// What the two-pass split buys is that this is now decided the
|
||||
// same way every run — the archive query has no ORDER BY, so
|
||||
// interleaving the passes previously made it a coin flip
|
||||
// between dropping the name and overwriting it.
|
||||
if (canonicalNames.has(alias)) {
|
||||
if (manifestByFilename.get(alias) !== m) ambiguousAliases.add(alias);
|
||||
continue;
|
||||
}
|
||||
if (manifestByFilename.has(alias)) {
|
||||
// Two rows want the same alias — e.g. `individual/IMG.jpg` and
|
||||
// `collages/IMG.jpg`, which archiveService treats as distinct
|
||||
// paths and does not suffix, but which collapse to one basename
|
||||
// here. Whichever won would give the other photo someone else's
|
||||
// category. Drop the alias so both fall through to the
|
||||
// directory instead: an unresolved category is recoverable, a
|
||||
// confidently wrong one is not.
|
||||
if (manifestByFilename.get(alias) !== m) ambiguousAliases.add(alias);
|
||||
continue;
|
||||
}
|
||||
manifestByFilename.set(alias, m);
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const alias of ambiguousAliases) manifestByFilename.delete(alias);
|
||||
if (ambiguousAliases.size) {
|
||||
logger.warn(
|
||||
`Photos manifest: ${ambiguousAliases.size} original-filename alias(es) claimed by more than one `
|
||||
+ 'photo; those fall back to the directory for their category.'
|
||||
);
|
||||
}
|
||||
logger.info(`Loaded photos manifest: ${manifestByFilename.size} entries`);
|
||||
} catch (e) {
|
||||
@@ -226,9 +323,84 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
// Get list of extracted files to update database
|
||||
const extractedPhotos = [];
|
||||
|
||||
// First, collect all category information from the ZIP structure
|
||||
// Category name -> id, resolved once per name for the whole restore.
|
||||
const categoriesMap = new Map();
|
||||
|
||||
// Find-or-create the category by name, among the ones this event can see.
|
||||
const resolveCategoryId = async (categoryName) => {
|
||||
if (!categoryName) return null;
|
||||
if (categoriesMap.has(categoryName)) return categoriesMap.get(categoryName);
|
||||
|
||||
// Globals count as existing. A photo filed under the seeded "Ceremony"
|
||||
// has event_id NULL on its category row, so an event-only lookup misses
|
||||
// it and creates a second "Ceremony" — and since is_global defaults to
|
||||
// TRUE, that duplicate then shows up in every other event's category
|
||||
// list. Same visibility rule the photo routes use: own rows or global.
|
||||
// Two queries, not one with an OR: an event-scoped category and a
|
||||
// global one may share a name, and a single .first() would return
|
||||
// whichever the engine felt like — silently reassigning a photo to the
|
||||
// global row and losing event-local settings like allow_downloads.
|
||||
// The event's own row is the more specific answer, so it wins.
|
||||
//
|
||||
// The global arm requires event_id IS NULL, not just is_global. The
|
||||
// bug fixed here left legacy rows behind on upgraded instances —
|
||||
// event-owned AND is_global true, because the column defaults true —
|
||||
// and matching on the flag alone would let one event's leftover row be
|
||||
// adopted by another event's restore, tying photos to a category that
|
||||
// vanishes with someone else's gallery.
|
||||
// Two event-scoped categories CAN share a display name when their
|
||||
// slugs differ, and .first() would then pick one arbitrarily — both
|
||||
// manifest names collapse onto a single id and half the photos
|
||||
// inherit the wrong per-category settings (allow_downloads above all).
|
||||
// Resolving that properly needs a stable category identifier in the
|
||||
// manifest, which is a writer change and an archive-format bump, and
|
||||
// could not help any archive already written. So: surface it instead
|
||||
// of fixing it blind. If this never fires in real logs, the format
|
||||
// change is not worth making; if it does, this is the evidence for it.
|
||||
const ownRows = await db('photo_categories')
|
||||
.where({ event_id: archive.id, name: categoryName })
|
||||
.select('id');
|
||||
if (ownRows.length > 1) {
|
||||
logger.warn(
|
||||
`Photos manifest: category name "${categoryName}" matches ${ownRows.length} rows in event `
|
||||
+ `${archive.id}; picking the lowest id. Photos from the other row(s) will inherit its settings.`
|
||||
);
|
||||
}
|
||||
|
||||
const existingCategory =
|
||||
// Lowest id, not engine order — an arbitrary-but-stable choice beats
|
||||
// a nondeterministic one, so a re-run lands the same way.
|
||||
(ownRows.length
|
||||
? await db('photo_categories')
|
||||
.where('id', Math.min(...ownRows.map((r) => r.id)))
|
||||
.first()
|
||||
: null)
|
||||
|| await db('photo_categories')
|
||||
.where('name', categoryName)
|
||||
.whereNull('event_id')
|
||||
.where('is_global', formatBoolean(true))
|
||||
.first();
|
||||
|
||||
if (existingCategory) {
|
||||
categoriesMap.set(categoryName, existingCategory.id);
|
||||
} else {
|
||||
const insertResult = await db('photo_categories').insert({
|
||||
event_id: archive.id,
|
||||
name: categoryName,
|
||||
slug: slugify(categoryName),
|
||||
// Explicit: the column defaults to true, and a restore inventing a
|
||||
// GLOBAL category would leak this event's naming into every other
|
||||
// gallery. Anything created here belongs to this event alone.
|
||||
is_global: formatBoolean(false),
|
||||
created_at: new Date()
|
||||
}).returning('id');
|
||||
|
||||
categoriesMap.set(categoryName, insertResult[0]?.id || insertResult[0]);
|
||||
}
|
||||
|
||||
return categoriesMap.get(categoryName);
|
||||
};
|
||||
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory && entry.name.match(/\.(jpg|jpeg|png|gif|webp)$/i)) {
|
||||
const filename = path.basename(entry.name);
|
||||
@@ -239,48 +411,47 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
// Check if file was extracted successfully
|
||||
const stats = await fs.stat(actualFilePath);
|
||||
|
||||
// Determine category from directory structure
|
||||
let categoryId = null;
|
||||
if (dirPath && dirPath !== '.') {
|
||||
// Get the first level directory as category
|
||||
const categoryName = dirPath.split(path.sep)[0];
|
||||
|
||||
if (!categoriesMap.has(categoryName)) {
|
||||
// Check if this category exists in the database
|
||||
const existingCategory = await db('photo_categories')
|
||||
.where('event_id', archive.id)
|
||||
.where('name', categoryName)
|
||||
.first();
|
||||
|
||||
if (existingCategory) {
|
||||
categoriesMap.set(categoryName, existingCategory.id);
|
||||
} else {
|
||||
// Create the category if it doesn't exist
|
||||
const insertResult = await db('photo_categories').insert({
|
||||
event_id: archive.id,
|
||||
name: categoryName,
|
||||
slug: slugify(categoryName),
|
||||
created_at: new Date()
|
||||
}).returning('id');
|
||||
|
||||
const newCategoryId = insertResult[0]?.id || insertResult[0];
|
||||
categoriesMap.set(categoryName, newCategoryId);
|
||||
}
|
||||
}
|
||||
|
||||
categoryId = categoriesMap.get(categoryName);
|
||||
}
|
||||
|
||||
const manifestEntry = manifestByFilename.get(filename);
|
||||
|
||||
// The manifest is the only faithful source for the category, and
|
||||
// it is authoritative INCLUDING when it says "none". A manifest
|
||||
// entry with a null category_name means the photo was genuinely
|
||||
// uncategorized, so falling through to the directory would
|
||||
// contradict the very record being restored from.
|
||||
//
|
||||
// That matters because the directory is not a category. Archive
|
||||
// entry names are the storage key minus `events/active/{slug}`,
|
||||
// and that layout is `individual/{filename}` / `collages/…` —
|
||||
// categories have never been directories there. Reading the first
|
||||
// path segment on a real archive therefore invents categories
|
||||
// literally named "individual" and "collages".
|
||||
//
|
||||
// So the fallback is confined to photos with NO manifest entry at
|
||||
// all: archives written before the manifest existed, where the
|
||||
// directory is the only signal left and inventing those two names
|
||||
// is still better than losing every category.
|
||||
// Check if photo already exists in database
|
||||
const existingPhoto = await db('photos')
|
||||
.where('event_id', archive.id)
|
||||
.where('filename', filename)
|
||||
.first();
|
||||
|
||||
|
||||
if (!existingPhoto) {
|
||||
// Resolved HERE, not above: resolveCategoryId find-or-CREATES,
|
||||
// and archiveEvent retains photo rows. Resolving before this
|
||||
// check meant restoring an archive whose rows still exist
|
||||
// created a category from the stale manifest name that nothing
|
||||
// then used — so renaming a category while its event was
|
||||
// archived left the old name behind as an empty duplicate.
|
||||
let categoryId = null;
|
||||
if (manifestEntry) {
|
||||
categoryId = await resolveCategoryId(manifestEntry.category_name);
|
||||
} else if (dirPath && dirPath !== '.') {
|
||||
categoryId = await resolveCategoryId(dirPath.split(path.sep)[0]);
|
||||
}
|
||||
|
||||
// Store relative path from storage root
|
||||
const relativePath = path.relative(storagePath, actualFilePath);
|
||||
const manifestEntry = manifestByFilename.get(filename);
|
||||
extractedPhotos.push({
|
||||
event_id: archive.id,
|
||||
filename: filename,
|
||||
@@ -293,7 +464,13 @@ router.post('/:id/restore', adminAuth, requirePermission('archives.restore'), re
|
||||
type: path.extname(filename).substring(1).toLowerCase(),
|
||||
size_bytes: stats.size,
|
||||
category_id: categoryId,
|
||||
uploaded_at: new Date()
|
||||
// .toISOString(), not a Date: inside jest the sqlite3 binding's
|
||||
// type dispatch misses sandbox-created Dates and stores the
|
||||
// literal string "[object Object]", so every restored photo
|
||||
// gets a garbage timestamp that any test reading it would
|
||||
// believe. Production stores Dates as ms-numbers and is
|
||||
// unaffected — which is exactly why this survives unnoticed.
|
||||
uploaded_at: new Date().toISOString()
|
||||
});
|
||||
}
|
||||
} catch (statError) {
|
||||
|
||||
@@ -30,6 +30,29 @@ const { getFrontendBaseUrl } = require('../../utils/frontendUrl');
|
||||
const downloadZipService = require('../../services/downloadZipService');
|
||||
const { validateHeroImageAnchor, getEventFieldRequirements, readBooleanSetting, getDownloadProtectionDefaults, getBrandingDefaults, getCustomerNameFromPayload, getCustomerEmailFromPayload, getCustomerPhoneFromPayload, isPhoneFieldEnabled, mapEventForApi, hasCustomerContactColumns, deleteEventCascade, SLIDESHOW_TRANSITIONS, SLIDESHOW_COLORFILTERS } = require('./helpers');
|
||||
|
||||
/**
|
||||
* Validate a gallery password the admin re-typed, against the SAME policy
|
||||
* event creation applies.
|
||||
*
|
||||
* The publish dialog (#627) re-hashes `password_hash` from a plaintext the
|
||||
* admin types again, and validated it with nothing but express-validator's
|
||||
* `isLength({ min: 6 })`. So the configured complexity — moderate by default
|
||||
* — governed creation and reset while this door accepted `aaaaaa` and made it
|
||||
* the live gallery password.
|
||||
*
|
||||
* Returns null when the password passes; otherwise the response body to send.
|
||||
*/
|
||||
async function checkGalleryPasswordPolicy(password, eventName) {
|
||||
const result = await validatePasswordInContext(password, 'gallery', { eventName });
|
||||
if (result.valid) return null;
|
||||
return {
|
||||
error: 'Password does not meet security requirements',
|
||||
details: result.errors,
|
||||
score: result.score,
|
||||
feedback: result.feedback,
|
||||
};
|
||||
}
|
||||
|
||||
module.exports = (router) => {
|
||||
|
||||
|
||||
@@ -854,6 +877,9 @@ module.exports = (router) => {
|
||||
// Re-hash so the stored hash matches what the email carries — even if
|
||||
// the admin mistypes vs. what was set at draft creation, the gallery
|
||||
// password the customer receives is the one that actually works.
|
||||
const policyError = await checkGalleryPasswordPolicy(password, event.event_name);
|
||||
if (policyError) return res.status(400).json(policyError);
|
||||
|
||||
publishUpdates.password_hash = await bcrypt.hash(password, getBcryptRounds());
|
||||
}
|
||||
await db('events').where('id', id).update(publishUpdates);
|
||||
|
||||
@@ -224,6 +224,117 @@ async function deleteEventCascade(eventId, adminContext) {
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Collect this event's storage keys BEFORE the transaction removes the
|
||||
// photo rows. Afterwards nothing records which objects belonged to this
|
||||
// event — the DB was the only place that knew, and on an S3/R2 backend the
|
||||
// objects are still sitting in the bucket, unreferenced and billable.
|
||||
//
|
||||
// The filesystem cleanup below (#608) only ever touched local disk: in S3
|
||||
// mode those paths don't exist, `fs.rm` succeeds against nothing, and the
|
||||
// real objects are never touched. Measured on a 403-photo event: bucket
|
||||
// object count unchanged, 679 referenced rows gone.
|
||||
//
|
||||
// A Set because a photo can carry the same key in two columns (an unresized
|
||||
// gallery's hero and preview can resolve to one object) and deleting it
|
||||
// twice would log a spurious failure for the second attempt.
|
||||
const storageKeys = new Set();
|
||||
// Derived keys separately: unlike the originals, whose keys embed the event
|
||||
// slug, these are not event-scoped and need a shared-ownership check below.
|
||||
const derivedKeys = new Set();
|
||||
try {
|
||||
const { resolvePhotoStorageKey } = require('../../services/photoResolver');
|
||||
const photos = await db('photos')
|
||||
.where('event_id', eventId)
|
||||
.select('id', 'path', 'thumbnail_path', 'hero_path', 'preview_path', 'watermark_path', 'source_origin');
|
||||
|
||||
for (const photo of photos) {
|
||||
try {
|
||||
// Returns null for reference/external photos, which live on a mount
|
||||
// outside the managed backend and must NOT be deleted — PicPeak does
|
||||
// not own those bytes.
|
||||
const originalKey = resolvePhotoStorageKey(event, photo);
|
||||
if (originalKey) storageKeys.add(originalKey);
|
||||
} catch (keyErr) {
|
||||
logger.warn('Could not resolve storage key during cascade delete', {
|
||||
eventId, photoId: photo.id, error: keyErr.message
|
||||
});
|
||||
}
|
||||
// Derived tiers are stored as canonical keys and pass through verbatim.
|
||||
// watermark_path included: it is storage-backed on the single-photo
|
||||
// path (watermarkService.deleteWatermarkFile) and leaked here the same
|
||||
// way the originals did.
|
||||
for (const derived of [photo.thumbnail_path, photo.hero_path, photo.preview_path, photo.watermark_path]) {
|
||||
if (derived) {
|
||||
storageKeys.add(derived);
|
||||
derivedKeys.add(derived);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (collectErr) {
|
||||
logger.warn('Could not enumerate stored objects before cascade delete', {
|
||||
eventId, error: collectErr.message
|
||||
});
|
||||
}
|
||||
|
||||
// A canonical derivative can belong to more than one gallery. Its basename
|
||||
// comes from the photo's filename — imageProcessor passes no outputBasename
|
||||
// for managed photos, so the key is `thumbnails/thumb_w300_<filename>` with
|
||||
// nothing event-scoped in it — and filenames are not unique across events.
|
||||
// The responsive-tier code says exactly that, which is why THOSE keys carry
|
||||
// a p{id}_ prefix; the canonical ones predate it. Deleting a shared key here
|
||||
// would blank a surviving gallery's tile until something regenerated it, so
|
||||
// anything another event still points at is left alone. Originals need no
|
||||
// such check: their keys embed the slug.
|
||||
const derived = Array.from(derivedKeys);
|
||||
try {
|
||||
// Chunked: SQLite caps bind variables at 999 and this is four columns wide.
|
||||
for (let i = 0; i < derived.length; i += 200) {
|
||||
const chunk = derived.slice(i, i + 200);
|
||||
const shared = await db('photos')
|
||||
.whereNot('event_id', eventId)
|
||||
.where((qb) => qb
|
||||
.whereIn('thumbnail_path', chunk)
|
||||
.orWhereIn('hero_path', chunk)
|
||||
.orWhereIn('preview_path', chunk)
|
||||
.orWhereIn('watermark_path', chunk))
|
||||
.select('thumbnail_path', 'hero_path', 'preview_path', 'watermark_path');
|
||||
for (const row of shared) {
|
||||
for (const key of [row.thumbnail_path, row.hero_path, row.preview_path, row.watermark_path]) {
|
||||
if (key && derivedKeys.has(key)) storageKeys.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (sharedErr) {
|
||||
// Can't prove ownership — keep the objects. An orphan costs storage; a
|
||||
// deleted derivative costs someone else's gallery.
|
||||
logger.warn('Could not check for shared derivatives; leaving them in place', {
|
||||
eventId, error: sharedErr.message
|
||||
});
|
||||
for (const key of derivedKeys) storageKeys.delete(key);
|
||||
}
|
||||
|
||||
// The archive zip is typically the largest single object an event owns, and
|
||||
// archiveService writes it through the backend (`storage.putFromFile`) — so
|
||||
// the `fs.unlink` below is a no-op on S3 and the zip outlives its event.
|
||||
if (event.archive_path) storageKeys.add(event.archive_path);
|
||||
|
||||
// The pre-built "Download All" zip is the subtle one: it lives UNDER
|
||||
// events/active/{slug}/.download-cache/ (downloadZipService.js:42), so the
|
||||
// recursive fs.rm below covers it on local disk and nothing covers it on
|
||||
// S3, where that prefix is not a directory. It is gallery-sized.
|
||||
// downloadZipService exposes a cleanup() documented as "used on event
|
||||
// deletion" that this cascade never called.
|
||||
// NOTE: an in-flight "Download All" build that started before this delete
|
||||
// can still upload its zip after the sweep and write the path onto a row
|
||||
// that no longer exists, orphaning it. downloadZipService.cleanup() is the
|
||||
// service's cancel primitive, but calling it here made the backend CI job
|
||||
// exceed its 10-minute budget on this branch — its _cleanup() reaches
|
||||
// getStorage() and, in a suite where the S3 backend is configured but
|
||||
// unreachable, every cascade delete then pays the adapter's retry backoff.
|
||||
// Left as a follow-up rather than shipped as a timeout: the race is narrow
|
||||
// and costs one orphaned object, the regression cost the whole suite.
|
||||
if (event.download_zip_path) storageKeys.add(event.download_zip_path);
|
||||
|
||||
await db.transaction(async (trx) => {
|
||||
// 1. Delete activity logs (audit trail)
|
||||
await trx('activity_logs').where('event_id', eventId).del();
|
||||
@@ -284,6 +395,57 @@ async function deleteEventCascade(eventId, adminContext) {
|
||||
}
|
||||
});
|
||||
|
||||
// Managed objects, deleted AFTER the commit: a rolled-back transaction must
|
||||
// never leave files destroyed for an event that still exists. Failures are
|
||||
// logged rather than thrown, matching the philosophy of the filesystem
|
||||
// cleanup above — the database is the source of truth, an orphaned object
|
||||
// is recoverable noise, a half-deleted event is not.
|
||||
if (storageKeys.size > 0) {
|
||||
const { getStorage } = require('../../services/storage');
|
||||
let removed = 0;
|
||||
try {
|
||||
const storage = getStorage();
|
||||
const keys = Array.from(storageKeys);
|
||||
|
||||
// Bounded concurrency rather than one await per key. A 400-photo gallery
|
||||
// owns well over a thousand objects once the derived tiers are counted,
|
||||
// and on S3 that many sequential DeleteObject round trips runs to
|
||||
// minutes — long enough for a proxy to time the request out AFTER the
|
||||
// commit, leaving the event deleted and the sweep half-finished.
|
||||
// Deleting is idempotent and order-independent, so there is nothing to
|
||||
// serialise for.
|
||||
//
|
||||
// A pool, not Promise.all over every key: an unbounded fan-out would
|
||||
// open one socket per object and exhaust the S3 client's connection
|
||||
// pool.
|
||||
const CONCURRENCY = 16;
|
||||
let cursor = 0;
|
||||
const worker = async () => {
|
||||
while (cursor < keys.length) {
|
||||
const key = keys[cursor++];
|
||||
try {
|
||||
await storage.delete(key);
|
||||
removed++;
|
||||
} catch (delErr) {
|
||||
logger.warn('Failed to delete stored object during cascade delete', {
|
||||
eventId, key, error: delErr.message
|
||||
});
|
||||
}
|
||||
}
|
||||
};
|
||||
await Promise.all(
|
||||
Array.from({ length: Math.min(CONCURRENCY, keys.length) }, worker)
|
||||
);
|
||||
} catch (storageErr) {
|
||||
logger.warn('Storage backend unavailable during cascade delete', {
|
||||
eventId, error: storageErr.message
|
||||
});
|
||||
}
|
||||
logger.info('Cascade delete removed stored objects', {
|
||||
eventId, removed, total: storageKeys.size
|
||||
});
|
||||
}
|
||||
|
||||
// Audit trail (outside the transaction so a logging failure can't undo
|
||||
// the actual delete).
|
||||
await logActivity('event_deleted',
|
||||
|
||||
@@ -350,6 +350,7 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
|
||||
let successCount = 0;
|
||||
let missingCount = 0;
|
||||
let errorCount = 0;
|
||||
let skippedCount = 0;
|
||||
let lostClaim = false;
|
||||
|
||||
// Same reasoning as the dimension repair: detached from the request, so
|
||||
@@ -424,11 +425,29 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
|
||||
// large library, and an import or a replacement finishing meanwhile
|
||||
// has already written a date this pass would otherwise overwrite
|
||||
// with the same-or-worse value.
|
||||
//
|
||||
// Fenced on path and filename as well as the id (#1201):
|
||||
// replacePhoto — reachable from the replace_by_name upload path
|
||||
// (adminPhotos.js) — swaps a NEW file under an existing row and
|
||||
// rewrites path/filename. That replacement carries no date of its
|
||||
// own, so captured_at is still NULL and whereNull alone would let
|
||||
// the previous file's EXIF date land on it. Matching the identity
|
||||
// that was actually read means the update affects no rows and the
|
||||
// row is simply skipped.
|
||||
const updated = await db('photos')
|
||||
.where({ id: photo.id })
|
||||
.where({ id: photo.id, path: photo.path, filename: photo.filename })
|
||||
.whereNull('captured_at')
|
||||
.update({ captured_at: captured.toISOString() });
|
||||
if (updated) successCount++;
|
||||
// Counted, not dropped: without this a candidate that was read but
|
||||
// not written falls out of the run's arithmetic entirely, and
|
||||
// success + noExif + failed silently stops adding up to the count
|
||||
// the operator was shown when they started it. Two ways to land
|
||||
// here, both "another writer got there first" — the row was dated
|
||||
// meanwhile (whereNull), or its file changed under us (the fence).
|
||||
// Neither is an error and neither needs a retry: captured_at is
|
||||
// still NULL for the fenced case, so the status endpoint keeps
|
||||
// reporting it as backlog and the next run picks it up.
|
||||
if (updated) successCount++; else skippedCount++;
|
||||
|
||||
if (successCount % 50 === 0 && successCount > 0) {
|
||||
logger.info(`Capture date backfill progress: ${successCount} updated...`);
|
||||
@@ -443,12 +462,15 @@ router.post('/repair-capture-dates', adminAuth, requirePermission('settings.edit
|
||||
logger.warn(`Capture date backfill stopped: claim taken over after ${successCount} updated, ${errorCount} errors`);
|
||||
return;
|
||||
}
|
||||
await maintenanceJobs.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount });
|
||||
logger.info(`Capture date backfill complete: ${successCount} updated, ${missingCount} without EXIF, ${errorCount} errors`);
|
||||
await maintenanceJobs.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, skipped: skippedCount });
|
||||
logger.info(
|
||||
`Capture date backfill complete: ${successCount} updated, ${missingCount} without EXIF, `
|
||||
+ `${errorCount} errors, ${skippedCount} skipped (dated or replaced mid-run)`
|
||||
);
|
||||
} catch (err) {
|
||||
logger.error('Capture date backfill aborted:', err);
|
||||
await maintenanceJobs
|
||||
.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, error: err.message })
|
||||
.release(JOB_CAPTURE_DATE_BACKFILL, token, { success: successCount, noExif: missingCount, failed: errorCount, skipped: skippedCount, error: err.message })
|
||||
.catch(() => {});
|
||||
} finally {
|
||||
lease.stop();
|
||||
|
||||
+244
-23
@@ -30,7 +30,7 @@ const { generateGuestIdentifier } = require('../middleware/feedbackRateLimit');
|
||||
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 { getEventShareToken, resolveShareIdentifier, buildShareLinkVariants } = require('../services/shareLinkService');
|
||||
const { handleAsync, errorResponse } = require('../utils/routeHelpers');
|
||||
const { NotFoundError } = require('../utils/errors');
|
||||
@@ -54,6 +54,43 @@ 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) };
|
||||
}
|
||||
|
||||
|
||||
// Check for slug redirect (for renamed events)
|
||||
async function checkSlugRedirect(slug) {
|
||||
try {
|
||||
@@ -996,6 +1033,47 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
|
||||
}
|
||||
}
|
||||
|
||||
// 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 headWmSettings = await watermarkService.getWatermarkSettings();
|
||||
const headWmEnabled = !!(headWmSettings && headWmSettings.enabled)
|
||||
|| req.event.watermark_downloads === true
|
||||
|| req.event.watermark_downloads === 1;
|
||||
if (!headWmEnabled) {
|
||||
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();
|
||||
}
|
||||
|
||||
// Update download count
|
||||
await db('photos').where('id', photoId).increment('download_count', 1);
|
||||
|
||||
@@ -1008,11 +1086,19 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
|
||||
photo_id: photoId
|
||||
});
|
||||
|
||||
let filePath;
|
||||
// Where the bytes actually live. Managed photos sit 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 fail in S3 mode, 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 filesystem path.
|
||||
let storageKey = null;
|
||||
try {
|
||||
filePath = resolvePhotoFilePath(req.event, photo);
|
||||
storageKey = resolvePhotoStorageKey(req.event, photo);
|
||||
} catch (resolveError) {
|
||||
logger.error('Failed to resolve photo path for download', {
|
||||
logger.error('Failed to resolve photo storage key for download', {
|
||||
slug: req.params.slug,
|
||||
photoId,
|
||||
eventId: req.event.id,
|
||||
@@ -1020,7 +1106,7 @@ 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;
|
||||
@@ -1041,7 +1127,39 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
|
||||
enabled: true,
|
||||
text: req.event.watermark_text || watermarkSettings?.text || 'Protected'
|
||||
};
|
||||
const watermarkedBuffer = await watermarkService.applyWatermark(filePath, effectiveSettings);
|
||||
|
||||
// applyWatermark takes a PATH, and caches on it — buffer inputs skip the
|
||||
// cache deliberately. In S3 mode materialize a tmp local copy and hand
|
||||
// it the copy's path, exactly as the zip builders below do, so the cache
|
||||
// still applies and the full-size original isn't re-processed per
|
||||
// download.
|
||||
let watermarkedBuffer;
|
||||
try {
|
||||
watermarkedBuffer = storageKey
|
||||
? await withLocalCopy(storageKey, (localPath) =>
|
||||
watermarkService.applyWatermark(localPath, effectiveSettings))
|
||||
: await watermarkService.applyWatermark(
|
||||
resolvePhotoFilePath(req.event, photo), effectiveSettings);
|
||||
} catch (watermarkError) {
|
||||
// Classify, the same way the pass-through branch below does. This can
|
||||
// fail 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 = watermarkError.code === 'ENOENT'
|
||||
|| watermarkError.name === 'NoSuchKey'
|
||||
|| watermarkError.name === 'NotFound'
|
||||
|| watermarkError.$metadata?.httpStatusCode === 404;
|
||||
logger.error('Failed to watermark photo for download', {
|
||||
slug: req.params.slug,
|
||||
photoId,
|
||||
eventId: req.event.id,
|
||||
error: watermarkError.message,
|
||||
});
|
||||
return gone
|
||||
? res.status(404).json({ error: 'Photo file not found' })
|
||||
: res.status(500).json({ error: 'Failed to download photo' });
|
||||
}
|
||||
|
||||
res.set({
|
||||
'Content-Type': photo.mime_type || 'image/jpeg',
|
||||
@@ -1049,27 +1167,130 @@ router.get('/:slug/download/:photoId', verifyGalleryAccess, denySlideshowToken,
|
||||
'Content-Length': watermarkedBuffer.length
|
||||
});
|
||||
|
||||
res.send(watermarkedBuffer);
|
||||
} 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({
|
||||
return res.send(watermarkedBuffer);
|
||||
}
|
||||
|
||||
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 the parts that matter for a download are reproduced: 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.
|
||||
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}`,
|
||||
});
|
||||
res.sendFile(filePath, (downloadError) => {
|
||||
if (downloadError) {
|
||||
logger.error('Error streaming gallery download', {
|
||||
slug: req.params.slug,
|
||||
photoId,
|
||||
eventId: req.event.id,
|
||||
error: downloadError.message,
|
||||
});
|
||||
}
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
let filePath;
|
||||
try {
|
||||
filePath = resolvePhotoFilePath(req.event, photo);
|
||||
} catch (resolveError) {
|
||||
logger.error('Failed to resolve photo path for download', {
|
||||
slug: req.params.slug,
|
||||
photoId,
|
||||
eventId: req.event.id,
|
||||
error: resolveError.message,
|
||||
});
|
||||
return res.status(404).json({ error: 'Photo file not found' });
|
||||
}
|
||||
|
||||
// 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');
|
||||
}
|
||||
|
||||
@@ -94,11 +94,14 @@ function validatePassword(password, options = {}) {
|
||||
// Check minimum strength score
|
||||
if (strength.score < config.minStrengthScore) {
|
||||
errors.push('Password is too weak. Please choose a stronger password');
|
||||
}
|
||||
|
||||
// Add zxcvbn suggestions
|
||||
if (strength.feedback.suggestions.length > 0) {
|
||||
errors.push(...strength.feedback.suggestions);
|
||||
// Surface zxcvbn's suggestions only alongside a real failure — they are
|
||||
// advice, not requirements. A password that meets the configured policy
|
||||
// must not be rejected just because zxcvbn has ideas for improving it
|
||||
// (e.g. "Natasha2023" scores exactly minStrengthScore but always carries
|
||||
// an "add another word" suggestion, which used to fail it).
|
||||
if (strength.feedback.suggestions.length > 0) {
|
||||
errors.push(...strength.feedback.suggestions);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "picpeak-frontend",
|
||||
"private": true,
|
||||
"version": "3.46.5",
|
||||
"version": "3.46.8",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "vite",
|
||||
|
||||
@@ -63,7 +63,12 @@ export const PhotoFilters: React.FC<PhotoFiltersProps> = ({
|
||||
className="px-3 py-2 border border-neutral-300 dark:border-neutral-600 rounded-lg bg-white dark:bg-neutral-800 text-neutral-900 dark:text-neutral-100 focus:ring-2 focus:ring-primary-500 focus:border-accent-dark"
|
||||
>
|
||||
<option value="">{t('gallery.allCategories', 'All Categories')}</option>
|
||||
<option value="0">{t('gallery.uncategorized', 'Uncategorized')}</option>
|
||||
{/* The literal the backend understands, not 0 (#1211). It skips
|
||||
'0' outright — `category_id !== '0'` — so this filter used to
|
||||
apply no condition at all and quietly returned the whole event.
|
||||
The onChange below passes non-numeric values through unchanged,
|
||||
so the string arrives intact. */}
|
||||
<option value="uncategorized">{t('gallery.uncategorized', 'Uncategorized')}</option>
|
||||
{categories.map(cat => (
|
||||
<option key={cat.id} value={cat.id}>
|
||||
{cat.name}
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* The category filter's wire values (#1211).
|
||||
*
|
||||
* "Uncategorized" was rendered as `value="0"`, and the backend skips `'0'`
|
||||
* outright (`adminPhotos.js`: `category_id !== '0'`), so the filter applied no
|
||||
* condition and returned the whole event. The value it does understand is the
|
||||
* literal `uncategorized`, four lines below that guard, which nothing sent.
|
||||
*
|
||||
* The failure was silent — a full list reads as "nothing to narrow" rather
|
||||
* than "the filter did not run" — so this pins the wire value rather than the
|
||||
* rendered label. Reported in #1209 by someone trying to isolate a few
|
||||
* thousand uncategorised imports to re-assign them.
|
||||
*/
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { PhotoFilters } from '../PhotoFilters';
|
||||
|
||||
vi.mock('react-i18next', async () => {
|
||||
const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
|
||||
return {
|
||||
...actual,
|
||||
useTranslation: () => ({
|
||||
t: (_key: string, fallback?: any) => (typeof fallback === 'string' ? fallback : _key),
|
||||
i18n: { language: 'en' }
|
||||
})
|
||||
};
|
||||
});
|
||||
|
||||
const categories = [
|
||||
{ id: 3, name: 'Ceremony' },
|
||||
{ id: 4, name: 'Reception' },
|
||||
];
|
||||
|
||||
const renderFilters = (selectedCategory: number | string | null = null) => {
|
||||
const onCategoryChange = vi.fn();
|
||||
render(
|
||||
<PhotoFilters
|
||||
selectedCategory={selectedCategory}
|
||||
categories={categories as any}
|
||||
onCategoryChange={onCategoryChange}
|
||||
searchTerm=""
|
||||
onSearchChange={vi.fn()}
|
||||
/>
|
||||
);
|
||||
return { onCategoryChange };
|
||||
};
|
||||
|
||||
const categorySelect = () => screen.getAllByRole('combobox')[0];
|
||||
|
||||
describe('category filter wire values (#1211)', () => {
|
||||
it('sends the literal the backend understands for Uncategorized', async () => {
|
||||
const { onCategoryChange } = renderFilters();
|
||||
|
||||
await userEvent.selectOptions(categorySelect(), 'uncategorized');
|
||||
|
||||
// Not 0 — the backend drops that and returns everything.
|
||||
expect(onCategoryChange).toHaveBeenCalledWith('uncategorized');
|
||||
});
|
||||
|
||||
it('still sends a numeric id for a real category', async () => {
|
||||
const { onCategoryChange } = renderFilters();
|
||||
|
||||
await userEvent.selectOptions(categorySelect(), '3');
|
||||
|
||||
expect(onCategoryChange).toHaveBeenCalledWith(3);
|
||||
});
|
||||
|
||||
it('clears back to null for All Categories', async () => {
|
||||
const { onCategoryChange } = renderFilters('uncategorized');
|
||||
|
||||
await userEvent.selectOptions(categorySelect(), '');
|
||||
|
||||
expect(onCategoryChange).toHaveBeenCalledWith(null);
|
||||
});
|
||||
|
||||
it('keeps Uncategorized selected once it is chosen', () => {
|
||||
renderFilters('uncategorized');
|
||||
// The select is controlled; if the option value and the state value ever
|
||||
// drift apart the control silently falls back to the first option.
|
||||
expect((categorySelect() as HTMLSelectElement).value).toBe('uncategorized');
|
||||
});
|
||||
});
|
||||
@@ -5,7 +5,7 @@ import { toast } from 'react-toastify';
|
||||
import { Button } from '../common';
|
||||
import { api } from '../../config/api';
|
||||
import { usePublicSettings } from '../../hooks/usePublicSettings';
|
||||
import { extensionsToMimeTypes, extensionsToAcceptString } from '../../utils/fileTypes';
|
||||
import { extensionsToMimeTypes, buildUploadAcceptString } from '../../utils/fileTypes';
|
||||
|
||||
interface UserPhotoUploadProps {
|
||||
eventId: number;
|
||||
@@ -48,8 +48,10 @@ export const UserPhotoUpload: React.FC<UserPhotoUploadProps> = ({
|
||||
[publicSettings?.allowed_file_types]
|
||||
);
|
||||
|
||||
// #1117 — on Android this appends a type the photo picker can't handle, so
|
||||
// the system falls back to the chooser that actually offers the camera.
|
||||
const acceptString = useMemo(
|
||||
() => extensionsToAcceptString(publicSettings?.allowed_file_types),
|
||||
() => buildUploadAcceptString(publicSettings?.allowed_file_types),
|
||||
[publicSettings?.allowed_file_types]
|
||||
);
|
||||
|
||||
|
||||
@@ -689,6 +689,23 @@ export const StatusTab: React.FC<StatusTabProps> = ({
|
||||
failed: captureDateStatus.lastResult.failed,
|
||||
defaultValue: 'Last run: {{success}} updated, {{noExif}} with no date found, {{failed}} unreachable',
|
||||
})}
|
||||
{/* Only when it happened. Without it the three numbers above
|
||||
silently stop adding up to the count the run started with: a
|
||||
photo that was replaced, renamed or dated by someone else
|
||||
mid-run is read but not written.
|
||||
Deliberately says "not updated" and not "will be retried":
|
||||
one of the two ways to land here is another writer having
|
||||
filled captured_at, and that photo is finished, not backlog.
|
||||
The Missing Capture Date figure above is what says whether
|
||||
anything is actually left to do. */}
|
||||
{Number(captureDateStatus.lastResult.skipped) > 0 && (
|
||||
<span className="block text-amber-600 dark:text-amber-400 mt-1">
|
||||
{t('settings.captureDates.skipped', {
|
||||
count: captureDateStatus.lastResult.skipped,
|
||||
defaultValue: '{{count}} photo(s) were changed by something else while the run was reading them and were not updated.',
|
||||
})}
|
||||
</span>
|
||||
)}
|
||||
</p>
|
||||
)}
|
||||
|
||||
|
||||
@@ -2077,6 +2077,7 @@
|
||||
"running": "Wird nachgetragen...",
|
||||
"noneToFill": "Alle Fotos haben bereits ein Aufnahmedatum",
|
||||
"resultSuccess": "Letzter Lauf: {{success}} aktualisiert, {{noExif}} ohne gefundenes Datum, {{failed}} nicht erreichbar",
|
||||
"skipped": "{{count}} Foto(s) wurden während des Laufs anderweitig geändert und daher nicht aktualisiert.",
|
||||
"description": "Trägt „Aufnahmedatum\" aus den EXIF-Daten nach, für Fotos die vor dieser Auswertung importiert wurden. Externe Importe haben nie eines gespeichert, dadurch sortieren diese Galerien nach Importreihenfolge statt nach Aufnahmezeit."
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1618,6 +1618,7 @@
|
||||
"running": "Backfilling...",
|
||||
"noneToFill": "All photos already have a capture date",
|
||||
"resultSuccess": "Last run: {{success}} updated, {{noExif}} with no date found, {{failed}} unreachable",
|
||||
"skipped": "{{count}} photo(s) were changed by something else while the run was reading them and were not updated.",
|
||||
"description": "Backfill \"Date Taken\" from EXIF for photos imported before capture dates were read. External/reference imports never recorded one, so their galleries sort by import order instead of when the photos were taken."
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1181,6 +1181,7 @@
|
||||
"running": "Traitement...",
|
||||
"noneToFill": "Toutes les photos ont déjà une date de prise de vue",
|
||||
"resultSuccess": "Dernier passage : {{success}} mises à jour, {{noExif}} sans date trouvée, {{failed}} inaccessibles",
|
||||
"skipped": "{{count}} photo(s) ont été modifiées par autre chose pendant le passage et n'ont donc pas été mises à jour.",
|
||||
"description": "Complète la « date de prise de vue » depuis les EXIF pour les photos importées avant sa lecture. Les imports externes n'en enregistraient aucune, si bien que ces galeries se trient par ordre d'import plutôt que par date de prise de vue."
|
||||
}
|
||||
},
|
||||
|
||||
@@ -1181,6 +1181,7 @@
|
||||
"running": "Dopolnjevanje...",
|
||||
"noneToFill": "Vse fotografije že imajo datum zajema",
|
||||
"resultSuccess": "Zadnji zagon: {{success}} posodobljenih, {{noExif}} brez najdenega datuma, {{failed}} nedosegljivih",
|
||||
"skipped": "{{count}} fotografij je bilo med zagonom spremenjenih drugje in zato niso bile posodobljene.",
|
||||
"description": "Dopolni »datum zajema« iz EXIF za fotografije, uvožene pred njegovim branjem. Zunanji uvozi ga niso zabeležili, zato se te galerije razvrščajo po vrstnem redu uvoza namesto po času zajema."
|
||||
}
|
||||
},
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { buildUploadAcceptString } from '../fileTypes';
|
||||
|
||||
describe('buildUploadAcceptString (#1117)', () => {
|
||||
const ANDROID = 'Mozilla/5.0 (Linux; Android 16; Pixel 9) AppleWebKit/537.36 Chrome/151.0.0.0 Mobile Safari/537.36';
|
||||
const IOS = 'Mozilla/5.0 (iPhone; CPU iPhone OS 26_6 like Mac OS X) AppleWebKit/605.1.15 Version/26.0 Mobile Safari/604.1';
|
||||
const DESKTOP = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 Chrome/151.0.0.0 Safari/537.36';
|
||||
const FIREFOX_ANDROID = 'Mozilla/5.0 (Android 16; Mobile; rv:140.0) Gecko/140.0 Firefox/140.0';
|
||||
|
||||
it('appends the camera token on Android so the chooser offers the camera', () => {
|
||||
expect(buildUploadAcceptString('jpg,png', ANDROID)).toBe('image/jpeg,image/png,android/allowCamera');
|
||||
});
|
||||
|
||||
it('adds nothing a guest could actually select', () => {
|
||||
// The token exists to flip Chrome out of the photo picker, not to widen
|
||||
// the allowlist. An earlier revision used .pdf, which does flip it but
|
||||
// also offers PDFs — pick one and you get "Invalid file type".
|
||||
const accept = buildUploadAcceptString('jpg,png', ANDROID);
|
||||
expect(accept).not.toMatch(/\.pdf|application\/pdf/);
|
||||
expect(accept.split(',').filter((t) => t.startsWith('image/') || t.startsWith('video/')))
|
||||
.toEqual(['image/jpeg', 'image/png']);
|
||||
});
|
||||
|
||||
|
||||
it('leaves Firefox for Android alone — its chooser already offers the camera', () => {
|
||||
// The UA says Android, but the behaviour this works around is Chromium's.
|
||||
expect(buildUploadAcceptString('jpg,png', FIREFOX_ANDROID)).toBe('image/jpeg,image/png');
|
||||
});
|
||||
it('leaves iOS and desktop untouched — their pickers already work', () => {
|
||||
expect(buildUploadAcceptString('jpg,png', IOS)).toBe('image/jpeg,image/png');
|
||||
expect(buildUploadAcceptString('jpg,png', DESKTOP)).toBe('image/jpeg,image/png');
|
||||
});
|
||||
|
||||
it('keeps offering video when the admin configured it', () => {
|
||||
// The workaround must not narrow the accept list to images: an install
|
||||
// with video enabled still has to offer mp4/mov in the chooser.
|
||||
expect(buildUploadAcceptString('jpg,mp4,mov', ANDROID)).toBe('image/jpeg,video/mp4,video/quicktime,android/allowCamera');
|
||||
});
|
||||
|
||||
it('falls back to the configured default set, not a wider image/*', () => {
|
||||
expect(buildUploadAcceptString('', DESKTOP)).toBe('image/jpeg,image/png,image/webp');
|
||||
expect(buildUploadAcceptString('', ANDROID)).toBe('image/jpeg,image/png,image/webp,android/allowCamera');
|
||||
});
|
||||
});
|
||||
@@ -46,3 +46,40 @@ export function extensionsToMimeTypes(extString?: string | null): string[] {
|
||||
export function extensionsToAcceptString(extString?: string | null): string {
|
||||
return extensionsToMimeTypes(extString).join(',');
|
||||
}
|
||||
|
||||
/**
|
||||
* `accept` for the guest upload input (#1117).
|
||||
*
|
||||
* Chrome and Edge on Android 14/15 route an `<input>` whose accept list is
|
||||
* entirely image and video types to the system *photo picker*, which has no
|
||||
* camera tile — so a guest standing at the event can only pick a photo already
|
||||
* in their gallery, never take one. Adding a value that picker cannot satisfy
|
||||
* makes Chrome fall back to the general document chooser, which does offer the
|
||||
* camera.
|
||||
*
|
||||
* `android/allowCamera` is the token the workaround converged on. It is not a
|
||||
* real MIME type and matches no file, which is the point: it flips the picker
|
||||
* without advertising anything extra as selectable. An earlier revision used
|
||||
* `.pdf`, which works by the same mechanism but offers PDFs in the chooser —
|
||||
* pick one and you get "Invalid file type" for your trouble.
|
||||
*
|
||||
* Gated to Android MINUS Firefox. The behaviour is Chromium's — Chrome and
|
||||
* Edge on Android 14/15 — and Firefox for Android, whose UA also says
|
||||
* `Android`, opens a chooser that already offers the camera. Handing it a
|
||||
* token invented to reroute a picker it does not use is at best inert and at
|
||||
* worst changes a chooser that was working.
|
||||
*
|
||||
* UA sniffing is the wrong tool in general, but there is no feature query for
|
||||
* "which picker will this open", and the failure mode of a wrong guess is an
|
||||
* accept token the browser ignores.
|
||||
*
|
||||
* Neither token widens what is actually accepted: `addFiles` validates every
|
||||
* file against `extensionsToMimeTypes`, which only ever emits types it has a
|
||||
* mapping for, so nothing new can get past it.
|
||||
*/
|
||||
export function buildUploadAcceptString(extString?: string | null, userAgent?: string): string {
|
||||
const accept = extensionsToAcceptString(extString);
|
||||
const ua = userAgent ?? (typeof navigator !== 'undefined' ? navigator.userAgent : '');
|
||||
const needsCameraToken = /Android/i.test(ua) && !/Firefox/i.test(ua);
|
||||
return needsCameraToken ? `${accept},android/allowCamera` : accept;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user