From ac9978897c8bbdb0804efb520b836469c69994d5 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 12 Nov 2026 12:11:48 +0100 Subject: [PATCH 1/3] Keep multi-byte characters whole across execProcess output chunks execProcess decoded each read chunk of subprocess output on its own. Pipe reads come in at most 64 KiB, which is not a multiple of 3 or 4, so a multi-byte UTF-8 character straddling a chunk boundary was decoded as two U+FFFD replacement characters in both the returned text and what was printed. filteredAsyncIterator is now an async generator with a streaming decoder per stream, and merged output always routes both streams through it. The merge multiplexer therefore only carries whole characters, so a character pending on one stream cannot be spliced with bytes from the other. A chunk that holds only part of a character decodes to nothing and is skipped, so the stderr filter never receives an empty string (it would print bare colour escapes). processOutput decodes each chunk once, with streaming, and reuses that text for printing and accumulation. The respectStreams path still writes the child's raw bytes unchanged, so binary output such as `quarto pandoc -t docx -o -` is not affected. The regression tests spawn the current runtime through a new execPath() in deno_ral/process.ts, so they add no direct runtime calls. --- news/changelog-1.12.md | 1 + src/core/process.ts | 50 ++++++--- src/deno_ral/process.ts | 4 + tests/unit/exec-process-utf8-chunks.test.ts | 115 ++++++++++++++++++++ 4 files changed, 152 insertions(+), 18 deletions(-) create mode 100644 tests/unit/exec-process-utf8-chunks.test.ts diff --git a/news/changelog-1.12.md b/news/changelog-1.12.md index 9d1014a83a4..4645655562d 100644 --- a/news/changelog-1.12.md +++ b/news/changelog-1.12.md @@ -65,3 +65,4 @@ All changes included in 1.12: - ([#15271](https://github.com/quarto-dev/quarto-cli/issues/15271)): Translate the `tools-share` and `tools-download` language keys in Catalan, Chinese, Czech, Danish, Dutch, Finnish, French, German, Hebrew, Indonesian, Italian, Japanese, Korean, Lithuanian, Norwegian Bokmål, Norwegian Nynorsk, Polish, Portuguese, Russian, Slovak, Swedish, Turkish and Ukrainian, and `tools-share` in Serbian (Latin), using reviewed machine translation. - ([#15289](https://github.com/quarto-dev/quarto-cli/issues/15289)): Translate the `announcement-dismiss-label` language key, the accessible name of the close button of a website announcement, in Azerbaijani, Basque, Bulgarian, Catalan, Chinese (Simplified and Traditional), Czech, Dutch, Finnish, French, German, Hebrew, Icelandic, Indonesian, Italian, Japanese, Korean, Lithuanian, Norwegian Nynorsk, Polish, Portuguese, Russian, Serbian (Latin), Slovak, Slovenian, Spanish, Swedish, Turkish and Ukrainian, using reviewed machine translation. - ([#15325](https://github.com/quarto-dev/quarto-cli/issues/15325)): Validate and offer completion for `column`, `fig-column` and `tbl-column` in document front matter. They already took effect there, but an invalid value was silently accepted. +- ([#15341](https://github.com/quarto-dev/quarto-cli/issues/15341)): Fix non-ASCII characters in large output from external tools such as Pandoc, knitr or LaTeX occasionally turning into `�`. diff --git a/src/core/process.ts b/src/core/process.ts index 13e0c12afa9..357d8ea8cb0 100644 --- a/src/core/process.ts +++ b/src/core/process.ts @@ -4,7 +4,7 @@ * Copyright (C) 2020-2022 Posit Software, PBC */ -import { MuxAsyncIterator, pooledMap } from "async"; +import { MuxAsyncIterator } from "async"; import { debug, info } from "../deno_ral/log.ts"; import { onCleanup } from "./cleanup.ts"; import { ProcessResult } from "./process-types.ts"; @@ -129,10 +129,7 @@ export async function execProcess( iterator: AsyncIterableIterator>, filter?: (output: string) => string, ) => { - const streamIter = filter - ? filteredAsyncIterator(iterator, filter) - : iterator; - multiplexIterator.add(streamIter); + multiplexIterator.add(filteredAsyncIterator(iterator, filter)); }; addStream(process.stdout.values()); @@ -227,17 +224,27 @@ export function processSuccessResult(): ProcessResult { }; } -function filteredAsyncIterator( - iterator: AsyncIterableIterator, - filter: (output: string) => string, -): AsyncIterableIterator { +// Re-encodes a byte stream so each yielded chunk holds whole characters: a +// multi-byte character split across two reads is held back until complete. +// Chunks that decode to no text yet are skipped, so the filter never sees "". +async function* filteredAsyncIterator( + iterator: AsyncIterable, + filter?: (output: string) => string, +): AsyncGenerator { const encoder = new TextEncoder(); const decoder = new TextDecoder(); - return pooledMap(1, iterator, (data: Uint8Array) => { - return Promise.resolve( - encoder.encode(filter(decoder.decode(data))), - ); - }); + const encode = (text: string) => + encoder.encode(filter ? filter(text) : text); + for await (const chunk of iterator) { + const text = decoder.decode(chunk, { stream: true }); + if (text !== "") { + yield encode(text); + } + } + const tail = decoder.decode(); + if (tail !== "") { + yield encode(tail); + } } // Processes ouptut from an interator (stderr, stdout, etc...) @@ -248,18 +255,25 @@ async function processOutput( ): Promise { const decoder = new TextDecoder(); let outputText = ""; + const accept = (text: string) => { + if (text === "") { + return; + } + if ((output === "inherit" || output === undefined) && !which) { + info(text, { newline: false }); + } + outputText += text; + }; for await (const chunk of iterator) { if (output === "inherit" || output === undefined) { if (which === "stdout") { Deno.stdout.writeSync(chunk); } else if (which === "stderr") { Deno.stderr.writeSync(chunk); - } else { - info(decoder.decode(chunk), { newline: false }); } } - const text = decoder.decode(chunk); - outputText += text; + accept(decoder.decode(chunk, { stream: true })); } + accept(decoder.decode()); return outputText; } diff --git a/src/deno_ral/process.ts b/src/deno_ral/process.ts index 83fe5067cdd..d28f8d98081 100644 --- a/src/deno_ral/process.ts +++ b/src/deno_ral/process.ts @@ -8,3 +8,7 @@ export const call = async (path: string, opts: Deno.CommandOptions = {}) => { const cmd = new Deno.Command(path, opts); return cmd.output(); }; + +export function execPath(): string { + return Deno.execPath(); +} diff --git a/tests/unit/exec-process-utf8-chunks.test.ts b/tests/unit/exec-process-utf8-chunks.test.ts new file mode 100644 index 00000000000..77d54c8a734 --- /dev/null +++ b/tests/unit/exec-process-utf8-chunks.test.ts @@ -0,0 +1,115 @@ +/* + * exec-process-utf8-chunks.test.ts + * + * Regression test for #15341: execProcess must keep a multi-byte UTF-8 + * character whole when the subprocess output splits it across two read + * chunks. The child writes a long run of 3-byte characters, so read + * boundaries (64 KiB is not a multiple of 3) fall inside a character. + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { unitTest } from "../test.ts"; +import { assertEquals } from "../asserts.ts"; +import { execProcess } from "../../src/core/process.ts"; +import { execPath } from "../../src/deno_ral/process.ts"; + +const kChar = "“"; +const kCount = 100_000; +const expected = kChar.repeat(kCount); + +// Child-side helpers: write all bytes to an fd, and sleep between writes. +const prelude = `import { writeSync } from "node:fs"; +const w = (fd, b) => { let o = 0; while (o < b.length) o += writeSync(fd, b, o); }; +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +`; + +const childCmd = (script: string) => ({ + cmd: execPath(), + args: ["eval", prelude + script], + stdout: "piped" as const, + stderr: "piped" as const, +}); + +const largeCmd = (fd: 1 | 2) => + childCmd(`w(${fd}, new TextEncoder().encode("${kChar}".repeat(${kCount})));`); + +// 3 rounds: stdout [E2]; stderr "”"; stdout [80 9C]. Each stdout character +// is split across two writes with the other stream's output in between. +const interleavedCmd = childCmd(`for (let i = 0; i < 3; i++) { + w(1, Uint8Array.of(0xE2)); + await sleep(100); + w(2, new TextEncoder().encode("”")); + await sleep(100); + w(1, Uint8Array.of(0x80, 0x9C)); + await sleep(100); +}`); + +const partialStderrCmd = childCmd(`w(2, Uint8Array.of(0xE2, 0x80));`); + +const truncatedCmd = childCmd(`w(1, Uint8Array.of(0x61, 0xE2, 0x80));`); + +unitTest("execProcess - stdout keeps characters split across chunks", async () => { + const result = await execProcess(largeCmd(1)); + assertEquals(result.stdout, expected); +}); + +unitTest("execProcess - stderr keeps characters split across chunks", async () => { + const result = await execProcess(largeCmd(2)); + assertEquals(result.stderr, expected); +}); + +unitTest( + "execProcess - filtered stderr keeps characters split across chunks", + async () => { + const result = await execProcess( + largeCmd(2), + undefined, + undefined, + (output) => output, + ); + assertEquals(result.stderr, expected); + }, +); + +unitTest( + "execProcess - merged output keeps characters split across chunks", + async () => { + const result = await execProcess( + largeCmd(2), + undefined, + "stderr>stdout", + (output) => output, + ); + assertEquals(result.stdout, expected); + }, +); + +unitTest( + "execProcess - merged output interleaving streams keeps characters whole", + async () => { + const result = await execProcess(interleavedCmd, undefined, "stderr>stdout"); + // Mux order across sources is not a contract, so compare sorted characters. + assertEquals([...result.stdout!].sort().join(""), "“““”””"); + }, +); + +unitTest("execProcess - filter never sees empty text", async () => { + const seen: string[] = []; + const result = await execProcess( + partialStderrCmd, + undefined, + undefined, + (s) => { + seen.push(s); + return s; + }, + ); + assertEquals(seen, ["�"]); + assertEquals(result.stderr, "�"); +}); + +unitTest("execProcess - truncated tail decodes to one U+FFFD", async () => { + const result = await execProcess(truncatedCmd); + assertEquals(result.stdout, "a�"); +}); From 9202f99e63f34b06f498e404c586f45f03f943eb Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 12 Nov 2026 12:12:56 +0100 Subject: [PATCH 2/3] Record #15341 impact on runtime-migration PRs 08, 28, 23 The merged execProcess UTF-8 fix changes three assumptions in unexecuted runtime-migration plans: PR 08's replacement processOutput and pooledMap import would revert it, PR 28's merged-mode addStream snippet would drop the per-stream decoding, and the new test starts its child with a Deno-only eval invocation that PR 23's inventory must list. Each executor reads these rows at step 0. --- dev-docs/runtime-migration/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev-docs/runtime-migration/README.md b/dev-docs/runtime-migration/README.md index d434c132513..d62c0037d29 100644 --- a/dev-docs/runtime-migration/README.md +++ b/dev-docs/runtime-migration/README.md @@ -152,6 +152,9 @@ The rows below are the ones that still need a named person or an external answer | PR 00c, Phase 1 plans, PRs 33–37 and tooling PR 38, release correction | Decided 2026-11-03: boundary work and the script notice target 1.12 on Deno; Node and script/extension changes target 1.13; bundled-Deno removal targets 1.14. In 00c, change "Starting with Quarto 1.12" to 1.13 in text and tests and publish the migration page before the 1.12 notice. Unlanded Phase 0/1 user-visible changes use `changelog-1.12.md`; PRs 33–36 use `changelog-1.13.md`, PR 37 uses `changelog-1.14.md`, and later tooling changes use their actual landing release. Shift the supported-surface matrix, compatibility warnings, retirement dates and soak from 1.12/1.13 to 1.13/1.14; rollback targets the last stable Deno release, expected 1.12. Do not change shipped 1.11 entries or historical evidence. | coordinator (plan refresh), release manager (cut dates) | step 0 of each affected plan; PR 09 review | | Any plan that type-checks `src/quarto.ts` (from PR 09 Task 3, 2026-10-28) | the command `deno check --unstable-kv --unstable-ffi --import-map=import_map.json quarto.ts` fails on Deno 2.7.14 and on 2.9.8 (re-checked 2026-11-03) with `Requires import access to "cdn.skypack.dev:443"` (a remote import in `src/core/yaml.ts:31`); add `--allow-import` | coordinator | step 0 of each | | PR 27 (from PR 09 Task 3, 2026-10-28) | The Deno types (2.7.14, and 2.9.8 re-checked 2026-11-03) make `Deno.SymlinkOptions.type` required and `Deno.truncateSync` string-only, so the layer's `symlink[Sync]` forwards an options object only when a `type` is given and `truncateSync` converts a URL with `toPathString`; the node:fs reimplementation keeps both behaviours | coordinator | PR 27 step 0 | +| PR 08 Task 1 and Task 2 step 2 (from #15341, the `execProcess` UTF-8 fix on `main`) | Task 1's premise that `src/deno_ral/process.ts` is 10 lines (`08-ral-process.md:25`) is stale: `execPath(): string` already exists there with the planned signature. Task 2 step 2 imports `pooledMap` (`:933`) and its `processOutput` snippet (`:1047-1066`) restores the per-chunk body that decodes each chunk twice, which would revert the fix. Re-derive the snippet from the current `processOutput` (decode once with `{ stream: true }`, flush at the end, skip empty text) and make `writeStdoutSync`/`writeStderrSync` the only change; drop `pooledMap` from the import as `filteredAsyncIterator` no longer uses it. Add `unit/exec-process-utf8-chunks.test.ts` to Task 2's verification | PR 08 executor | PR 08 step 0 | +| PR 28 Task 4 step 3 (from #15341) | the merged-mode `addStream` snippet (`28-ral-process-on-node-apis.md:894-900`) wraps only filtered streams (`filter ? filteredAsyncIterator(iterator, filter) : iterator`), which drops the per-stream decoding that keeps a character split across chunks whole when both streams are merged. Keep `filteredAsyncIterator(iterator, filter)` on both streams, and add `unit/exec-process-utf8-chunks.test.ts` to the PR's gate | PR 28 executor | PR 28 step 0 | +| PRs 23 and 31 (from #15341) | `tests/unit/exec-process-utf8-chunks.test.ts` starts its child as `execPath() eval '