Skip to content

Commit 4ad1cf4

Browse files
fix(parsers): validate Windows FFmpeg discovery candidates (#2871)
* fix(parsers): validate ffmpeg discovery candidates fixes reported:1785304892.118879:unicode-home-ffmpeg-discovery; PR #2859 remains unmodified. * fix(parsers): avoid Windows console path decoding
1 parent 04e0ccc commit 4ad1cf4

2 files changed

Lines changed: 63 additions & 18 deletions

File tree

packages/parsers/src/ffBinaries.test.ts

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ describe("findFfBinary", () => {
1717
const originalPlatform = process.platform;
1818

1919
afterEach(() => {
20+
vi.restoreAllMocks();
2021
vi.resetModules();
2122
vi.doUnmock("node:child_process");
2223
vi.doUnmock("node:fs");
@@ -44,17 +45,49 @@ describe("findFfBinary", () => {
4445
expect(findFfBinary("ffmpeg")).toBe(resolve(join(tmpdir(), "definitely-missing-ffmpeg")));
4546
});
4647

47-
it("prefers the real Windows exe when where lists a cmd shim first", async () => {
48+
it("prefers the real Windows exe over a cmd shim in PATH", async () => {
4849
delete process.env.HYPERFRAMES_FFMPEG_PATH;
4950
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
51+
process.env.PATH = "/tools";
5052
vi.resetModules();
51-
vi.doMock("node:child_process", () => {
52-
const mocked = { execFileSync: () => "C:\\tools\\ffmpeg.cmd\r\nC:\\tools\\ffmpeg.exe\r\n" };
53+
vi.doMock("node:fs", () => {
54+
const mocked = {
55+
existsSync: (candidate: unknown) => candidate === "/tools/ffmpeg.exe",
56+
accessSync: () => {},
57+
constants: { X_OK: 1 },
58+
};
59+
return { ...mocked, default: mocked };
60+
});
61+
const { findFfBinary } = await importFresh();
62+
63+
expect(findFfBinary("ffmpeg")).toBe("/tools/ffmpeg.exe");
64+
});
65+
66+
it("discovers a Windows binary in a Unicode current directory without decoding console output", async () => {
67+
delete process.env.HYPERFRAMES_FFMPEG_PATH;
68+
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
69+
process.env.PATH = "";
70+
const unicodeDirectory = "/用户/工具";
71+
const ffmpegPath = join(unicodeDirectory, "ffmpeg.exe");
72+
vi.spyOn(process, "cwd").mockReturnValue(unicodeDirectory);
73+
const execFileSync = vi.fn();
74+
vi.resetModules();
75+
vi.doMock("node:child_process", () => ({
76+
execFileSync,
77+
default: { execFileSync },
78+
}));
79+
vi.doMock("node:fs", () => {
80+
const mocked = {
81+
existsSync: (candidate: unknown) => candidate === ffmpegPath,
82+
accessSync: () => {},
83+
constants: { X_OK: 1 },
84+
};
5385
return { ...mocked, default: mocked };
5486
});
5587
const { findFfBinary } = await importFresh();
5688

57-
expect(findFfBinary("ffmpeg")).toBe(resolve("C:\\tools\\ffmpeg.exe"));
89+
expect(findFfBinary("ffmpeg")).toBe(ffmpegPath);
90+
expect(execFileSync).not.toHaveBeenCalled();
5891
});
5992

6093
it("falls back to scanning PATH when which/where fails", async () => {

packages/parsers/src/ffBinaries.ts

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ function isExecutablePathCandidate(candidate: string): boolean {
4949

5050
function scanPath(name: FfBinaryName): string | undefined {
5151
const pathValue = process.env.PATH;
52-
if (!pathValue) return undefined;
52+
const searchDirs = [
53+
...(process.platform === "win32" ? [process.cwd()] : []),
54+
...(pathValue ? pathValue.split(delimiter) : []),
55+
].filter(Boolean);
56+
if (searchDirs.length === 0) return undefined;
5357

5458
const extensions =
5559
process.platform === "win32"
@@ -65,8 +69,7 @@ function scanPath(name: FfBinaryName): string | undefined {
6569
]
6670
: [""];
6771
const candidates: string[] = [];
68-
for (const dir of pathValue.split(delimiter)) {
69-
if (!dir) continue;
72+
for (const dir of new Set(searchDirs)) {
7073
for (const ext of extensions) {
7174
const candidate = join(dir, `${name}${ext}`);
7275
if (isExecutablePathCandidate(candidate)) candidates.push(candidate);
@@ -101,16 +104,24 @@ function findInProjectLocalBin(name: FfBinaryName): string | undefined {
101104
function lookupOnSystem(name: FfBinaryName): string | undefined {
102105
if (pathLookupCache.has(name)) return pathLookupCache.get(name);
103106
let found: string | undefined;
104-
try {
105-
const command = process.platform === "win32" ? "where" : "which";
106-
const output = execFileSync(command, [name], {
107-
encoding: "utf-8",
108-
stdio: ["pipe", "pipe", "pipe"],
109-
timeout: 5000,
110-
});
111-
found = chooseBestPathCandidate(name, output.split(/\r?\n/));
112-
} catch {
107+
if (process.platform === "win32") {
108+
// `where.exe` writes bytes in the active console code page, while Node
109+
// decodes its stdout using a caller-selected encoding. Enumerating the
110+
// same current-directory + PATH search space from Node keeps Unicode
111+
// paths as native JS strings and avoids mojibake entirely.
113112
found = scanPath(name);
113+
} else {
114+
try {
115+
const output = execFileSync("which", [name], {
116+
encoding: "utf-8",
117+
stdio: ["pipe", "pipe", "pipe"],
118+
timeout: 5000,
119+
});
120+
const candidate = chooseBestPathCandidate(name, output.split(/\r?\n/));
121+
found = candidate && isExecutablePathCandidate(candidate) ? candidate : scanPath(name);
122+
} catch {
123+
found = scanPath(name);
124+
}
114125
}
115126
found ??= findInProjectLocalBin(name);
116127
found ??= findInCommonDirs(name);
@@ -131,8 +142,9 @@ export interface FindFfBinaryOptions {
131142
}
132143

133144
/**
134-
* Resolve an FFmpeg-family binary: env override first, then `which`/`where`,
135-
* then a manual PATH scan (covers Windows PATHEXT), a project-local
145+
* Resolve an FFmpeg-family binary: env override first, then a native
146+
* current-directory/PATH scan on Windows or `which` plus PATH scan on Unix,
147+
* then a project-local
136148
* `.hyperframes/bin`, then well-known Unix install dirs. System lookups are
137149
* cached per binary for the process lifetime; the env override is re-read on
138150
* every call.

0 commit comments

Comments
 (0)