fix(video): harden upload handling and fix video playback edge cases

Security: the gallery-video poster is now always stored as .jpg instead of the client-supplied extension, so a poster declared image/* but named x.html / x.js can't be written with that extension and served inline same-origin; local gallery files are also served with X-Content-Type-Options: nosniff.

Robustness: rejected/unauthorised uploads no longer orphan their bytes on disk (the gallery-video and file-manager handlers unlink before throwing); the file-manager per-type size cap is keyed on the extension like the filter, so a real video labelled application/octet-stream isn't wrongly rejected. UX: the file-manager thumbnail strip shows a play placeholder for video instead of a broken image; shared (public) journeys now return media_type and play videos with a play badge; and a poster-less video shows a neutral tile instead of a broken thumbnail.
This commit is contained in:
Maurice
2026-06-30 11:44:29 +02:00
committed by Maurice
parent e986c9ab27
commit 20c1858b23
8 changed files with 87 additions and 29 deletions
@@ -1,5 +1,5 @@
import { useState, useEffect } from 'react'
import { ExternalLink, Download, X, ChevronLeft, ChevronRight } from 'lucide-react'
import { ExternalLink, Download, X, ChevronLeft, ChevronRight, Play } from 'lucide-react'
import { useTranslation } from '../../i18n'
import type { TripFile } from '../../types'
import { getAuthUrl } from '../../api/authUrl'
@@ -126,14 +126,20 @@ export function ImageLightbox({ files, initialIndex, onClose }: ImageLightboxPro
}
function ThumbImg({ file, active, onClick }: { file: TripFile & { url: string }; active: boolean; onClick: () => void }) {
const fileIsVideo = isVideo(file.mime_type)
const [src, setSrc] = useState('')
useEffect(() => { getAuthUrl(file.url, 'download').then(setSrc) }, [file.url])
// Videos have no stored thumbnail and can't render as an <img>; show a play
// placeholder and don't mint a download token for them (#823).
useEffect(() => { if (!fileIsVideo) getAuthUrl(file.url, 'download').then(setSrc) }, [file.url, fileIsVideo])
return (
<button onClick={onClick} style={{
width: 48, height: 48, borderRadius: 6, overflow: 'hidden', border: active ? '2px solid #fff' : '2px solid transparent',
opacity: active ? 1 : 0.5, cursor: 'pointer', padding: 0, background: '#111', flexShrink: 0, transition: 'opacity 0.15s',
display: 'flex', alignItems: 'center', justifyContent: 'center', color: 'rgba(255,255,255,0.7)',
}}>
{src && <img src={src} alt="" style={{ width: '100%', height: '100%', objectFit: 'cover', display: 'block' }} />}
{fileIsVideo
? <Play size={16} fill="currentColor" />
: (src && <img src={src} alt="" style={{ width: '100%', height: '100%', objectFit: 'cover', display: 'block' }} />)}
</button>
)
}
@@ -163,12 +163,18 @@ export function GalleryView({ entries, gallery, journeyId, userId, trips, onPhot
className="relative aspect-square rounded-lg overflow-hidden cursor-pointer group"
onClick={() => onPhotoClick(allPhotos, i)}
>
<img
src={photoUrl(photo, 'thumbnail')}
alt={photo.caption || ''}
className="w-full h-full object-cover transition-transform group-hover:scale-105"
loading="lazy"
/>
{photo.media_type === 'video' && !photo.thumbnail_path ? (
// Poster-less video (capture failed / unsupported codec): show a
// neutral tile rather than a broken 404 thumbnail (#823).
<div className="w-full h-full bg-zinc-200 dark:bg-zinc-800" />
) : (
<img
src={photoUrl(photo, 'thumbnail')}
alt={photo.caption || ''}
className="w-full h-full object-cover transition-transform group-hover:scale-105"
loading="lazy"
/>
)}
<div className="absolute inset-0 bg-black/0 group-hover:bg-black/20 transition-colors" />
{photo.media_type === 'video' && (
<div className="absolute inset-0 flex items-center justify-center pointer-events-none">
+12 -4
View File
@@ -1,7 +1,7 @@
import { useTranslation, SUPPORTED_LANGUAGES } from '../i18n'
import { useSettingsStore } from '../store/settingsStore'
import {
List, Grid, MapPin, Camera, BookOpen, Image, Clock,
List, Grid, MapPin, Camera, BookOpen, Image, Clock, Play,
Laugh, Smile, Meh, Frown,
Sun, CloudSun, Cloud, CloudRain, CloudLightning, Snowflake,
ThumbsUp, ThumbsDown,
@@ -123,7 +123,7 @@ export default function JourneyPublicPage() {
const prosArr = entry.pros_cons?.pros ?? []
const consArr = entry.pros_cons?.cons ?? []
const hasProscons = prosArr.length > 0 || consArr.length > 0
const lightboxPhotos = photos.map(p => ({ id: String(p.id), src: photoUrl(p, token!, 'original'), caption: p.caption }))
const lightboxPhotos = photos.map(p => ({ id: String(p.id), src: photoUrl(p, token!, 'original'), caption: p.caption, mediaType: (p as any).media_type }))
const isActive = activeEntryId === String(entry.id)
return (
@@ -296,10 +296,17 @@ export default function JourneyPublicPage() {
{allPhotos.map((photo, idx) => (
<div
key={photo.id}
className="aspect-square rounded-lg overflow-hidden cursor-pointer"
onClick={() => setLightbox({ photos: allPhotos.map(p => ({ id: String(p.id), src: photoUrl(p, token!, 'original'), caption: p.caption })), index: idx })}
className="relative aspect-square rounded-lg overflow-hidden cursor-pointer"
onClick={() => setLightbox({ photos: allPhotos.map(p => ({ id: String(p.id), src: photoUrl(p, token!, 'original'), caption: p.caption, mediaType: (p as any).media_type })), index: idx })}
>
<img src={photoUrl(photo, token!, 'thumbnail')} className="w-full h-full object-cover hover:scale-105 transition-transform" alt="" loading="lazy" />
{(photo as any).media_type === 'video' && (
<div className="absolute inset-0 flex items-center justify-center pointer-events-none">
<span className="w-9 h-9 rounded-full bg-black/55 backdrop-blur flex items-center justify-center text-white">
<Play size={16} className="ml-0.5" fill="currentColor" />
</span>
</div>
)}
</div>
))}
</div>
@@ -513,6 +520,7 @@ export default function JourneyPublicPage() {
id: String(p.id),
src: photoUrl(p as any, token!, 'original'),
caption: (p as any).caption ?? null,
mediaType: (p as any).media_type,
})),
index: idx,
})}
@@ -23,7 +23,7 @@ export function useJourneyPublic() {
const [error, setError] = useState(false)
const isMobile = useIsMobile()
const [view, setView] = useState<'timeline' | 'gallery' | 'map'>('timeline')
const [lightbox, setLightbox] = useState<{ photos: { id: string; src: string; caption?: string | null }[]; index: number } | null>(null)
const [lightbox, setLightbox] = useState<{ photos: { id: string; src: string; caption?: string | null; mediaType?: string | null }[]; index: number } | null>(null)
const [showLangPicker, setShowLangPicker] = useState(false)
const locale = useSettingsStore(s => s.settings.language) || 'en'
const mapRef = useRef<JourneyMapHandle>(null)
+28 -11
View File
@@ -24,7 +24,7 @@ import type { User } from '../../types';
import { FilesService } from './files.service';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
import { MAX_FILE_SIZE, MAX_VIDEO_SIZE, BLOCKED_EXTENSIONS, filesDir, getAllowedExtensions, isVideoExtension, isVideoMime } from '../../services/fileService';
import { MAX_FILE_SIZE, MAX_VIDEO_SIZE, BLOCKED_EXTENSIONS, filesDir, getAllowedExtensions, isVideoExtension } from '../../services/fileService';
import { isDemoEmail } from '../../services/demo';
const UPLOAD = {
@@ -99,22 +99,39 @@ export class FilesController {
@Body() body: { place_id?: string; description?: string; reservation_id?: string },
@Headers('x-socket-id') socketId?: string,
) {
const trip = this.requireTrip(tripId, user);
if (process.env.DEMO_MODE?.toLowerCase() === 'true' && isDemoEmail(user.email)) {
throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403);
}
if (!this.files.can('file_upload', trip, user)) {
throw new HttpException({ error: 'No permission to upload files' }, 403);
// multer (diskStorage) has already written the upload by the time we get here,
// so every rejection below must remove the orphaned bytes — otherwise a 404/403
// leaves up to the 500 MB video cap on disk (#823).
const cleanup = () => { if (file?.path) { try { fs.unlinkSync(file.path); } catch { /* best-effort */ } } };
try {
const trip = this.requireTrip(tripId, user);
if (process.env.DEMO_MODE?.toLowerCase() === 'true' && isDemoEmail(user.email)) {
throw new HttpException({ error: 'Uploads are disabled in demo mode. Self-host TREK for full functionality.' }, 403);
}
if (!this.files.can('file_upload', trip, user)) {
throw new HttpException({ error: 'No permission to upload files' }, 403);
}
} catch (err) {
cleanup();
throw err;
}
if (!file) {
throw new HttpException({ error: 'No file uploaded' }, 400);
}
// Only video may use the larger cap; other types stay at MAX_FILE_SIZE (#823).
if (!isVideoMime(file.mimetype) && file.size > MAX_FILE_SIZE) {
try { fs.unlinkSync(file.path); } catch { /* best-effort cleanup */ }
// The per-type cap is keyed on the EXTENSION, matching how the fileFilter
// decides acceptance — so a real video labelled application/octet-stream isn't
// wrongly rejected, and the 500 MB cap only applies to actual video extensions.
const isVideoUpload = isVideoExtension(path.extname(file.originalname || ''));
if (!isVideoUpload && file.size > MAX_FILE_SIZE) {
cleanup();
throw new HttpException({ error: 'File is too large' }, 400);
}
this.assertLinkTargets(tripId, { reservation_id: body.reservation_id, place_id: body.place_id });
try {
this.assertLinkTargets(tripId, { reservation_id: body.reservation_id, place_id: body.place_id });
} catch (err) {
cleanup();
throw err;
}
const created = this.files.createFile(tripId, file, user.id, {
place_id: body.place_id,
description: body.description,
+19 -2
View File
@@ -57,7 +57,15 @@ const IMAGE_UPLOAD = {
const VIDEO_UPLOAD = {
storage: diskStorage({
destination: (_req, _file, cb) => { if (!fs.existsSync(uploadsBase)) fs.mkdirSync(uploadsBase, { recursive: true }); cb(null, uploadsBase); },
filename: (_req, file, cb) => cb(null, `${crypto.randomUUID()}${path.extname(file.originalname).toLowerCase() || (file.fieldname === 'poster' ? '.jpg' : '.mp4')}`),
filename: (_req, file, cb) => {
// The poster is ALWAYS stored as .jpg, never the client-supplied extension:
// otherwise a poster declared image/* but named x.html / x.js would land on
// disk with that extension and be served inline same-origin (stored XSS,
// reachable via the public share proxy). The video extension is validated by
// the fileFilter, so it is safe to keep.
const ext = file.fieldname === 'poster' ? '.jpg' : (path.extname(file.originalname).toLowerCase() || '.mp4');
cb(null, `${crypto.randomUUID()}${ext}`);
},
}),
limits: { fileSize: MAX_VIDEO_SIZE },
fileFilter: (_req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => {
@@ -259,10 +267,18 @@ export class JourneyController {
@Body() body: { duration_ms?: string },
) {
const video = files?.video?.[0];
const poster = files?.poster?.[0];
// multer already wrote both parts; clean them up on any rejection so a POST to
// a journey the user can't edit doesn't orphan a 500 MB clip on disk (#823).
const cleanup = () => {
for (const f of [video, poster]) {
if (f?.path) { try { fs.unlinkSync(f.path); } catch { /* best-effort */ } }
}
};
if (!video) {
cleanup();
throw new HttpException({ error: 'No video uploaded' }, 400);
}
const poster = files?.poster?.[0];
const durationMs = body?.duration_ms != null ? Number(body.duration_ms) : null;
const photos = this.journey.uploadGalleryPhotos(Number(id), user.id, [{
path: `journey/${video.filename}`,
@@ -271,6 +287,7 @@ export class JourneyController {
durationMs: durationMs != null && Number.isFinite(durationMs) ? durationMs : null,
}]);
if (!photos.length) {
cleanup();
throw new HttpException({ error: 'Not allowed' }, 403);
}
return { photos };
+4 -2
View File
@@ -105,7 +105,8 @@ export function getPublicJourney(token: string) {
const photos = db.prepare(`
SELECT gp.id, jep.entry_id, gp.photo_id, gp.caption, jep.sort_order, gp.shared, gp.created_at,
tkp.provider, tkp.asset_id, tkp.owner_id, tkp.file_path, tkp.thumbnail_path, tkp.width, tkp.height
tkp.provider, tkp.asset_id, tkp.owner_id, tkp.file_path, tkp.thumbnail_path, tkp.width, tkp.height,
tkp.media_type, tkp.duration_ms
FROM journey_entry_photos jep
JOIN journey_photos gp ON gp.id = jep.journey_photo_id
JOIN trek_photos tkp ON tkp.id = gp.photo_id
@@ -120,7 +121,8 @@ export function getPublicJourney(token: string) {
const gallery = db.prepare(`
SELECT gp.id, gp.journey_id, gp.photo_id, gp.caption, gp.shared, gp.sort_order, gp.created_at,
tkp.provider, tkp.asset_id, tkp.owner_id, tkp.file_path, tkp.thumbnail_path, tkp.width, tkp.height
tkp.provider, tkp.asset_id, tkp.owner_id, tkp.file_path, tkp.thumbnail_path, tkp.width, tkp.height,
tkp.media_type, tkp.duration_ms
FROM journey_photos gp
JOIN trek_photos tkp ON tkp.id = gp.photo_id
WHERE gp.journey_id = ?
@@ -126,6 +126,7 @@ export async function streamPhoto(
const thumbAbs = path.join(uploadsRoot, thumbRel);
if (fs.existsSync(thumbAbs)) {
res.set('Cache-Control', 'public, max-age=86400, immutable');
res.set('X-Content-Type-Options', 'nosniff');
res.sendFile(thumbAbs);
return;
}
@@ -142,6 +143,7 @@ export async function streamPhoto(
const localPath = path.join(uploadsRoot, photo.file_path);
if (fs.existsSync(localPath)) {
res.set('Cache-Control', 'public, max-age=86400');
res.set('X-Content-Type-Options', 'nosniff');
res.sendFile(localPath);
return;
}