fix(gallery): keep videos playable under enhanced and maximum protection (stable) (#1408)

Stable twin of the main-branch fix.

Once an event left `standard` protection, both halves of the video path were
routed through /api/secure-images, and neither half can carry a video: the
lightbox drops the emitted `{{token}}` template straight into a <video>
element and nothing substitutes the placeholder, while the secure-images route
pipes every byte through sharp, which throws on an mp4. The /photo/:photoId
route bounced to that same endpoint before reaching its own video branch —
isVideo was computed and then ignored — so there was no way through.

Videos now keep the JWT route at every protection level, on both sides. Not a
new exposure: thumbnails of those same videos have always been served from it,
and a valid gallery token is still required. Still images are unaffected and
keep bouncing to the secure endpoint.

VideoPlayer had no `error` listener, so all of this rendered as a poster
frozen at "0:00 / 0:00" behind a play button that did nothing —
indistinguishable from a codec the browser cannot decode, which is the other
common cause (HEVC/H.265 phone footage plays in Safari and nowhere else). It
now surfaces the failure and names the codec case.

The frontend half is identical to main; the backend half is hand-ported
because stable keeps these routes in the monolithic routes/gallery.js.

Relates to issue 1370

Co-authored-by: Paul Nothaft <paul@MacStudio-von-Paul.local>
This commit is contained in:
Paul Nothaft
2026-09-11 11:25:24 +02:00
committed by GitHub
parent b869de33a5
commit 0732a160b5
12 changed files with 326 additions and 14 deletions
@@ -0,0 +1,163 @@
/**
* Videos under enhanced/maximum image protection (#1370).
*
* Both halves of the video path used to be routed through /api/secure-images
* once an event left `standard` protection, and neither half could carry a
* video:
*
* 1. galleryQueryService emitted `/api/secure-images/{slug}/secure/{id}/{{token}}`
* as the video's `url`. The lightbox drops that straight into a <video>
* element, nothing substitutes `{{token}}` (the helper that could is
* unreferenced), and the route answers 403 "Invalid or expired token".
* 2. Even with a valid token it would still fail: the secure-images route
* pipes every byte through secureImageService.processProtectedImage,
* which calls sharp() and throws on an mp4 → 404.
*
* The guest saw a poster frozen at 0:00 with no error of any kind.
*
* Videos now keep the JWT route at every protection level. That is not a new
* exposure — thumbnails of those same videos have always been served from it —
* so these tests also pin the inverse: still images must keep bouncing to the
* secure endpoint. Every assertion here fails on the unfixed code except the
* two guarding images.
*/
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-video-urls-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'video-urls-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-video-urls-storage-'));
const request = require('supertest');
const express = require('express');
const cookieParser = require('cookie-parser');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'protected-video-gallery';
const VIDEO_BYTES = Buffer.from('not really an mp4, but the route only streams bytes');
describe('videos stay playable under enhanced/maximum protection (#1370)', () => {
let db; let cleanup; let app; let eventId; let videoId; let imageId;
async function setProtection(level) {
await db('events').where('id', eventId).update({ protection_level: level });
}
async function photoPayload(id) {
const res = await request(app).get(`/api/gallery/${SLUG}/photos`);
expect(res.status).toBe(200);
const photo = res.body.photos.find((p) => p.id === id);
expect(photo).toBeDefined();
return photo;
}
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const ev = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Protected Video',
event_date: '2026-09-01',
host_email: 'h@example.com',
admin_email: 'a@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/s`,
share_token: 'protected-video-share',
expires_at: new Date(Date.now() + 7 * 864e5).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
// Password-free so verifyGalleryAccess takes the public path, same as
// the sibling gallery suites.
require_password: 0,
created_at: new Date().toISOString(),
}).returning('id');
eventId = ev[0]?.id ?? ev[0];
const mediaDir = path.join(process.env.STORAGE_PATH, 'events/active', SLUG, 'individual');
fs.mkdirSync(mediaDir, { recursive: true });
fs.writeFileSync(path.join(mediaDir, 'clip.mp4'), VIDEO_BYTES);
fs.writeFileSync(path.join(mediaDir, 'still.jpg'), Buffer.from('jpeg-ish'));
const vid = await db('photos').insert({
event_id: eventId,
filename: 'clip.mp4',
path: `${SLUG}/individual/clip.mp4`,
type: 'individual',
media_type: 'video',
mime_type: 'video/mp4',
duration: 43,
uploaded_at: new Date().toISOString(),
}).returning('id');
videoId = vid[0]?.id ?? vid[0];
const img = await db('photos').insert({
event_id: eventId,
filename: 'still.jpg',
path: `${SLUG}/individual/still.jpg`,
type: 'individual',
uploaded_at: new Date().toISOString(),
}).returning('id');
imageId = img[0]?.id ?? img[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(); });
describe.each(['enhanced', 'maximum'])('protection_level = %s', (level) => {
beforeAll(async () => { await setProtection(level); });
test('the video url is the JWT route, not a {{token}} template', async () => {
const photo = await photoPayload(videoId);
expect(photo.url).toBe(`/api/gallery/${SLUG}/photo/${videoId}`);
expect(photo.url).not.toContain('{{token}}');
expect(photo.requires_token).toBe(false);
});
test('the video streams instead of bouncing to the secure endpoint', async () => {
const res = await request(app).get(`/api/gallery/${SLUG}/photo/${videoId}`);
expect(res.status).toBe(200);
expect(res.headers['content-type']).toBe('video/mp4');
expect(res.headers['accept-ranges']).toBe('bytes');
expect(Buffer.from(res.body)).toEqual(VIDEO_BYTES);
});
test('range requests still work, so seeking is possible', async () => {
const res = await request(app)
.get(`/api/gallery/${SLUG}/photo/${videoId}`)
.set('Range', 'bytes=0-9');
expect(res.status).toBe(206);
expect(res.headers['content-range']).toBe(`bytes 0-9/${VIDEO_BYTES.length}`);
});
test('still images keep bouncing to the secure endpoint', async () => {
const photo = await photoPayload(imageId);
expect(photo.url).toBe(`/api/secure-images/${SLUG}/secure/${imageId}/{{token}}`);
expect(photo.requires_token).toBe(true);
const res = await request(app).get(`/api/gallery/${SLUG}/photo/${imageId}`);
expect(res.status).toBe(302);
expect(res.body.error).toBe('Secure access required');
});
});
describe('protection_level = standard', () => {
beforeAll(async () => { await setProtection('standard'); });
test('both media types take the JWT route, as before', async () => {
expect((await photoPayload(videoId)).url).toBe(`/api/gallery/${SLUG}/photo/${videoId}`);
expect((await photoPayload(imageId)).url).toBe(`/api/gallery/${SLUG}/photo/${imageId}`);
});
});
});
+19 -2
View File
@@ -849,7 +849,17 @@ router.get('/:slug/photos', verifyGalleryAccess, resolveGuest, async (req, res)
},
categories: categories,
photos: photos.map(photo => {
const useJwtUrl = (protectionSettings.protection_level === 'basic' || protectionSettings.protection_level === 'standard');
// Videos always take the JWT route (#1370). The secure-images template
// below can never serve one — the route runs the bytes through sharp,
// which throws on an mp4 — and nothing substitutes the {{token}}
// placeholder for the <video> element either, so under enhanced/maximum
// a video resolved to a 403 and the lightbox sat at 0:00. The matching
// exemption is on the /photo/:photoId route below.
const isVideo = photo.media_type === 'video'
|| (photo.mime_type && photo.mime_type.startsWith('video/'));
const useJwtUrl = isVideo
|| protectionSettings.protection_level === 'basic'
|| protectionSettings.protection_level === 'standard';
// Add watermark version to URLs for cache busting when settings change
const wmQuery = wmVersion ? `?${wmVersion}` : '';
const photoUrl = useJwtUrl ?
@@ -1734,7 +1744,14 @@ router.get('/:slug/photo/:photoId',
// Check protection level - basic and standard protection allow direct JWT access
const protectionLevel = req.event.protection_level || 'standard';
if (protectionLevel === 'enhanced' || protectionLevel === 'maximum') {
// Videos are exempt (#1370). The secure-images endpoint this bounces to
// pipes every byte through sharp (secureImageService.processProtectedImage),
// which throws on an mp4 — so under enhanced/maximum a video was
// unservable by either route, and the lightbox showed a poster stuck at
// 0:00. Serving it here instead is not a new exposure: thumbnails of the
// same videos already come from this route at every protection level, and
// the guest still needs a valid gallery token to get here at all.
if (!isVideo && (protectionLevel === 'enhanced' || protectionLevel === 'maximum')) {
// For enhanced/maximum protection, redirect to secure endpoint
return res.status(302).json({
error: 'Secure access required',
@@ -1,5 +1,6 @@
import React, { useRef, useState, useEffect } from 'react';
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize } from 'lucide-react';
import { useTranslation } from 'react-i18next';
import { Play, Pause, Volume2, VolumeX, Maximize, Minimize, AlertTriangle } from 'lucide-react';
interface VideoPlayerProps {
src: string;
@@ -24,8 +25,10 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
width = '100%',
height = 'auto'
}) => {
const { t } = useTranslation();
const videoRef = useRef<HTMLVideoElement>(null);
const [isPlaying, setIsPlaying] = useState(false);
const [loadError, setLoadError] = useState<string | null>(null);
const [isMuted, setIsMuted] = useState(muted);
const [isFullscreen, setIsFullscreen] = useState(false);
const [progress, setProgress] = useState(0);
@@ -51,11 +54,28 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
const handlePause = () => setIsPlaying(false);
const handleEnded = () => setIsPlaying(false);
// Without this the element just sits on its poster at 0:00 and says
// nothing (#1370) — a guest cannot tell a failed request from a codec
// their browser will not decode, and neither could we from their report.
// MEDIA_ERR_SRC_NOT_SUPPORTED is the one worth naming: it is what an
// HEVC/H.265 phone recording does everywhere except Safari, and the
// photographer's answer is to download the file rather than retry.
const handleError = () => {
const code = video.error?.code;
setLoadError(
code === MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED
? t('gallery.videoFormatUnsupported', 'This video format cannot be played in this browser. Download it to watch it.')
: t('gallery.videoLoadFailed', 'This video could not be loaded.')
);
setIsPlaying(false);
};
video.addEventListener('timeupdate', handleTimeUpdate);
video.addEventListener('loadedmetadata', handleLoadedMetadata);
video.addEventListener('play', handlePlay);
video.addEventListener('pause', handlePause);
video.addEventListener('ended', handleEnded);
video.addEventListener('error', handleError);
return () => {
video.removeEventListener('timeupdate', handleTimeUpdate);
@@ -63,8 +83,15 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
video.removeEventListener('play', handlePlay);
video.removeEventListener('pause', handlePause);
video.removeEventListener('ended', handleEnded);
video.removeEventListener('error', handleError);
};
}, []);
}, [t]);
// Arrowing to the next video in the lightbox reuses this element, so a
// stale error would otherwise stick to a clip that loads fine.
useEffect(() => {
setLoadError(null);
}, [src]);
const togglePlayPause = () => {
const video = videoRef.current;
@@ -161,7 +188,16 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
onClick={togglePlayPause}
/>
{controls && (
{loadError && (
<div className="absolute inset-0 flex items-center justify-center bg-black/70 p-6 text-center">
<div className="flex flex-col items-center gap-2 text-white">
<AlertTriangle size={28} />
<span className="text-sm max-w-xs">{loadError}</span>
</div>
</div>
)}
{controls && !loadError && (
<div
className={`absolute bottom-0 left-0 right-0 bg-gradient-to-t from-black/80 to-transparent p-4 transition-opacity duration-300 ${
showControls ? 'opacity-100' : 'opacity-0'
@@ -214,7 +250,7 @@ export const VideoPlayer: React.FC<VideoPlayerProps> = ({
)}
{/* Play button overlay when paused */}
{!isPlaying && showControls && (
{!isPlaying && showControls && !loadError && (
<div className="absolute inset-0 flex items-center justify-center">
<button
onClick={togglePlayPause}
@@ -0,0 +1,80 @@
/**
* A video that fails to load has to say so (#1370).
*
* The element carried no `error` listener, so every failure — an unplayable
* codec, a 403, a missing file — rendered identically: the poster frame, the
* transport stuck at "0:00 / 0:00", and a play button that did nothing. The
* reporter could not tell us which one they had hit, and neither could we.
*
* MEDIA_ERR_SRC_NOT_SUPPORTED gets its own wording because it is by far the
* most common cause in practice (HEVC/H.265 phone footage plays in Safari and
* nowhere else) and the useful advice for it — download the file — is not the
* advice for a transport failure.
*/
import { describe, it, expect, beforeAll } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { VideoPlayer } from '../VideoPlayer';
function failWith(video: HTMLVideoElement, code: number) {
Object.defineProperty(video, 'error', { value: { code }, configurable: true });
fireEvent.error(video);
}
describe('VideoPlayer load errors (#1370)', () => {
beforeAll(() => {
// jsdom implements HTMLMediaElement but not the MediaError constants.
if (typeof MediaError === 'undefined') {
(globalThis as unknown as { MediaError: unknown }).MediaError = {
MEDIA_ERR_ABORTED: 1,
MEDIA_ERR_NETWORK: 2,
MEDIA_ERR_DECODE: 3,
MEDIA_ERR_SRC_NOT_SUPPORTED: 4,
};
}
});
it('shows the transport controls while nothing has gone wrong', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
expect(container.querySelector('video')).toBeInTheDocument();
// Two: the transport button and the centre overlay.
expect(screen.getAllByLabelText('Play')).toHaveLength(2);
expect(screen.getByText('0:00 / 0:00')).toBeInTheDocument();
});
it('names the codec case and points at the download', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.getByText(/cannot be played in this browser/i)).toBeInTheDocument();
expect(screen.getByText(/Download it/i)).toBeInTheDocument();
});
it('reports a transport failure without blaming the format', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_NETWORK);
expect(screen.getByText(/could not be loaded/i)).toBeInTheDocument();
expect(screen.queryByText(/cannot be played in this browser/i)).not.toBeInTheDocument();
});
it('hides the 0:00 transport, which only ever misled', () => {
const { container } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.queryByText('0:00 / 0:00')).not.toBeInTheDocument();
// Including the centre overlay, which was the dead play button in the
// screenshots on the issue.
expect(screen.queryAllByLabelText('Play')).toHaveLength(0);
});
it('clears the error when the lightbox arrows to the next video', () => {
const { container, rerender } = render(<VideoPlayer src="/api/gallery/e/photo/1" />);
failWith(container.querySelector('video')!, MediaError.MEDIA_ERR_SRC_NOT_SUPPORTED);
expect(screen.getByText(/cannot be played in this browser/i)).toBeInTheDocument();
rerender(<VideoPlayer src="/api/gallery/e/photo/2" />);
expect(screen.queryByText(/cannot be played in this browser/i)).not.toBeInTheDocument();
expect(screen.getByText('0:00 / 0:00')).toBeInTheDocument();
});
});
+3 -1
View File
@@ -931,7 +931,9 @@
},
"photosCount_one": "{{count}} Foto",
"photosCount_other": "{{count}} Fotos",
"poweredBy": "Bereitgestellt von PicPeak"
"poweredBy": "Bereitgestellt von PicPeak",
"videoFormatUnsupported": "Dieses Videoformat kann in diesem Browser nicht abgespielt werden. Laden Sie es herunter, um es anzusehen.",
"videoLoadFailed": "Dieses Video konnte nicht geladen werden."
},
"categories": {
"title": "Fotokategorien",
+3 -1
View File
@@ -472,7 +472,9 @@
"photosSelected_one": "{{count}} photo selected",
"photosSelected_other": "{{count}} photos selected",
"downloadSelected_one": "Download {{count}} photo",
"downloadSelected_other": "Download {{count}} photos"
"downloadSelected_other": "Download {{count}} photos",
"videoFormatUnsupported": "This video format cannot be played in this browser. Download it to watch it.",
"videoLoadFailed": "This video could not be loaded."
},
"categories": {
"title": "Photo Categories",
+3 -1
View File
@@ -313,7 +313,9 @@
"anonymous": "Anónimo"
},
"rated": "Valorado",
"commented": "Comentado"
"commented": "Comentado",
"videoFormatUnsupported": "Este formato de vídeo no se puede reproducir en este navegador. Descárgalo para verlo.",
"videoLoadFailed": "No se ha podido cargar este vídeo."
},
"categories": {
"title": "Categorías de fotos",
+3 -1
View File
@@ -334,7 +334,9 @@
"photosSelected_one": "{{count}} photo sélectionnée",
"photosSelected_other": "{{count}} photos sélectionnées",
"downloadSelected_one": "Télécharger {{count}} photo",
"downloadSelected_other": "Télécharger {{count}} photos"
"downloadSelected_other": "Télécharger {{count}} photos",
"videoFormatUnsupported": "Ce format vidéo ne peut pas être lu dans ce navigateur. Téléchargez-la pour la regarder.",
"videoLoadFailed": "Cette vidéo n'a pas pu être chargée."
},
"categories": {
"title": "Catégories de photos",
+3 -1
View File
@@ -334,7 +334,9 @@
},
"photosCount_one": "{{count}} foto",
"photosCount_other": "{{count}} foto's",
"poweredBy": "Mogelijk gemaakt door PicPeak"
"poweredBy": "Mogelijk gemaakt door PicPeak",
"videoFormatUnsupported": "Deze video-indeling kan niet in deze browser worden afgespeeld. Download de video om hem te bekijken.",
"videoLoadFailed": "Deze video kon niet worden geladen."
},
"categories": {
"title": "Fotocategorieen",
+3 -1
View File
@@ -342,7 +342,9 @@
"photosCount_many": "{{count}} fotos",
"photosCount_one": "{{count}} foto",
"photosCount_other": "{{count}} fotos",
"poweredBy": "Desenvolvido por PicPeak"
"poweredBy": "Desenvolvido por PicPeak",
"videoFormatUnsupported": "Este formato de vídeo não pode ser reproduzido neste navegador. Baixe o arquivo para assistir.",
"videoLoadFailed": "Não foi possível carregar este vídeo."
},
"categories": {
"title": "Categorias de Fotos",
+3 -1
View File
@@ -350,7 +350,9 @@
"photosCount_many": "{{count}} фото",
"photosCount_one": "{{count}} фото",
"photosCount_other": "{{count}} фото",
"poweredBy": "Работает на PicPeak"
"poweredBy": "Работает на PicPeak",
"videoFormatUnsupported": "Этот формат видео не воспроизводится в этом браузере. Скачайте файл, чтобы посмотреть его.",
"videoLoadFailed": "Не удалось загрузить это видео."
},
"categories": {
"title": "Категории фото",
+3 -1
View File
@@ -334,7 +334,9 @@
"photosSelected_one": "Izbrana {{count}} fotografija",
"photosSelected_other": "Izbranih {{count}} fotografij",
"downloadSelected_one": "Prenesi {{count}} fotografijo",
"downloadSelected_other": "Prenesi {{count}} fotografij"
"downloadSelected_other": "Prenesi {{count}} fotografij",
"videoFormatUnsupported": "Tega videoformata v tem brskalniku ni mogoče predvajati. Prenesite ga, da si ga ogledate.",
"videoLoadFailed": "Tega videa ni bilo mogoče naložiti."
},
"categories": {
"title": "Kategorije fotografij",