Repository navigation
Fix non-ASCII characters garbled in large output from external tools - #15346
Merged
Merged
Conversation
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.
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.
The cited line 25 is in PR 08's context section, not Task 1. The snippet replaces only the signature and raw writes, so applied over the current processOutput it would print inherited output twice as well as revert the streaming decode; the row now says so.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
3 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an external tool run through
execProcess(Pandoc, knitr, LaTeX, ...) writes a lot of non-ASCII output, a character can come back as two�. #15341 found this withquarto dev-call build-artifacts, where the CSLevent-placedescription sent through Pandoc ended up garbled in the generated editor tooling files.Root cause
Subprocess output is read in chunks of up to 64 KiB, and
execProcessdecoded each chunk on its own. A UTF-8 character straddling two chunks was decoded as two replacement characters. This affects piped stdout and stderr, stderr with a filter, and merged output, including the knitr output printed to the console during render.Fix
Output is now decoded with a streaming
TextDecoder, which holds back an incomplete character until the next chunk completes it, and flushes at the end of the stream.filteredAsyncIteratoris now a generator with one decoder per stream. A chunk holding only part of a character yields nothing, so the stderr filter never receives an empty string.processOutputdecodes each chunk once and uses that text both for printing and for the returned result. It used to decode the same chunk twice when printing inherited output, which a streaming decoder can't do.quarto pandoc,quarto run), the raw bytes are still written as is, so binary output likequarto pandoc -t docx -o -is unaffected.The new unit tests start the child process through a new
execPath()insrc/deno_ral/process.tsrather than calling the Deno API directly.dev-docs/runtime-migration/README.mdrecords where the runtime-migration plans that rewriteprocess.tsneed to account for this change.Fixes #15341
Checklist
I have (if applicable):
AI-assisted PR
Note: autonomous AI agents submitting PRs without human oversight are not permitted — see the Code of Conduct.