fix(subsync): decode file:// tracks mpv reports for dropped subtitles

- Sync a subtitle dropped onto mpv without hitting "Protocol file: not supported"; decode file:// external-filename/path back to a real path for target, reference, and video
- Stream strip proxy: destroy the connection instead of retrying/502 once a response is handed off, and cap buffered playlist bodies
- Bridge installer: cap downloaded bundle size to guard against a lying/missing content-length
- Anime browser playback: bump generation on dispose so a stale in-flight playEpisode cleans up its own subtitle cache
- Fix a flaky sidecar-process test by binding to an OS-assigned port instead of pre-allocating one
This commit is contained in:
2026-08-06 22:41:16 -07:00
parent 47d31e9628
commit 5c778a9442
10 changed files with 200 additions and 8 deletions
+4
View File
@@ -0,0 +1,4 @@
type: fixed
area: subsync
- Subsync no longer fails with `Protocol "file:" not supported` on a subtitle that was dropped onto mpv. mpv reports such a track as a percent-encoded `file://` URL, which subsync read as a stream and tried to fetch over HTTP; the URL is now decoded back to its path, so both the retimed target and an alass reference work. A `file://` video path is treated as local too, which restores the video reference and ffsubsync for a dropped file.
+5 -2
View File
@@ -2,6 +2,7 @@ import test from 'node:test';
import assert from 'node:assert/strict';
import http from 'node:http';
import { EventEmitter } from 'node:events';
import type { AddressInfo } from 'node:net';
import type { spawn as spawnType, ChildProcess } from 'node:child_process';
import { allocatePort, startSidecar } from './sidecar-process';
import type { BundleBinaries } from './sidecar-bundle';
@@ -92,7 +93,6 @@ test('an early exit is reported with its code rather than waiting out the deadli
});
test('onExit reports a death after readiness, including to late subscribers', async () => {
const port = await allocatePort();
// Fake the bridge's capabilities endpoint so startSidecar reports ready.
const server = http.createServer((_req, res) => {
res.writeHead(200, { 'content-type': 'application/json' });
@@ -100,7 +100,10 @@ test('onExit reports a death after readiness, including to late subscribers', as
JSON.stringify({ mangatanMihonBridge: 1, sourceFactory: true, preferenceCallbacks: true }),
);
});
await new Promise<void>((resolve) => server.listen(port, '127.0.0.1', resolve));
// Bind first and take the port the OS assigned: allocating one up front and
// binding it after leaves a window for another listener to claim it.
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const { port } = server.address() as AddressInfo;
const child = fakeChild();
const spawnImpl = (() => child) as unknown as typeof spawnType;
+23 -1
View File
@@ -149,6 +149,10 @@ export function startStreamStripProxy(
attempt: number,
): void {
const mayRetry = req.method === 'GET' && attempt === 0;
// Once the response is handed off, its headers (and often part of its body)
// are already on the wire: a later upstream error can only be reported by
// killing the connection, never by retrying or writing a 502.
let handedOff = false;
const retry = (): void => {
setTimeout(
() => requestUpstream(req, res, upstreamUrl, requestHeaders, attempt + 1),
@@ -172,6 +176,7 @@ export function startStreamStripProxy(
}
log(`[stream-proxy] upstream ${status} for ${upstreamUrl.pathname}`);
}
handedOff = true;
handleUpstreamResponse(req, res, upstream);
},
);
@@ -180,6 +185,11 @@ export function startStreamStripProxy(
upstreamRequest.destroy(new Error(`upstream silent for ${UPSTREAM_TIMEOUT_MS}ms`));
});
upstreamRequest.on('error', (error) => {
if (handedOff) {
log(`[stream-proxy] upstream failed mid-response: ${String(error)}`);
res.destroy();
return;
}
if (mayRetry) {
log(`[stream-proxy] upstream request failed: ${String(error)}; retrying once`);
retry();
@@ -214,7 +224,19 @@ export function startStreamStripProxy(
if (isPlaylist) {
const chunks: Buffer[] = [];
upstream.on('data', (chunk: Buffer) => chunks.push(chunk));
let buffered = 0;
upstream.on('data', (chunk: Buffer) => {
buffered += chunk.length;
// A playlist is text and small; anything this large is not one, and it
// has to be held whole in memory to be rewritten.
if (buffered > DECISION_BYTES) {
log(`[stream-proxy] playlist body over ${DECISION_BYTES} bytes; dropping`);
upstream.destroy();
res.destroy();
return;
}
chunks.push(chunk);
});
upstream.on('end', () => {
const body = rewritePlaylistOrigins(
Buffer.concat(chunks).toString('utf8'),
+5 -2
View File
@@ -5,6 +5,7 @@ import {
codecToExtension,
fileExists,
MpvTrack,
resolveLocalMediaPath,
runCommand,
summarizeCommandFailure,
} from '../../subsync/utils';
@@ -148,10 +149,12 @@ export async function extractSubtitleTrackToFile(
input: SubtitleExtractionInput,
): Promise<FileExtractionResult> {
if (input.track.external) {
const externalPath = input.track['external-filename'];
if (typeof externalPath !== 'string' || externalPath.length === 0) {
const externalFilename = input.track['external-filename'];
if (typeof externalFilename !== 'string' || externalFilename.length === 0) {
throw new Error('External subtitle track has no file path');
}
// A dropped subtitle arrives as a `file://` URL, which is local, not remote.
const externalPath = resolveLocalMediaPath(externalFilename);
if (isRemoteMediaPath(externalPath)) {
return downloadRemoteSubtitleTrack(externalPath, input.httpHeaders);
}
+95
View File
@@ -4,6 +4,7 @@ import * as fs from 'fs';
import * as http from 'http';
import * as os from 'os';
import * as path from 'path';
import { pathToFileURL } from 'url';
import { runSubsyncManual } from './subsync';
import type { TriggerSubsyncFromConfigDeps } from './subsync';
@@ -122,3 +123,97 @@ test('runSubsyncManual syncs stream subtitle tracks served over http', async (t)
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
/**
* Dropping a subtitle onto mpv hands it a `file://` URL, and mpv reports that
* URL back as the track's `external-filename`. Reading it as a stream made the
* run die with `Protocol "file:" not supported. Expected "http:"`.
*/
test('runSubsyncManual retimes a dropped file:// track against a stream reference', async (t) => {
if (process.platform === 'win32') {
t.skip('stub shell scripts are not executable on Windows');
return;
}
const host = await startStubHost();
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subsync-file-url-'));
const alassLogPath = path.join(tmpDir, 'alass-args.log');
const alassPath = path.join(tmpDir, 'alass.sh');
fs.writeFileSync(
alassPath,
`#!/bin/sh\n: > "${alassLogPath}"\nfor arg in "$@"; do printf '%s\\n' "$arg" >> "${alassLogPath}"; done\ncp "$2" "${tmpDir}/target.copy"\nprintf '%s' "retimed" > "$3"\nexit 0\n`,
{ mode: 0o755 },
);
// Percent-encoded on purpose: the space and the CJK are what a naive
// "strip file://" would leave mangled.
const targetPath = path.join(tmpDir, 'ワンピース sdh.srt');
const targetBody = '1\n00:00:03,000 --> 00:00:04,000\nドロップ\n';
fs.writeFileSync(targetPath, targetBody);
const referenceUrl = host.url('/subs/en.srt');
const sentCommands: Array<Array<string | number>> = [];
const deps: Pick<TriggerSubsyncFromConfigDeps, 'getMpvClient' | 'getResolvedConfig'> = {
getMpvClient: () => ({
connected: true,
currentAudioStreamIndex: null,
send: (payload) => {
sentCommands.push(payload.command);
},
requestProperty: async (name: string) => {
if (name === 'path') return host.url('/stream/video.m3u8');
if (name === 'sid') return 2;
if (name === 'secondary-sid') return null;
if (name === 'track-list') {
return [
{
id: 1,
type: 'sub',
selected: true,
external: true,
lang: 'en',
'external-filename': referenceUrl,
},
{
id: 2,
type: 'sub',
selected: true,
external: true,
lang: 'ja',
'external-filename': pathToFileURL(targetPath).href,
},
];
}
return null;
},
}),
getResolvedConfig: () => ({
alassPath,
ffsubsyncPath: '',
ffmpegPath: path.join(tmpDir, 'no-such-ffmpeg'),
replace: true,
}),
};
try {
const result = await runSubsyncManual(
{ engine: 'alass', referenceTrackId: 1, targetTrackId: 2 },
deps,
);
assert.equal(result.ok, true, result.message);
const alassArgs = fs.readFileSync(alassLogPath, 'utf8').trim().split('\n');
// The dropped file went in as-is, decoded, and was retimed in place.
assert.equal(alassArgs[1], targetPath);
assert.equal(fs.readFileSync(path.join(tmpDir, 'target.copy'), 'utf8'), targetBody);
assert.equal(alassArgs[2], targetPath);
assert.equal(fs.readFileSync(targetPath, 'utf8'), 'retimed');
const loadCommand = sentCommands.find((command) => command[0] === 'sub-add');
assert.equal(loadCommand?.[1], targetPath);
} finally {
await host.close();
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
+4 -1
View File
@@ -6,6 +6,7 @@ import {
formatTrackLabel,
getTrackById,
MpvTrack,
resolveLocalMediaPath,
runCommand,
summarizeCommandFailure,
SubsyncContext,
@@ -136,7 +137,9 @@ async function gatherSubsyncContext(client: MpvClientLike): Promise<SubsyncConte
client.requestProperty('track-list'),
]);
const videoPath = typeof videoPathRaw === 'string' ? videoPathRaw : '';
// A dropped file is reported as a `file://` URL; alass, ffsubsync and ffmpeg
// all need the plain path, and the stream checks must not read it as remote.
const videoPath = typeof videoPathRaw === 'string' ? resolveLocalMediaPath(videoPathRaw) : '';
if (!videoPath) {
throw new Error('No video is currently loaded');
}
+23 -1
View File
@@ -21,6 +21,18 @@ export type InstallStage = 'locating' | 'downloading' | 'verifying' | 'extractin
/** Neither call has a default deadline, so a hung network would stall install. */
const RELEASES_TIMEOUT_MS = 30_000;
const DOWNLOAD_TIMEOUT_MS = 300_000;
/**
* Hard ceiling on the buffered bundle. The real asset is a JRE plus a jar, well
* under this; the cap only stops a wrong or hostile URL from filling memory.
*/
const MAX_BUNDLE_BYTES = 512 * 1024 * 1024;
function oversizedBundle(): Error {
return new Error(
`The anime bridge bundle exceeded ${Math.round(MAX_BUNDLE_BYTES / (1024 * 1024))} MB; ` +
'the download was stopped.',
);
}
export interface InstallProgress {
stage: InstallStage;
@@ -78,7 +90,11 @@ async function downloadWithProgress(
): Promise<Uint8Array> {
const declared = Number(response.headers.get('content-length') ?? '0');
const reader = response.body?.getReader();
if (!reader) return new Uint8Array(await response.arrayBuffer());
if (!reader) {
const buffer = await response.arrayBuffer();
if (buffer.byteLength > MAX_BUNDLE_BYTES) throw oversizedBundle();
return new Uint8Array(buffer);
}
const chunks: Uint8Array[] = [];
let received = 0;
@@ -86,6 +102,12 @@ async function downloadWithProgress(
const { done, value } = await reader.read();
if (done) break;
if (value) {
// Checked against the bytes actually read, not content-length: a lying
// (or absent) header must not let the bundle allocate without bound.
if (received + value.length > MAX_BUNDLE_BYTES) {
await reader.cancel().catch(() => {});
throw oversizedBundle();
}
chunks.push(value);
received += value.length;
if (declared > 0) onProgress?.(Math.min(1, received / declared));
@@ -183,6 +183,9 @@ export function createAnimeBrowserPlayback(options: AnimeBrowserPlaybackOptions)
}
async function dispose(): Promise<void> {
// Bumping the generation makes any in-flight playEpisode stale, so a cache
// it is still writing gets removed by that call instead of outliving us.
playbackGeneration += 1;
const cacheDir = subtitleCacheDir;
subtitleCacheDir = null;
await removeSubtitleCache(cacheDir, deps.subtitleCacheIo);
+19 -1
View File
@@ -1,6 +1,6 @@
import test from 'node:test';
import assert from 'node:assert/strict';
import { codecToExtension, getSubsyncConfig } from './utils';
import { codecToExtension, getSubsyncConfig, resolveLocalMediaPath } from './utils';
test('codecToExtension maps stream/web formats to ffmpeg extractable extensions', () => {
assert.equal(codecToExtension('subrip'), 'srt');
@@ -22,3 +22,21 @@ test('getSubsyncConfig respects explicit replace value', () => {
assert.equal(getSubsyncConfig({ replace: false }).replace, false);
assert.equal(getSubsyncConfig({ replace: true }).replace, true);
});
test('resolveLocalMediaPath decodes file URLs mpv reports for dropped files', () => {
assert.equal(
resolveLocalMediaPath('file:///home/user/subs/%E3%83%AF%E3%83%B3%20sdh.srt'),
'/home/user/subs/ワン sdh.srt',
);
assert.equal(resolveLocalMediaPath('FILE:///tmp/ref.srt'), '/tmp/ref.srt');
});
test('resolveLocalMediaPath leaves plain paths and stream URLs alone', () => {
assert.equal(resolveLocalMediaPath('/tmp/ref.srt'), '/tmp/ref.srt');
assert.equal(
resolveLocalMediaPath('https://jellyfin.example/subs/eng.srt'),
'https://jellyfin.example/subs/eng.srt',
);
// A UNC host has no local path; the caller's own error is the useful one.
assert.equal(resolveLocalMediaPath('file://host/share/ref.srt'), 'file://host/share/ref.srt');
});
+19
View File
@@ -1,6 +1,7 @@
import * as fs from 'fs';
import * as childProcess from 'child_process';
import * as path from 'path';
import { fileURLToPath } from 'url';
import { DEFAULT_CONFIG } from '../config';
import { SubsyncConfig } from '../types';
@@ -119,6 +120,24 @@ export function summarizeCommandFailure(command: string, result: CommandResult):
return `command failed (${command}) ${parts.join(' | ')}`;
}
/**
* Turn a `file://` URL back into a plain path.
*
* mpv echoes back whatever it was given, and a drag-and-drop (or a `file://`
* argument) hands it a percent-encoded URL. Everything downstream wants a real
* path: `fs` cannot stat the URL, alass and ffmpeg cannot open it, and the
* remote-track check reads it as a stream and tries to fetch a local file over
* HTTP — which fails with `Protocol "file:" not supported`.
*/
export function resolveLocalMediaPath(value: string): string {
if (!/^file:\/\//i.test(value)) return value;
try {
return fileURLToPath(new URL(value));
} catch {
return value;
}
}
export function fileExists(pathOrEmpty: string): boolean {
if (!pathOrEmpty) return false;
try {