fix(desktop): media-range review follow-ups — ignore multi-range as a whole, fstat the handle being streamed, 404 for missing files

- Multi-range requests (any comma) now fall back to a full 200 instead of silently serving only
  the first part (RFC 7233 permits ignoring Range); documented + pinned by a test.
- Open the file first and fstat that handle, then stream from the same FileHandle, so
  Content-Length and the bytes delivered come from one open file (no stat/stream race).
  Handing the FileHandle (not the raw fd) to createReadStream avoids a double close.
- ENOENT/ENOTDIR return a 404 Response instead of rejecting; covered by a test.
This commit is contained in:
YuhGuan
2026-08-23 14:43:17 +08:00
committed by brooklyn!
parent 360c1ee836
commit 244a42f636
2 changed files with 275 additions and 0 deletions

View File

@@ -0,0 +1,105 @@
import { mkdtemp, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import path from 'node:path'
import { describe, expect, it } from 'vitest'
import { buildLocalMediaResponse, mediaMimeFor, parseByteRange } from './media-range'
describe('media-range: parseByteRange', () => {
it('parses open, closed and suffix ranges', () => {
expect(parseByteRange('bytes=0-99', 1000)).toEqual({ start: 0, end: 99 })
expect(parseByteRange('bytes=500-', 1000)).toEqual({ start: 500, end: 999 })
expect(parseByteRange('bytes=-100', 1000)).toEqual({ start: 900, end: 999 })
// An end past EOF is clamped (Chromium commonly sends bytes=0-).
expect(parseByteRange('bytes=990-5000', 1000)).toEqual({ start: 990, end: 999 })
})
it('ignores multi-range requests as a whole (RFC 7233 allows answering 200 instead of multipart)', () => {
expect(parseByteRange('bytes=0-99,500-599', 1000)).toBeNull()
})
it('returns null without a usable Range and unsatisfiable when out of bounds', () => {
expect(parseByteRange(null, 1000)).toBeNull()
expect(parseByteRange('', 1000)).toBeNull()
expect(parseByteRange('items=0-1', 1000)).toBeNull()
expect(parseByteRange('bytes=1000-', 1000)).toBe('unsatisfiable')
expect(parseByteRange('bytes=50-10', 1000)).toBe('unsatisfiable')
expect(parseByteRange('bytes=0-', 0)).toBe('unsatisfiable')
})
it('maps media extensions to mime types', () => {
expect(mediaMimeFor('/x/a.MP4')).toBe('video/mp4')
expect(mediaMimeFor('/x/a.mp3')).toBe('audio/mpeg')
expect(mediaMimeFor('/x/a.bin')).toBe('application/octet-stream')
})
})
describe('media-range: buildLocalMediaResponse', () => {
async function fixture() {
const dir = await mkdtemp(path.join(tmpdir(), 'media-range-'))
const file = path.join(dir, 'clip.mp4')
const bytes = Buffer.from(Array.from({ length: 1000 }, (_, i) => i % 256))
await writeFile(file, bytes)
return { bytes, file }
}
it('serves the whole file with Accept-Ranges when no Range is given', async () => {
const { bytes, file } = await fixture()
const res = await buildLocalMediaResponse(file)
expect(res.status).toBe(200)
expect(res.headers.get('accept-ranges')).toBe('bytes')
expect(res.headers.get('content-length')).toBe('1000')
expect(res.headers.get('content-type')).toBe('video/mp4')
expect(Buffer.from(await res.arrayBuffer()).equals(bytes)).toBe(true)
})
it('serves a 206 slice with Content-Range for a Range request (seeking)', async () => {
const { bytes, file } = await fixture()
const res = await buildLocalMediaResponse(file, { rangeHeader: 'bytes=100-199' })
expect(res.status).toBe(206)
expect(res.headers.get('content-range')).toBe('bytes 100-199/1000')
expect(res.headers.get('content-length')).toBe('100')
expect(Buffer.from(await res.arrayBuffer()).equals(bytes.subarray(100, 200))).toBe(true)
})
it('serves the tail for an open-ended range and 416 when unsatisfiable', async () => {
const { file } = await fixture()
const tail = await buildLocalMediaResponse(file, { rangeHeader: 'bytes=900-' })
expect(tail.status).toBe(206)
expect(tail.headers.get('content-range')).toBe('bytes 900-999/1000')
expect((await tail.arrayBuffer()).byteLength).toBe(100)
const bad = await buildLocalMediaResponse(file, { rangeHeader: 'bytes=5000-' })
expect(bad.status).toBe(416)
expect(bad.headers.get('content-range')).toBe('bytes */1000')
})
it('serves the whole file (200) for a multi-range request and 404 for a missing file', async () => {
const { file } = await fixture()
const multi = await buildLocalMediaResponse(file, { rangeHeader: 'bytes=0-99,500-599' })
expect(multi.status).toBe(200)
expect(multi.headers.get('content-length')).toBe('1000')
expect((await multi.arrayBuffer()).byteLength).toBe(1000)
const missing = await buildLocalMediaResponse(path.join(path.dirname(file), 'nope.mp4'))
expect(missing.status).toBe(404)
})
it('answers HEAD with headers only', async () => {
const { file } = await fixture()
const res = await buildLocalMediaResponse(file, { method: 'HEAD', rangeHeader: 'bytes=0-9' })
expect(res.status).toBe(206)
expect(res.headers.get('content-length')).toBe('10')
expect(res.body).toBeNull()
})
})

View File

@@ -0,0 +1,170 @@
import { createReadStream, promises as fsp } from 'node:fs'
import path from 'node:path'
import { Readable } from 'node:stream'
/**
* Range-aware responses for local audio/video served through `hermes-media://stream/…`.
*
* The media protocol used to delegate local files to `net.fetch(file://…)` and forward the
* renderer's `Range` header, expecting a `206 Partial Content` back. In practice Electron's
* file:// loader ignores `Range` and always answers `200` with the whole body and no
* `Accept-Ranges`, so Chromium reports `video.seekable` as `[0, 0]`: the progress bar cannot be
* dragged and any seek snaps back to 0. Short clips hide this (they buffer fully within a
* second and seeking inside buffered data works), long recordings expose it immediately.
*
* This module answers the request itself: it parses `Range`, slices the file with
* `createReadStream`, and returns `206` with `Content-Range` / `Accept-Ranges` (plus `416` for
* unsatisfiable ranges, header-only `HEAD` responses, and `404` when the file is gone).
*/
const MEDIA_MIME: Record<string, string> = {
'.aac': 'audio/aac',
'.avi': 'video/x-msvideo',
'.flac': 'audio/flac',
'.m4a': 'audio/mp4',
'.m4v': 'video/mp4',
'.mkv': 'video/x-matroska',
'.mov': 'video/quicktime',
'.mp3': 'audio/mpeg',
'.mp4': 'video/mp4',
'.oga': 'audio/ogg',
'.ogg': 'audio/ogg',
'.ogv': 'video/ogg',
'.opus': 'audio/ogg',
'.wav': 'audio/wav',
'.webm': 'video/webm'
}
export function mediaMimeFor(filePath: string): string {
return MEDIA_MIME[path.extname(filePath).toLowerCase()] || 'application/octet-stream'
}
export interface ByteRange {
start: number
/** Inclusive. */
end: number
}
/**
* Parse `Range: bytes=a-b` / `bytes=a-` / `bytes=-n`.
*
* Multi-range requests (any comma in the header) are deliberately ignored as a whole — RFC 7233
* lets a server answer `200` with the full representation instead of `multipart/byteranges`,
* and Chromium's media stack only ever sends a single range. Returns `null` when there is no
* usable Range header (serve the whole file) and `'unsatisfiable'` when the range starts beyond
* the end of the file (answer 416).
*/
export function parseByteRange(header: string | null | undefined, size: number): ByteRange | null | 'unsatisfiable' {
if (!header || header.includes(',')) {
return null
}
const m = /^\s*bytes\s*=\s*(\d*)\s*-\s*(\d*)\s*$/i.exec(header)
if (!m || (m[1] === '' && m[2] === '')) {
return null
}
if (size <= 0) {
return 'unsatisfiable'
}
let start: number
let end: number
if (m[1] === '') {
// Suffix range: the last n bytes.
const suffix = Math.min(Number(m[2]), size)
if (suffix <= 0) {
return 'unsatisfiable'
}
start = size - suffix
end = size - 1
} else {
start = Number(m[1])
end = m[2] === '' ? size - 1 : Math.min(Number(m[2]), size - 1)
}
if (!Number.isFinite(start) || !Number.isFinite(end) || start >= size || start > end) {
return 'unsatisfiable'
}
return { start, end }
}
export interface LocalMediaResponseInit {
method?: string
rangeHeader?: string | null
}
/** Build the 200 / 206 / 404 / 416 response for a local media file (HEAD gets headers only). */
export async function buildLocalMediaResponse(resolvedPath: string, init: LocalMediaResponseInit = {}): Promise<Response> {
let handle: fsp.FileHandle
try {
handle = await fsp.open(resolvedPath, 'r')
} catch (error) {
const code = (error as NodeJS.ErrnoException)?.code
if (code === 'ENOENT' || code === 'ENOTDIR') {
return new Response('Media not found', { status: 404 })
}
throw error
}
// fstat the handle we are about to read from, so `Content-Length` and the streamed bytes come
// from the same open file even if the path is truncated or replaced meanwhile.
const stat = await handle.stat()
const size = stat.size
const mime = mediaMimeFor(resolvedPath)
const isHead = (init.method || 'GET').toUpperCase() === 'HEAD'
const range = parseByteRange(init.rangeHeader, size)
const baseHeaders: Record<string, string> = {
'Accept-Ranges': 'bytes',
'Cache-Control': 'no-store',
'Content-Type': mime,
'Last-Modified': stat.mtime.toUTCString()
}
const stream = (opts: { end?: number; start?: number }) =>
// Hand the FileHandle itself (not the raw fd) to the stream: it owns and closes it on end, so
// the handle is never closed twice (EBADF on garbage collection).
Readable.toWeb(createReadStream(resolvedPath, { ...opts, autoClose: true, fd: handle })) as unknown as ReadableStream
if (range === 'unsatisfiable') {
await handle.close()
return new Response(null, { headers: { ...baseHeaders, 'Content-Range': `bytes */${size}` }, status: 416 })
}
if (range === null) {
const headers = { ...baseHeaders, 'Content-Length': String(size) }
if (isHead || size === 0) {
await handle.close()
return new Response(null, { headers, status: 200 })
}
return new Response(stream({}), { headers, status: 200 })
}
const length = range.end - range.start + 1
const headers = {
...baseHeaders,
'Content-Length': String(length),
'Content-Range': `bytes ${range.start}-${range.end}/${size}`
}
if (isHead) {
await handle.close()
return new Response(null, { headers, status: 206 })
}
return new Response(stream({ end: range.end, start: range.start }), { headers, status: 206 })
}