diff --git a/packages/compiler/src/frontend/lowering/coercions/unions.ts b/packages/compiler/src/frontend/lowering/coercions/unions.ts index 316b1ecad..bc4ca4b0e 100644 --- a/packages/compiler/src/frontend/lowering/coercions/unions.ts +++ b/packages/compiler/src/frontend/lowering/coercions/unions.ts @@ -445,6 +445,7 @@ export function narrowedArmHelper( message ?? null, null, origin, + lowerer.targetPlatform === "win32", ); if (!fn) throw new InternalCompilerError("lowerer bug: invalid checked union extraction"); lowerer.coercions.checkedNarrowArms.set(name, { @@ -549,6 +550,7 @@ export function checkedNarrowCall( ? "undefined" : "null" : lowerer.fmt(failing), + lowerer.targetPlatform === "win32", )); const thrown = nodeThrowExpr(1, "", message, type, loc); failure = diff --git a/packages/compiler/src/frontend/lowering/decl-origin.ts b/packages/compiler/src/frontend/lowering/decl-origin.ts index fc5a98514..6eae57966 100644 --- a/packages/compiler/src/frontend/lowering/decl-origin.ts +++ b/packages/compiler/src/frontend/lowering/decl-origin.ts @@ -353,15 +353,51 @@ export function declarationSelector(decl: ts.Node): string | null { return null; } +/** One argument of the suggested `scriptc types patch` command, quoted so + * the shell passes it as written (scr_type_origin.c quotes the same way): + * bare when it has only characters no shell treats specially; otherwise, + * for a POSIX shell, in single quotes with each `'` as `'\''`, and for + * Windows (cmd and PowerShell), in double quotes when it has none of the + * characters either shell expands, escapes or ends a string with there. Null when no quoting + * is inert there: the note then names the command without arguments. + * Package names, selectors and types come from package declarations. */ +export function shellArgument(value: string, windows: boolean): string | null { + if (/^[A-Za-z0-9_@+=:,./-]+$/.test(value)) return value; + if (!windows) + return /^[A-Za-z0-9_@%+=:,./-]+$/.test(value) ? value : `'${value.replaceAll("'", "'\\''")}'`; + for (const ch of value) { + const code = ch.codePointAt(0)!; + // PowerShell also takes the typographic quotes U+2018-U+201F as quotes. + if (code < 0x20 || code === 0x7f || (code >= 0x2018 && code <= 0x201f)) return null; + if ('"%!^$`'.includes(ch)) return null; + } + return `"${value}"`; +} + +/** The suggested patch for a declaration: `scriptc types patch` with the + * package, the selector and the type, each quoted for the target's shell. */ +export function patchSuggestion( + pkg: string, + selector: string, + type: string, + windows: boolean, +): string { + const args = [pkg, selector, type].map((a) => shellArgument(a, windows)); + return args.every((a) => a !== null) + ? `if that declaration is wrong, patch it: scriptc types patch ${args.join(" ")}` + : "if that declaration is wrong, patch it with scriptc types patch"; +} + /** The note a union extraction's TypeError carries for a failing `arm` - * (the same text the runtime renders for checked conversions). */ -export function renderOriginNote(origin: string, arm: string): string { + * (the same text the runtime renders for checked conversions). `windows`: + * the program targets Windows, whose shells quote differently. */ +export function renderOriginNote(origin: string, arm: string, windows = false): string { const [pkg = "", version = "", , selector = "", where = "", declared = ""] = origin.split(ORIGIN_SEPARATOR); return ( `\n ${pkg}${version === "" ? "" : `@${version}`} declares ${selector}` + `${declared === "" ? "" : ` as ${declared}`} (${where})` + - `\n if that declaration is wrong, patch it: scriptc types patch ${pkg} "${selector}" "| ${arm}"` + `\n ${patchSuggestion(pkg, selector, `| ${arm}`, windows)}` ); } diff --git a/packages/compiler/src/frontend/lowering/union-narrow.ts b/packages/compiler/src/frontend/lowering/union-narrow.ts index d224d3e9e..f9a858fb6 100644 --- a/packages/compiler/src/frontend/lowering/union-narrow.ts +++ b/packages/compiler/src/frontend/lowering/union-narrow.ts @@ -39,6 +39,8 @@ export function buildUnionNarrow( /** The target's package-declared origin (decl-origin.ts): each arm's * message names the declaration and the patch admitting that arm. */ origin: string | null = null, + /** The program targets Windows (the patch command's quoting). */ + windows = false, ): IrFunction | null { if (isUnitType(target) || target.kind === "void") return null; const targetTag = from.arms.findIndex((arm) => typeEquals(arm, target)); @@ -102,6 +104,7 @@ export function buildUnionNarrow( ? "undefined" : "null" : formatType(arm), + windows, )), type: STRING, loc, diff --git a/packages/compiler/test/decl-origin-quoting.test.ts b/packages/compiler/test/decl-origin-quoting.test.ts new file mode 100644 index 000000000..40b351440 --- /dev/null +++ b/packages/compiler/test/decl-origin-quoting.test.ts @@ -0,0 +1,73 @@ +/* The suggested `scriptc types patch` command in a declared-type note + * (frontend/lowering/decl-origin.ts) passes each value it carries as one + * argument. Package names, selectors and types come from package + * declarations. Run through sh with printf in place of scriptc, the POSIX + * form prints each value exactly and creates no file. The Windows form + * double-quotes values that cmd and PowerShell keep as written there, and + * leaves the arguments out when either shell would expand or escape one. */ +import { execFile } from "node:child_process"; +import { mkdtemp, readdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { expect, test } from "vitest"; +import { patchSuggestion, shellArgument } from "../src/frontend/lowering/decl-origin.js"; + +const exec = promisify(execFile); + +const hostile = [ + "Profile.a;touch semicolon", + "Profile.$(touch subst)", + "Profile.`touch backtick`", + "Profile.it's \"quoted\" \\ end'", + "Profile.a&&touch and|touch pipe>out", + "Profile.a\nb", +]; + +test("plain values stay bare and types are quoted", () => { + expect(patchSuggestion("kit", "Profile.email", "| undefined", false)).toBe( + "if that declaration is wrong, patch it: scriptc types patch kit Profile.email '| undefined'", + ); + expect(patchSuggestion("@scope/kit", "Terminal.columns()", "| undefined", true)).toBe( + 'if that declaration is wrong, patch it: scriptc types patch @scope/kit "Terminal.columns()" "| undefined"', + ); +}); + +test.skipIf(process.platform === "win32")( + "the POSIX form passes each value as one argument", + async () => { + const dir = await mkdtemp(join(tmpdir(), "scriptc-decl-origin-")); + try { + for (const selector of hostile) { + const pkg = "evil;touch pkg"; + const command = patchSuggestion(pkg, selector, "| undefined", false).replace( + "if that declaration is wrong, patch it: scriptc types patch", + "printf '%s\\n'", + ); + const { stdout } = await exec("sh", ["-c", command], { cwd: dir }); + expect(stdout).toBe(`${pkg}\n${selector}\n| undefined\n`); + expect(await readdir(dir)).toEqual([]); + } + } finally { + await rm(dir, { recursive: true, force: true }); + } + }, +); + +test("the Windows form double-quotes values only where both shells keep them", () => { + expect(shellArgument("Profile.a;b&c|df", true)).toBe('"Profile.a;b&c|df"'); + for (const value of [ + "Profile.$(x)", + "Profile.`x`", + 'Profile."x"', + "Profile.%PATH%", + "Profile.!x!", + "Profile.“x”", + "Profile.‘x’", + "Profile.a\nb", + ]) + expect(shellArgument(value, true), value).toBeNull(); + expect(patchSuggestion("kit", "Profile.$(x)", "| undefined", true)).toBe( + "if that declaration is wrong, patch it with scriptc types patch", + ); +}); diff --git a/packages/runtime/src/scr_type_origin.c b/packages/runtime/src/scr_type_origin.c index cf9c6fc49..6ea872304 100644 --- a/packages/runtime/src/scr_type_origin.c +++ b/packages/runtime/src/scr_type_origin.c @@ -42,6 +42,65 @@ static void scr_origin_put(ScrOriginBuf *b, const char *s, size_t n) { static void scr_origin_puts(ScrOriginBuf *b, const char *s) { scr_origin_put(b, s, strlen(s)); } +/* One argument of the suggested `scriptc types patch` command, quoted as + * the shell passes it as written (decl-origin.ts's shellArgument quotes the + * same way): bare when it has only characters no shell treats specially; + * otherwise, for a POSIX shell, in single quotes with each ' as '\'', and + * on Windows (cmd and PowerShell), in double quotes when it has none of the + * characters either shell expands, escapes or ends a string with there. + * Package names, selectors and types come from package declarations. + * Returns false, writing nothing, when no quoting is inert. */ +static bool scr_origin_bare(const char *s, size_t n) { + if (n == 0) return false; + for (size_t i = 0; i < n; i++) { + unsigned char c = (unsigned char)s[i]; + bool plain = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || + strchr("_@+=:,./-", c) != NULL; +#ifndef _WIN32 + plain = plain || c == '%'; +#endif + if (!plain || c == 0) return false; + } + return true; +} + +static bool scr_origin_quotable(const char *s, size_t n) { +#ifdef _WIN32 + for (size_t i = 0; i < n; i++) { + unsigned char c = (unsigned char)s[i]; + if (c < 0x20 || c == 0x7f || (c != 0 && strchr("\"%!^$`", c) != NULL)) return false; + /* PowerShell also takes the typographic quotes U+2018-U+201F (UTF-8 + * E2 80 98..9F) as quotes. */ + if (c == 0xe2 && i + 2 < n && (unsigned char)s[i + 1] == 0x80 && + (unsigned char)s[i + 2] >= 0x98 && (unsigned char)s[i + 2] <= 0x9f) + return false; + } +#else + (void)s; + (void)n; +#endif + return true; +} + +static void scr_origin_put_arg(ScrOriginBuf *b, const char *s, size_t n) { + if (scr_origin_bare(s, n)) { + scr_origin_put(b, s, n); + return; + } +#ifdef _WIN32 + scr_origin_puts(b, "\""); + scr_origin_put(b, s, n); + scr_origin_puts(b, "\""); +#else + scr_origin_puts(b, "'"); + for (size_t i = 0; i < n; i++) { + if (s[i] == '\'') scr_origin_puts(b, "'\\''"); + else scr_origin_put(b, s + i, 1); + } + scr_origin_puts(b, "'"); +#endif +} + /* Field `index` of the origin string (not terminated: [*out, *out+len)). */ static size_t scr_origin_field(const char *origin, int index, const char **out) { const char *p = origin; @@ -116,29 +175,40 @@ void scr_type_origin_note(const char *origin) { } scr_origin_puts(&b, " ("); scr_origin_put(&b, where, where_len); - scr_origin_puts(&b, ")\n if that declaration is wrong, patch it: scriptc types patch "); - scr_origin_put(&b, pkg, pkg_len); - scr_origin_puts(&b, " \""); - scr_origin_put(&b, selector, selector_len); + scr_origin_puts(&b, ")\n "); + ScrOriginBuf sel = {0}, type = {0}; + scr_origin_put(&sel, selector, selector_len); /* $[3].email extends the selector as [].email: elements of an array * share one declared type. */ for (size_t i = 1; path != NULL && i < path_len; i++) { if (path[i] == '[') { - scr_origin_puts(&b, "[]"); + scr_origin_puts(&sel, "[]"); while (i < path_len && path[i] != ']') i++; } else { - scr_origin_put(&b, path + i, 1); + scr_origin_put(&sel, path + i, 1); } } - scr_origin_puts(&b, "\" \"| "); + scr_origin_puts(&type, "| "); if (got != NULL) { const char *arm = scr_origin_arm(got, got_len); - if (arm != NULL) scr_origin_puts(&b, arm); - else scr_origin_put(&b, got, got_len); + if (arm != NULL) scr_origin_puts(&type, arm); + else scr_origin_put(&type, got, got_len); } else { - scr_origin_puts(&b, "undefined"); + scr_origin_puts(&type, "undefined"); } - scr_origin_puts(&b, "\""); + if (scr_origin_quotable(pkg, pkg_len) && scr_origin_quotable(sel.data, sel.len) && + scr_origin_quotable(type.data, type.len)) { + scr_origin_puts(&b, "if that declaration is wrong, patch it: scriptc types patch "); + scr_origin_put_arg(&b, pkg, pkg_len); + scr_origin_puts(&b, " "); + scr_origin_put_arg(&b, sel.data, sel.len); + scr_origin_puts(&b, " "); + scr_origin_put_arg(&b, type.data, type.len); + } else { + scr_origin_puts(&b, "if that declaration is wrong, patch it with scriptc types patch"); + } + free(sel.data); + free(type.data); ScrStr *next = scr_str_new(b.data, b.len); free(b.data); diff --git a/packages/runtime/src/scr_type_origin.test.c b/packages/runtime/src/scr_type_origin.test.c new file mode 100644 index 000000000..bc6477d23 --- /dev/null +++ b/packages/runtime/src/scr_type_origin.test.c @@ -0,0 +1,24 @@ +/* Built by scr_type_origin.test.ts: a pending TypeError gains the + * declaration note for an origin whose package name and selector carry + * shell syntax; the test runs the suggested command through sh. */ +#include "scr_runtime.h" + +#include +#include + +int main(int argc, char **argv) { + if (argc != 3) return 2; + /* package, version, range, selector, where, declared type */ + char origin[1024]; + snprintf(origin, sizeof origin, "%s\x1f" "1.0.0\x1f^1.0.0\x1f%s\x1findex.d.ts:3\x1fstring", argv[1], + argv[2]); + const char *text = "expected string at $, got undefined"; + ScrExcCell *cell = scr_exc_current_cell(); + cell->kind = SCR_EXC_OBJ; + cell->payload = scr_error_new(SCR_ERR_TYPE, scr_str_new(text, strlen(text))); + scr_type_origin_note(origin); + ScrError *e = cell->payload; + fwrite(e->message->data, 1, e->message->len, stdout); + fputc('\n', stdout); + return 0; +} diff --git a/packages/runtime/src/scr_type_origin.test.ts b/packages/runtime/src/scr_type_origin.test.ts new file mode 100644 index 000000000..b31195deb --- /dev/null +++ b/packages/runtime/src/scr_type_origin.test.ts @@ -0,0 +1,69 @@ +/* The suggested `scriptc types patch` command on a checked-conversion + * TypeError (scr_type_origin.c) passes each value it carries as one + * argument: package names and selectors come from package declarations, + * and a command copied from the message must not run anything they spell. + * Run through sh with printf in place of scriptc, the command prints the + * package, the selector and the type exactly, and creates no file. */ +import { execFile } from "node:child_process"; +import { mkdtemp, readdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { promisify } from "node:util"; +import { expect, test } from "vitest"; +import { EXECUTABLE_RUNTIME_SOURCES } from "../../compiler/src/backend/native-toolchain.js"; + +const exec = promisify(execFile); + +const hostile = [ + ["kit", "Profile.email"], + ["evil;touch pkg", "Profile.a;touch semicolon"], + ["kit", "Profile.$(touch subst)"], + ["kit", "Profile.`touch backtick`"], + ["kit", "Profile.it's \"quoted\" \\ end'"], + ["kit", "Profile.a&&touch and|touch pipe>out"], +] as const; + +test.skipIf(process.platform === "win32")( + "the suggested patch command passes declaration values as single arguments", + { timeout: 300_000 }, + async () => { + const dir = await mkdtemp(join(tmpdir(), "scriptc-type-origin-")); + try { + const bin = join(dir, "origin"); + await exec("clang", [ + "-std=c11", + "-O1", + "-w", + "-pthread", + ...(process.platform === "linux" ? ["-D_GNU_SOURCE"] : []), + join(import.meta.dirname, "scr_type_origin.test.c"), + join(import.meta.dirname, "scr_type_origin.c"), + ...EXECUTABLE_RUNTIME_SOURCES.map((name) => join(import.meta.dirname, name)), + ...(process.platform === "linux" ? ["-lm"] : []), + "-o", + bin, + ]); + const sandbox = join(dir, "run"); + await exec("mkdir", [sandbox]); + const commands: string[] = []; + for (const [pkg, selector] of hostile) { + const { stdout } = await exec(bin, [pkg, selector]); + const line = stdout.split("\n").find((l) => l.includes("scriptc types patch")); + expect(line, stdout).toBeDefined(); + const command = line!.slice(line!.indexOf("scriptc types patch")); + commands.push(command); + const printed = await exec( + "sh", + ["-c", command.replace("scriptc types patch", "printf '%s\\n'")], + { cwd: sandbox }, + ); + expect(printed.stdout).toBe(`${pkg}\n${selector}\n| undefined\n`); + expect(await readdir(sandbox)).toEqual([]); + } + // Values without shell syntax stay bare. + expect(commands[0]).toBe("scriptc types patch kit Profile.email '| undefined'"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }, +); diff --git a/tests/harness/type-patches.test.ts b/tests/harness/type-patches.test.ts index c7975a36f..a2996d724 100644 --- a/tests/harness/type-patches.test.ts +++ b/tests/harness/type-patches.test.ts @@ -121,7 +121,7 @@ describe("type patches", () => { expect(native.stderr).toBe( "Uncaught TypeError: expected string at $, got undefined\n" + " profilekit@3.1.0 declares Profile.email as string (index.d.ts:7)\n" + - ` if that declaration is wrong, patch it: scriptc types patch profilekit "Profile.email" "| undefined"\n`, + ` if that declaration is wrong, patch it: scriptc types patch profilekit Profile.email '| undefined'\n`, ); } } finally { @@ -179,12 +179,12 @@ describe("type patches", () => { const result = await build(join(fixture, "unpatched/main.ts"), "unpatched", ["termctx"]); const native = await run(result.binaryPath, []); const patch = (selector: string) => - ` if that declaration is wrong, patch it: scriptc types patch termctx "${selector}" "| undefined"`; + ` if that declaration is wrong, patch it: scriptc types patch termctx ${selector} '| undefined'`; expect(native.stdout).toBe( [ "expected number at $, got undefined", " termctx@1.2.0 declares Terminal.columns() as number (index.js:16)", - patch("Terminal.columns()"), + patch("'Terminal.columns()'"), "undefined is not representable in the target union (a value narrowed or asserted past it still held it)", " termctx@1.2.0 declares OutputContext.width as number (index.js:24)", patch("OutputContext.width"), @@ -206,7 +206,7 @@ describe("type patches", () => { const failure = (pkg: string, version: string) => [ "undefined is not representable in the target union (a value narrowed or asserted past it still held it)", ` ${pkg}@${version} declares OutputContext.width as number (index.js:24)`, - ` if that declaration is wrong, patch it: scriptc types patch ${pkg} "OutputContext.width" "| undefined"`, + ` if that declaration is wrong, patch it: scriptc types patch ${pkg} OutputContext.width '| undefined'`, ]; expect(native.stdout).toBe( [...failure("panectx", "2.0.0"), ...failure("gridctx", "4.1.0"), ""].join("\n"),