mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 23:19:43 +02:00
fix(file-viewer): make previewed video seekable and stop it on close
Two bugs in the File Viewer's media player, both reproduced in a real browser against an 18MB mp4 before and after the fix. 1. Closing the preview left the video playing. closeFilePreview() only dropped the overlay's `visible` class, which is display:none and nothing else, so the audio kept going with no visible player to pause. Detaching the element is not a fix either: a detached HTMLMediaElement plays on until it is garbage collected. _stopFilePreviewMedia() now pauses, drops src and load()s every media element (also on re-open, where overwriting innerHTML had the same effect), which additionally aborts the in-flight download. 2. The scrub bar was inert. file-raw read the whole file and answered 200 with no Accept-Ranges, so Chrome reported video.seekable as [0, 0] and silently reverted `currentTime = x`; Safari refuses to start such media at all. Raw bodies are now streamed and range-aware: Accept-Ranges: bytes on every response, 206 + Content-Range for a Range request, 416 for one past EOF, and a malformed spec ignored (200) per RFC 9110. Parsing is pure in src/web/http-range.ts. Measured on tmp/codeman-crt-v5-66s.mp4 (18MB, 66.6s): before seekable [0, 0] seek to 56.6s reverted to 3.9s close: still playing after seekable [0, 66.56] seek to 56.6s landed at 60.2s close: paused, NETWORK_EMPTY Range slices are byte-identical to `dd`, the full-file path is byte-identical to the file, and the SVG octet-stream/attachment hardening and the 50MB cap are unchanged (the cap is still checked before the range). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,86 @@
|
||||
/**
|
||||
* @fileoverview Pure HTTP byte-range parsing for the raw file-serving routes.
|
||||
*
|
||||
* Why this exists: a `<video>`/`<audio>` element is only seekable when the
|
||||
* server advertises `Accept-Ranges: bytes` and answers `Range` requests with
|
||||
* `206 Partial Content`. Serving the whole file with `200 OK` (what file-raw
|
||||
* did) makes Chrome report `video.seekable === [0, 0]`, so the scrub bar is
|
||||
* inert and `currentTime = x` is silently ignored; Safari refuses to start the
|
||||
* media at all. Parsing lives here, away from the IO, so the edge cases
|
||||
* (suffix ranges, open-ended ranges, oversized specs, empty files) are unit
|
||||
* testable without touching the filesystem.
|
||||
*
|
||||
* Deliberately single-range only: multi-range responses require a
|
||||
* `multipart/byteranges` body that no media element asks for, and RFC 9110
|
||||
* §14.2 lets a server ignore a Range it does not want to honor and answer with
|
||||
* the full representation. Same for syntactically invalid specs — those are
|
||||
* ignored (200), while a syntactically valid but out-of-bounds spec is the one
|
||||
* case that earns a 416.
|
||||
*/
|
||||
|
||||
/** Result of parsing a `Range` header against a known representation size. */
|
||||
export type ByteRangeRequest =
|
||||
/** No range, an unsupported unit, or a malformed spec — serve the whole file with 200. */
|
||||
| { kind: 'full' }
|
||||
/** A satisfiable single range, inclusive on both ends — serve 206. */
|
||||
| { kind: 'partial'; start: number; end: number }
|
||||
/** Syntactically valid but outside the representation — serve 416. */
|
||||
| { kind: 'unsatisfiable' };
|
||||
|
||||
const BYTES_RANGE_SPEC = /^(\d*)-(\d*)$/;
|
||||
|
||||
/**
|
||||
* Digits → number, bounded. A range spec is arbitrary client input, so a
|
||||
* 100-digit first-byte-pos must not become `Infinity` (which would then flow
|
||||
* into a `createReadStream` offset). Anything longer than a safe integer is
|
||||
* clamped, which the callers then treat as "past the end of the file".
|
||||
*/
|
||||
function parseBoundedInt(digits: string): number {
|
||||
return digits.length > 15 ? Number.MAX_SAFE_INTEGER : Number(digits);
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a `Range` request header against a file of `size` bytes.
|
||||
*
|
||||
* @param header - Raw header value (`req.headers.range`). Arrays (a duplicated
|
||||
* header) are ignored rather than guessed at.
|
||||
* @param size - Size of the full representation in bytes.
|
||||
*/
|
||||
export function parseByteRange(header: string | string[] | undefined, size: number): ByteRangeRequest {
|
||||
if (typeof header !== 'string') return { kind: 'full' };
|
||||
|
||||
const trimmed = header.trim();
|
||||
const eq = trimmed.indexOf('=');
|
||||
if (eq < 0 || trimmed.slice(0, eq).trim().toLowerCase() !== 'bytes') return { kind: 'full' };
|
||||
|
||||
const spec = trimmed.slice(eq + 1).trim();
|
||||
// Multi-range requests would need a multipart/byteranges body; ignoring the
|
||||
// header and serving the full representation is a valid answer.
|
||||
if (!spec || spec.includes(',')) return { kind: 'full' };
|
||||
|
||||
const match = BYTES_RANGE_SPEC.exec(spec);
|
||||
if (!match) return { kind: 'full' };
|
||||
const [, rawStart, rawEnd] = match;
|
||||
if (!rawStart && !rawEnd) return { kind: 'full' };
|
||||
|
||||
// Suffix range: `bytes=-N` means the LAST N bytes, not "from N to the end".
|
||||
if (!rawStart) {
|
||||
const suffix = parseBoundedInt(rawEnd);
|
||||
if (suffix === 0 || size === 0) return { kind: 'unsatisfiable' };
|
||||
return { kind: 'partial', start: Math.max(0, size - suffix), end: size - 1 };
|
||||
}
|
||||
|
||||
const start = parseBoundedInt(rawStart);
|
||||
if (size === 0 || start >= size) return { kind: 'unsatisfiable' };
|
||||
|
||||
// `bytes=N-` — from N to the end of the file. This is the form Chrome opens
|
||||
// a media element with (`bytes=0-`), so it must answer 206, not 200.
|
||||
if (!rawEnd) return { kind: 'partial', start, end: size - 1 };
|
||||
|
||||
const requestedEnd = parseBoundedInt(rawEnd);
|
||||
// last-byte-pos < first-byte-pos is an invalid spec, not an unsatisfiable
|
||||
// one: RFC 9110 §14.1.1 says the whole header field is then ignored.
|
||||
if (requestedEnd < start) return { kind: 'full' };
|
||||
|
||||
return { kind: 'partial', start, end: Math.min(requestedEnd, size - 1) };
|
||||
}
|
||||
@@ -3246,6 +3246,9 @@ Object.assign(CodemanApp.prototype, {
|
||||
|
||||
// Edit mode: reset any prior editor state whenever a preview (re)loads.
|
||||
this._resetFilePreviewEdit();
|
||||
// Stop whatever the previous preview was playing. Overwriting innerHTML
|
||||
// only DETACHES a <video>/<audio>; a detached media element keeps playing.
|
||||
this._stopFilePreviewMedia();
|
||||
|
||||
// Show overlay with loading state
|
||||
overlay.classList.add('visible');
|
||||
@@ -3330,10 +3333,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
bodyEl.innerHTML = `<img src="${data.url}" alt="${escapeHtml(filePath)}">`;
|
||||
footerEl.textContent = `${this.formatFileSize(data.size)} \u2022 ${data.extension}`;
|
||||
} else if (data.type === 'video') {
|
||||
bodyEl.innerHTML = `<video src="${data.url}" controls autoplay></video>`;
|
||||
// playsinline: iOS otherwise hijacks playback into its fullscreen
|
||||
// player, which leaves the overlay behind it and its own close button
|
||||
// as the only way back.
|
||||
bodyEl.innerHTML = `<video src="${escapeHtml(data.url)}" controls autoplay playsinline preload="metadata"></video>`;
|
||||
footerEl.textContent = `${this.formatFileSize(data.size)} \u2022 ${data.extension}`;
|
||||
} else if (data.type === 'audio') {
|
||||
bodyEl.innerHTML = `<audio src="${data.url}" controls autoplay></audio>`;
|
||||
bodyEl.innerHTML = `<audio src="${escapeHtml(data.url)}" controls autoplay preload="metadata"></audio>`;
|
||||
footerEl.textContent = `${this.formatFileSize(data.size)} \u2022 ${data.extension}`;
|
||||
} else if (data.type === 'binary') {
|
||||
const downloadHref = `/api/sessions/${sessionId}/file-raw?path=${encodeURIComponent(filePath)}&download=true`;
|
||||
@@ -3366,9 +3372,36 @@ Object.assign(CodemanApp.prototype, {
|
||||
if (overlay) {
|
||||
overlay.classList.remove('visible');
|
||||
}
|
||||
// The overlay is hidden with display:none, which stops it being PAINTED and
|
||||
// nothing else: a <video>/<audio> inside it keeps playing, keeps its audio
|
||||
// audible and keeps streaming from the server. Closing has to stop it.
|
||||
this._stopFilePreviewMedia();
|
||||
this.filePreviewContent = '';
|
||||
},
|
||||
|
||||
/**
|
||||
* Pause and unload every media element in the preview body, then empty it.
|
||||
*
|
||||
* Removing the element from the DOM is NOT enough — a detached HTMLMediaElement
|
||||
* plays on until it is garbage collected, which is why the X button used to
|
||||
* leave a video audible. pause() stops playback, dropping src + load() aborts
|
||||
* the in-flight network fetch and puts the element back in NETWORK_EMPTY.
|
||||
*/
|
||||
_stopFilePreviewMedia() {
|
||||
const bodyEl = this.$('filePreviewBody');
|
||||
if (!bodyEl) return;
|
||||
for (const media of bodyEl.querySelectorAll('video, audio')) {
|
||||
try {
|
||||
media.pause();
|
||||
media.removeAttribute('src');
|
||||
media.load();
|
||||
} catch (err) {
|
||||
console.warn('Failed to stop preview media:', err);
|
||||
}
|
||||
}
|
||||
bodyEl.innerHTML = '';
|
||||
},
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════
|
||||
// File Viewer edit mode (issue #212 — docs/file-viewer-edit-plan.md)
|
||||
// ═══════════════════════════════════════════════════════════════
|
||||
|
||||
@@ -46,6 +46,7 @@ import {
|
||||
} from '../route-helpers.js';
|
||||
import type { FastifyRequest } from 'fastify';
|
||||
import type { SessionAttachmentHistoryItem, SessionState } from '../../types/session.js';
|
||||
import { parseByteRange } from '../http-range.js';
|
||||
import { isSensitivePath } from '../sensitive-path.js';
|
||||
import { SseEvent } from '../sse-events.js';
|
||||
import type { ConfigPort, EventPort, SessionPort } from '../ports/index.js';
|
||||
@@ -86,7 +87,13 @@ function buildContentDisposition(disposition: 'inline' | 'attachment', fileName:
|
||||
|
||||
function sendRawStream(reply: FastifyReply, content: ReadStream): void {
|
||||
const headers = reply.getHeaders();
|
||||
// hijack() answers on reply.raw, which keeps Fastify's own status handling out
|
||||
// of the picture — so a 206 set with reply.code() has to be carried across by
|
||||
// hand or a partial body would go out labelled 200 and the browser would treat
|
||||
// it as the whole file.
|
||||
const statusCode = reply.statusCode;
|
||||
reply.hijack();
|
||||
reply.raw.statusCode = statusCode;
|
||||
|
||||
for (const [name, value] of Object.entries(headers)) {
|
||||
if (value !== undefined) {
|
||||
@@ -106,12 +113,54 @@ function sendRawStream(reply: FastifyReply, content: ReadStream): void {
|
||||
content.pipe(reply.raw);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stream a file body, honoring a `Range` request header.
|
||||
*
|
||||
* Callers set Content-Type/Content-Disposition first; this adds the
|
||||
* range-related headers and the body. Range support is what makes the file
|
||||
* viewer's `<video>`/`<audio>` seekable: with a plain 200 and no
|
||||
* `Accept-Ranges`, Chrome reports `video.seekable` as `[0, 0]`, the scrub bar
|
||||
* does nothing and `currentTime = x` is silently reverted (measured against an
|
||||
* 18MB mp4 before this existed). It also stops each seek from re-reading the
|
||||
* whole file into memory.
|
||||
*/
|
||||
function sendFileBody(
|
||||
reply: FastifyReply,
|
||||
resolvedPath: string,
|
||||
size: number,
|
||||
rangeHeader: string | string[] | undefined
|
||||
): void {
|
||||
reply.header('Accept-Ranges', 'bytes');
|
||||
const range = parseByteRange(rangeHeader, size);
|
||||
|
||||
if (range.kind === 'unsatisfiable') {
|
||||
reply
|
||||
.code(416)
|
||||
.header('Content-Range', `bytes */${size}`)
|
||||
.type('application/json; charset=utf-8')
|
||||
.send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Requested range not satisfiable'));
|
||||
return;
|
||||
}
|
||||
|
||||
if (range.kind === 'partial') {
|
||||
reply.code(206);
|
||||
reply.header('Content-Range', `bytes ${range.start}-${range.end}/${size}`);
|
||||
reply.header('Content-Length', range.end - range.start + 1);
|
||||
sendRawStream(reply, createReadStream(resolvedPath, { start: range.start, end: range.end }));
|
||||
return;
|
||||
}
|
||||
|
||||
reply.header('Content-Length', size);
|
||||
sendRawStream(reply, createReadStream(resolvedPath));
|
||||
}
|
||||
|
||||
async function serveRawFile(
|
||||
reply: FastifyReply,
|
||||
resolvedPath: string,
|
||||
fileName: string,
|
||||
extension: string,
|
||||
download?: boolean
|
||||
download?: boolean,
|
||||
rangeHeader?: string | string[]
|
||||
): Promise<void> {
|
||||
const stat = await fs.stat(resolvedPath);
|
||||
const MAX_RAW_ATTACHMENT_SIZE = 50 * 1024 * 1024; // 50MB, matching file-raw / download
|
||||
@@ -126,24 +175,21 @@ async function serveRawFile(
|
||||
);
|
||||
return;
|
||||
}
|
||||
const content = createReadStream(resolvedPath);
|
||||
if (download || extension === 'svg') {
|
||||
reply.header(
|
||||
'Content-Type',
|
||||
extension === 'svg' ? 'application/octet-stream' : MIME_TYPES[extension] || 'application/octet-stream'
|
||||
);
|
||||
reply.header('Content-Disposition', buildContentDisposition('attachment', fileName));
|
||||
reply.header('Content-Length', stat.size);
|
||||
reply.header('X-Content-Type-Options', 'nosniff');
|
||||
sendRawStream(reply, content);
|
||||
sendFileBody(reply, resolvedPath, stat.size, rangeHeader);
|
||||
return;
|
||||
}
|
||||
|
||||
reply.header('Content-Type', MIME_TYPES[extension] || 'application/octet-stream');
|
||||
reply.header('Content-Disposition', buildContentDisposition('inline', fileName));
|
||||
reply.header('Content-Length', stat.size);
|
||||
reply.header('X-Content-Type-Options', 'nosniff');
|
||||
sendRawStream(reply, content);
|
||||
sendFileBody(reply, resolvedPath, stat.size, rangeHeader);
|
||||
}
|
||||
|
||||
function getAttachmentOr404(
|
||||
@@ -849,7 +895,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
await serveConvertedPreview(reply, resolvedPath, fileName, extension);
|
||||
return;
|
||||
}
|
||||
await serveRawFile(reply, resolvedPath, fileName, extension);
|
||||
await serveRawFile(reply, resolvedPath, fileName, extension, false, req.headers.range);
|
||||
});
|
||||
|
||||
// File tree listing
|
||||
@@ -1369,24 +1415,24 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
json: 'application/json',
|
||||
};
|
||||
|
||||
const content = await fs.readFile(resolvedPath);
|
||||
const rawBasename = filePath!.split('/').pop() || 'download';
|
||||
// Sanitize filename for Content-Disposition header (prevent header injection)
|
||||
const basename = rawBasename.replace(/["\\\r\n]/g, '_');
|
||||
if (download === 'true' || ext === 'svg') {
|
||||
reply.raw.writeHead(200, {
|
||||
...inheritedHeaders(reply),
|
||||
'Content-Type': ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream',
|
||||
'Content-Disposition': `attachment; filename="${basename}"`,
|
||||
'Content-Length': content.length,
|
||||
'X-Content-Type-Options': 'nosniff',
|
||||
});
|
||||
reply.raw.end(content);
|
||||
reply.header(
|
||||
'Content-Type',
|
||||
ext === 'svg' ? 'application/octet-stream' : mimeTypes[ext] || 'application/octet-stream'
|
||||
);
|
||||
reply.header('Content-Disposition', `attachment; filename="${basename}"`);
|
||||
reply.header('X-Content-Type-Options', 'nosniff');
|
||||
sendFileBody(reply, resolvedPath, stat.size, req.headers.range);
|
||||
return;
|
||||
}
|
||||
reply.header('Content-Type', mimeTypes[ext] || 'application/octet-stream');
|
||||
reply.header('X-Content-Type-Options', 'nosniff');
|
||||
reply.send(content);
|
||||
// Streamed, range-aware: this is the <video>/<audio> source the file
|
||||
// viewer points at, and a 200-only response makes the media unseekable.
|
||||
sendFileBody(reply, resolvedPath, stat.size, req.headers.range);
|
||||
} catch (err) {
|
||||
reply
|
||||
.code(500)
|
||||
@@ -1503,7 +1549,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even
|
||||
if (!servePath) return;
|
||||
|
||||
try {
|
||||
await serveRawFile(reply, servePath, record.fileName, record.extension, download === 'true');
|
||||
await serveRawFile(reply, servePath, record.fileName, record.extension, download === 'true', req.headers.range);
|
||||
} catch (err) {
|
||||
reply
|
||||
.code(500)
|
||||
|
||||
Reference in New Issue
Block a user