* fix(admin): serve videos with their real MIME type in the admin photo view (#908) The admin view route built Content-Type from the filename extension — image/<ext> — which is invalid for videos (image/mp4). The admin player fetches this URL into a blob that inherits the type, and browsers refuse to play a <video> blob labeled image/*: blank/grey preview, while download (which already uses photo.mime_type) worked fine. Stored mime_type now wins; videos without one fall back to video/mp4, images to the extension, and extensionless files to image/jpeg instead of the equally invalid bare 'image/'. Also unrefs chunkedUploadService's module-level hourly cleanup interval: it kept Jest from exiting for any suite requiring adminPhotos (it's why adminPhotos.reference sits on the CI ignore list). Production behavior unchanged — the HTTP listener keeps the process alive. New adminPhotoContentType suite pins all four MIME cases. * fix(admin): harden admin photo Content-Type resolution (#908 review round) External review findings, all verified: - The header is now ALWAYS image/* or video/*. photos.mime_type is never echoed verbatim unless it is a video/ type — the chunked-upload path stores the client-sent MIME unvalidated, so a stored text/html served inline under the app origin was a same-origin XSS hazard. - MIME-less videos map from the extension via the shared EXTENSION_TO_MIME (.mov → video/quicktime, .webm → video/webm) instead of a blanket video/mp4 that would mislabel them. - Images ignore the stored MIME entirely: migration 039 backfilled image/jpeg onto every legacy row (PNGs included), so trusting it would regress previously-correct extension-derived types. Extension wins, normalized (jpg → image/jpeg). Suite extended to 8 MIME cases including the XSS guard and the 039-backfill immunity. * fix(admin): validate stored video MIME as a full header-safe token (#908 review round 2) A prefix check let malformed client-stored values through: 'video/mp4\r\nX: y' makes res.setHeader throw ERR_INVALID_CHAR — a permanent 500 for that photo — and a bare 'video/' is an invalid type. Strict /^video\/[\w.+-]+$/ now gates the stored value; anything else falls back to the extension map. Two new tests pin both shapes. * fix(admin): map-only image Content-Type — no raw extension interpolation (#908 review round 3) image/${ext} could synthesize image/svg+xml (scriptable when served inline) or header-invalid values from client-controlled chunked-upload filenames. The shared EXTENSION_TO_MIME map is now the allowlist on the image side too; unmapped extensions serve as image/jpeg — browsers sniff image bytes in img/blob contexts, so a mislabel is harmless where an injected type is not. --------- Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
co-authored by
Paul Nothaft
parent
1ee7fe7336
commit
67c56c5b61
@@ -281,8 +281,12 @@ async function cleanupExpiredUploads() {
|
||||
return expiredIds.length;
|
||||
}
|
||||
|
||||
// Run cleanup every hour
|
||||
setInterval(cleanupExpiredUploads, 60 * 60 * 1000);
|
||||
// Run cleanup every hour. unref so this module-level housekeeping timer
|
||||
// never holds the process open on its own — in production the HTTP
|
||||
// listener keeps the loop alive, and in Jest this exact handle kept the
|
||||
// runner from exiting for every suite that requires adminPhotos (#908;
|
||||
// it is why adminPhotos.reference sits on the CI ignore list).
|
||||
setInterval(cleanupExpiredUploads, 60 * 60 * 1000).unref();
|
||||
|
||||
module.exports = {
|
||||
initializeUpload,
|
||||
|
||||
Reference in New Issue
Block a user