fix: percent escaping for git --format on Windows (#629)
* test(server): add real-spawn round-trip for % in argv RED test: pass --format=%(refname)... through spawnProcess to a child node script that echoes argv[2]. Asserts the child receives the original string verbatim. Wired into the Windows CI matrix to expose the escapeWindowsCmdValue %-doubling bug under cmd.exe. * test(server): use bare 'node' command so cmd.exe path is exercised process.execPath has both an extension and a path separator on Windows, so shouldUseWindowsShell returns false and the broken %-escape pipeline is never invoked. Use the bare command name 'node' instead. * fix(server): stop doubling % in cmd.exe arg escaping on Windows cmd.exe only collapses `%%` → `%` inside batch files; on the command line / via `cmd /c "..."` `%%` stays literal. Doubling `%` in escapeWindowsCmdValue meant args like `--format=%(refname)` reached git as `--format=%%(refname)`, which git interprets as the escape sequence for a literal `%`, so format atoms appeared verbatim and the branch picker showed `%(refname)%09%(committerdate:unix)` instead of branch names. Update unit tests that pinned the broken doubling behavior.
This commit is contained in:
@@ -122,6 +122,7 @@ jobs:
|
|||||||
npx vitest run
|
npx vitest run
|
||||||
src/utils/executable.test.ts
|
src/utils/executable.test.ts
|
||||||
src/utils/spawn.launch-regression.test.ts
|
src/utils/spawn.launch-regression.test.ts
|
||||||
|
src/utils/spawn.percent-escape.test.ts
|
||||||
src/utils/spawn.test.ts
|
src/utils/spawn.test.ts
|
||||||
src/utils/run-git-command.test.ts
|
src/utils/run-git-command.test.ts
|
||||||
src/utils/checkout-git-rev-parse.test.ts
|
src/utils/checkout-git-rev-parse.test.ts
|
||||||
|
|||||||
@@ -207,9 +207,18 @@ describe("quoteWindowsCommand", () => {
|
|||||||
expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix");
|
expect(quoteWindowsCommand("feature|bugfix")).toBe("feature^|bugfix");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("doubles percent signs", () => {
|
test("does not double percent signs", () => {
|
||||||
setPlatform("win32");
|
setPlatform("win32");
|
||||||
expect(quoteWindowsCommand("100%")).toBe("100%%");
|
// cmd.exe only collapses %% → % inside batch files; on the command line
|
||||||
|
// it stays literal, which corrupts git --format atoms etc.
|
||||||
|
expect(quoteWindowsCommand("100%")).toBe("100%");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("preserves git --format atoms verbatim", () => {
|
||||||
|
setPlatform("win32");
|
||||||
|
expect(quoteWindowsCommand("--format=%(refname)%09%(committerdate:unix)")).toBe(
|
||||||
|
"--format=%^(refname^)%09%^(committerdate:unix^)",
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
test("escapes carets", () => {
|
test("escapes carets", () => {
|
||||||
@@ -227,7 +236,7 @@ describe("quoteWindowsCommand", () => {
|
|||||||
test("quotes commands with spaces after escaping metacharacters", () => {
|
test("quotes commands with spaces after escaping metacharacters", () => {
|
||||||
setPlatform("win32");
|
setPlatform("win32");
|
||||||
expect(quoteWindowsCommand("C:\\Program Files\\My Tool&Stuff\\run 100%.cmd")).toBe(
|
expect(quoteWindowsCommand("C:\\Program Files\\My Tool&Stuff\\run 100%.cmd")).toBe(
|
||||||
'"C:\\Program Files\\My Tool^&Stuff\\run 100%%.cmd"',
|
'"C:\\Program Files\\My Tool^&Stuff\\run 100%.cmd"',
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,49 @@
|
|||||||
|
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
import { afterEach, describe, expect, test } from "vitest";
|
||||||
|
|
||||||
|
import { execCommand } from "./spawn.js";
|
||||||
|
|
||||||
|
const tempDirs: string[] = [];
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
for (const dir of tempDirs.splice(0)) {
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
function writeEchoArgScript(): string {
|
||||||
|
const dir = mkdtempSync(path.join(tmpdir(), "paseo-percent-escape-"));
|
||||||
|
tempDirs.push(dir);
|
||||||
|
const scriptPath = path.join(dir, "echo-arg.js");
|
||||||
|
writeFileSync(scriptPath, "process.stdout.write(process.argv[2] ?? '');\n");
|
||||||
|
return scriptPath;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Use the bare command name "node" (no extension, no path separator) so
|
||||||
|
// `shouldUseWindowsShell` resolves to true on Windows. That routes the spawn
|
||||||
|
// through cmd.exe and exercises `quoteWindowsArgument`, which is where the
|
||||||
|
// %-doubling bug lives. Using `process.execPath` here would skip the shell
|
||||||
|
// path entirely and silently pass the test.
|
||||||
|
const COMMAND = "node";
|
||||||
|
|
||||||
|
describe("spawn argument escaping for % characters", () => {
|
||||||
|
test("delivers a git for-each-ref --format atom to the child verbatim", async () => {
|
||||||
|
const scriptPath = writeEchoArgScript();
|
||||||
|
const formatArg = "--format=%(refname)%09%(committerdate:unix)";
|
||||||
|
|
||||||
|
const { stdout } = await execCommand(COMMAND, [scriptPath, formatArg]);
|
||||||
|
|
||||||
|
expect(stdout).toBe(formatArg);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("delivers a bare percent-prefixed token to the child verbatim", async () => {
|
||||||
|
const scriptPath = writeEchoArgScript();
|
||||||
|
const arg = "%(refname)";
|
||||||
|
|
||||||
|
const { stdout } = await execCommand(COMMAND, [scriptPath, arg]);
|
||||||
|
|
||||||
|
expect(stdout).toBe(arg);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -10,7 +10,11 @@ function escapeWindowsCmdValue(value: string): string {
|
|||||||
|
|
||||||
const isQuoted = value.startsWith('"') && value.endsWith('"');
|
const isQuoted = value.startsWith('"') && value.endsWith('"');
|
||||||
const unquoted = isQuoted ? value.slice(1, -1) : value;
|
const unquoted = isQuoted ? value.slice(1, -1) : value;
|
||||||
const escaped = unquoted.replace(/%/g, "%%").replace(/([&|^<>()!])/g, "^$1");
|
// Do NOT double `%` here. cmd.exe only collapses `%%` → `%` inside batch
|
||||||
|
// files; on the command line / `cmd /c "..."` `%%` stays literal, which
|
||||||
|
// breaks args like git's `--format=%(refname)` (git treats `%%` as the
|
||||||
|
// escape for a literal `%`, so the format atoms become literals).
|
||||||
|
const escaped = unquoted.replace(/([&|^<>()!])/g, "^$1");
|
||||||
|
|
||||||
if (isQuoted || /[\s"]/u.test(unquoted)) {
|
if (isQuoted || /[\s"]/u.test(unquoted)) {
|
||||||
const quoted = escaped
|
const quoted = escaped
|
||||||
|
|||||||
Reference in New Issue
Block a user