fix(adapters): decode child stdout/stderr as streaming UTF-8
String(Buffer) is latin1, so adapter I/O and SSH text streams corrupted non-ASCII (and glyphs split across data events). Use Node's UTF-8 StringDecoder and keep binary SSH pipes unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
932c8bec56
commit
b229d2f99d
|
|
@ -580,6 +580,69 @@ describe("runChildProcess", () => {
|
|||
expect(result.stdout).toBe("done");
|
||||
});
|
||||
|
||||
it("captures multilingual UTF-8 stdout exactly", async () => {
|
||||
const expected = "مرحبا / Привет / 체크리스트 / 你好 / 😀";
|
||||
const result = await runChildProcess(
|
||||
randomUUID(),
|
||||
process.execPath,
|
||||
["-e", "process.stdout.write(process.env.PAPERCLIP_UTF8_PAYLOAD);"],
|
||||
{
|
||||
cwd: process.cwd(),
|
||||
env: { PAPERCLIP_UTF8_PAYLOAD: expected },
|
||||
timeoutSec: 10,
|
||||
graceSec: 1,
|
||||
onLog: async () => {},
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toBe(expected);
|
||||
expect(result.stdout).not.toContain("?");
|
||||
expect(result.stdout).not.toContain("\uFFFD");
|
||||
});
|
||||
|
||||
it("reassembles a 3-byte UTF-8 glyph split across stdout writes", async () => {
|
||||
const result = await runChildProcess(
|
||||
randomUUID(),
|
||||
process.execPath,
|
||||
[
|
||||
"-e",
|
||||
"const bytes = Buffer.from([0xe4, 0xbd, 0xa0]); process.stdout.write(bytes.subarray(0, 1)); process.stdout.write(bytes.subarray(1));",
|
||||
],
|
||||
{
|
||||
cwd: process.cwd(),
|
||||
env: {},
|
||||
timeoutSec: 10,
|
||||
graceSec: 1,
|
||||
onLog: async () => {},
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toBe("你");
|
||||
expect(result.stdout).not.toContain("\uFFFD");
|
||||
expect(result.stdout).not.toContain("?");
|
||||
});
|
||||
|
||||
it("leaves ASCII-only stdout unchanged", async () => {
|
||||
const result = await runChildProcess(
|
||||
randomUUID(),
|
||||
process.execPath,
|
||||
["-e", "process.stdout.write('hello world'); process.stderr.write('err');"],
|
||||
{
|
||||
cwd: process.cwd(),
|
||||
env: {},
|
||||
timeoutSec: 10,
|
||||
graceSec: 1,
|
||||
onLog: async () => {},
|
||||
},
|
||||
);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toBe("hello world");
|
||||
expect(result.stderr).toBe("err");
|
||||
});
|
||||
|
||||
it("waits for onSpawn before sending stdin to the child", async () => {
|
||||
const spawnDelayMs = 150;
|
||||
const startedAt = Date.now();
|
||||
|
|
|
|||
|
|
@ -4699,11 +4699,16 @@ export async function runChildProcess(
|
|||
}, opts.timeoutSec * 1000)
|
||||
: null;
|
||||
|
||||
child.stdout?.on("data", (chunk: unknown) => {
|
||||
// Decode as streaming UTF-8. A multibyte character can be split across
|
||||
// two `data` events, so per-chunk decoding corrupts it; the stream's
|
||||
// StringDecoder holds the incomplete sequence until the rest arrives.
|
||||
child.stdout?.setEncoding("utf8");
|
||||
child.stderr?.setEncoding("utf8");
|
||||
|
||||
child.stdout?.on("data", (text: string) => {
|
||||
const readable = child.stdout;
|
||||
if (!readable) return;
|
||||
readable.pause();
|
||||
const text = String(chunk);
|
||||
stdout = appendWithCap(stdout, text);
|
||||
maybeArmTerminalResultCleanup();
|
||||
logChain = logChain
|
||||
|
|
@ -4717,11 +4722,10 @@ export async function runChildProcess(
|
|||
});
|
||||
});
|
||||
|
||||
child.stderr?.on("data", (chunk: unknown) => {
|
||||
child.stderr?.on("data", (text: string) => {
|
||||
const readable = child.stderr;
|
||||
if (!readable) return;
|
||||
readable.pause();
|
||||
const text = String(chunk);
|
||||
stderr = appendWithCap(stderr, text);
|
||||
maybeArmTerminalResultCleanup();
|
||||
logChain = logChain
|
||||
|
|
|
|||
|
|
@ -0,0 +1,32 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import { createStreamingUtf8Accumulator } from "./ssh.js";
|
||||
|
||||
describe("createStreamingUtf8Accumulator", () => {
|
||||
it("keeps multilingual text exact across writes", () => {
|
||||
const expected = "مرحبا / Привет / 체크리스트 / 你好 / 😀";
|
||||
const acc = createStreamingUtf8Accumulator();
|
||||
acc.write(Buffer.from(expected, "utf8"));
|
||||
expect(acc.flush()).toBe(expected);
|
||||
expect(acc.flush()).not.toContain("?");
|
||||
expect(acc.flush()).not.toContain("\uFFFD");
|
||||
});
|
||||
|
||||
it("reassembles a 3-byte UTF-8 glyph split across Buffer chunks", () => {
|
||||
const bytes = Buffer.from("你", "utf8");
|
||||
expect(bytes.length).toBe(3);
|
||||
const acc = createStreamingUtf8Accumulator();
|
||||
acc.write(bytes.subarray(0, 1));
|
||||
expect(acc.text).toBe("");
|
||||
acc.write(bytes.subarray(1));
|
||||
expect(acc.flush()).toBe("你");
|
||||
expect(acc.flush()).not.toContain("\uFFFD");
|
||||
expect(acc.flush()).not.toContain("?");
|
||||
});
|
||||
|
||||
it("leaves ASCII-only output unchanged", () => {
|
||||
const acc = createStreamingUtf8Accumulator();
|
||||
acc.write(Buffer.from("hello world", "utf8"));
|
||||
acc.write(" and more");
|
||||
expect(acc.flush()).toBe("hello world and more");
|
||||
});
|
||||
});
|
||||
|
|
@ -4,6 +4,7 @@ import { constants as fsConstants, createReadStream, createWriteStream, promises
|
|||
import net from "node:net";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { StringDecoder } from "node:string_decoder";
|
||||
import { Transform } from "node:stream";
|
||||
import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js";
|
||||
import {
|
||||
|
|
@ -36,6 +37,50 @@ export interface SshCommandResult {
|
|||
stderr: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decode a Node readable stream as UTF-8 across `data` events.
|
||||
* Per-chunk `toString("utf8")` / `String(chunk)` corrupts a glyph whose
|
||||
* bytes are split across two events; StringDecoder holds the incomplete
|
||||
* sequence until the rest arrives. Call `flush()` once on end/close.
|
||||
*/
|
||||
export function createStreamingUtf8Accumulator() {
|
||||
const decoder = new StringDecoder("utf8");
|
||||
let text = "";
|
||||
let flushed = false;
|
||||
|
||||
const writeBuffer = (buf: Buffer) => {
|
||||
if (flushed || buf.length === 0) return;
|
||||
text += decoder.write(buf);
|
||||
};
|
||||
|
||||
return {
|
||||
get text() {
|
||||
return text;
|
||||
},
|
||||
write(chunk: unknown) {
|
||||
if (flushed) return;
|
||||
if (typeof chunk === "string") {
|
||||
writeBuffer(Buffer.from(chunk, "utf8"));
|
||||
return;
|
||||
}
|
||||
if (Buffer.isBuffer(chunk)) {
|
||||
writeBuffer(chunk);
|
||||
return;
|
||||
}
|
||||
if (chunk instanceof Uint8Array) {
|
||||
writeBuffer(Buffer.from(chunk.buffer, chunk.byteOffset, chunk.byteLength));
|
||||
}
|
||||
},
|
||||
flush() {
|
||||
if (!flushed) {
|
||||
flushed = true;
|
||||
text += decoder.end();
|
||||
}
|
||||
return text;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export interface SshRemoteExecutionSpec extends SshConnectionConfig {
|
||||
remoteCwd: string;
|
||||
}
|
||||
|
|
@ -234,16 +279,16 @@ async function spawnText(
|
|||
});
|
||||
|
||||
const maxBuffer = options.maxBuffer ?? 1024 * 128;
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
const stdoutAcc = createStreamingUtf8Accumulator();
|
||||
const stderrAcc = createStreamingUtf8Accumulator();
|
||||
let settled = false;
|
||||
let timedOut = false;
|
||||
|
||||
const finishReject = (error: Error & { stdout?: string; stderr?: string; code?: number | null; killed?: boolean }) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
error.stdout = stdout;
|
||||
error.stderr = stderr;
|
||||
error.stdout = stdoutAcc.flush();
|
||||
error.stderr = stderrAcc.flush();
|
||||
error.killed = timedOut;
|
||||
reject(error);
|
||||
};
|
||||
|
|
@ -252,13 +297,12 @@ async function spawnText(
|
|||
streamName: "stdout" | "stderr",
|
||||
chunk: unknown,
|
||||
) => {
|
||||
const text = String(chunk);
|
||||
if (streamName === "stdout") {
|
||||
stdout += text;
|
||||
} else {
|
||||
stderr += text;
|
||||
}
|
||||
if (Buffer.byteLength(stdout, "utf8") > maxBuffer || Buffer.byteLength(stderr, "utf8") > maxBuffer) {
|
||||
const acc = streamName === "stdout" ? stdoutAcc : stderrAcc;
|
||||
acc.write(chunk);
|
||||
if (
|
||||
Buffer.byteLength(stdoutAcc.text, "utf8") > maxBuffer ||
|
||||
Buffer.byteLength(stderrAcc.text, "utf8") > maxBuffer
|
||||
) {
|
||||
child.kill("SIGTERM");
|
||||
finishReject(Object.assign(new Error(`Process output exceeded maxBuffer of ${maxBuffer} bytes.`), {
|
||||
code: null,
|
||||
|
|
@ -307,6 +351,8 @@ async function spawnText(
|
|||
clearTimers();
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
const stdout = stdoutAcc.flush();
|
||||
const stderr = stderrAcc.flush();
|
||||
if (code === 0) {
|
||||
resolve({ stdout, stderr });
|
||||
return;
|
||||
|
|
@ -668,7 +714,7 @@ async function streamLocalFileToSsh(input: {
|
|||
stdio: ["pipe", "ignore", "pipe"],
|
||||
});
|
||||
|
||||
let sshStderr = "";
|
||||
const sshStderrAcc = createStreamingUtf8Accumulator();
|
||||
let settled = false;
|
||||
|
||||
const fail = (error: Error) => {
|
||||
|
|
@ -680,7 +726,7 @@ async function streamLocalFileToSsh(input: {
|
|||
};
|
||||
|
||||
ssh.stderr?.on("data", (chunk) => {
|
||||
sshStderr += String(chunk);
|
||||
sshStderrAcc.write(chunk);
|
||||
});
|
||||
source.on("error", fail);
|
||||
ssh.on("error", fail);
|
||||
|
|
@ -693,6 +739,7 @@ async function streamLocalFileToSsh(input: {
|
|||
ssh.on("close", (code) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
const sshStderr = sshStderrAcc.flush();
|
||||
if ((code ?? 0) !== 0) {
|
||||
reject(new Error(sshStderr.trim() || `ssh exited with code ${code ?? -1}`));
|
||||
return;
|
||||
|
|
@ -723,7 +770,7 @@ async function streamSshToLocalFile(input: {
|
|||
});
|
||||
const sink = createWriteStream(input.localFile, { mode: 0o600 });
|
||||
|
||||
let sshStderr = "";
|
||||
const sshStderrAcc = createStreamingUtf8Accumulator();
|
||||
let settled = false;
|
||||
|
||||
const fail = (error: Error) => {
|
||||
|
|
@ -741,7 +788,7 @@ async function streamSshToLocalFile(input: {
|
|||
ssh.stdout?.pipe(sink);
|
||||
}
|
||||
ssh.stderr?.on("data", (chunk) => {
|
||||
sshStderr += String(chunk);
|
||||
sshStderrAcc.write(chunk);
|
||||
});
|
||||
ssh.on("error", fail);
|
||||
sink.on("error", fail);
|
||||
|
|
@ -749,6 +796,7 @@ async function streamSshToLocalFile(input: {
|
|||
sink.end(() => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
const sshStderr = sshStderrAcc.flush();
|
||||
if ((code ?? 0) !== 0) {
|
||||
reject(new Error(sshStderr.trim() || `ssh exited with code ${code ?? -1}`));
|
||||
return;
|
||||
|
|
@ -1369,8 +1417,8 @@ export async function syncDirectoryToSsh(input: {
|
|||
stdio: ["pipe", "ignore", "pipe"],
|
||||
});
|
||||
|
||||
let tarStderr = "";
|
||||
let sshStderr = "";
|
||||
const tarStderrAcc = createStreamingUtf8Accumulator();
|
||||
const sshStderrAcc = createStreamingUtf8Accumulator();
|
||||
let settled = false;
|
||||
let tarExited = false;
|
||||
let sshExited = false;
|
||||
|
|
@ -1382,6 +1430,8 @@ export async function syncDirectoryToSsh(input: {
|
|||
return;
|
||||
}
|
||||
settled = true;
|
||||
const tarStderr = tarStderrAcc.flush();
|
||||
const sshStderr = sshStderrAcc.flush();
|
||||
if ((tarExitCode ?? 0) !== 0) {
|
||||
reject(new Error(tarStderr.trim() || `tar exited with code ${tarExitCode ?? -1}`));
|
||||
return;
|
||||
|
|
@ -1410,10 +1460,10 @@ export async function syncDirectoryToSsh(input: {
|
|||
tar.stdout?.pipe(ssh.stdin ?? null);
|
||||
}
|
||||
tar.stderr?.on("data", (chunk) => {
|
||||
tarStderr += String(chunk);
|
||||
tarStderrAcc.write(chunk);
|
||||
});
|
||||
ssh.stderr?.on("data", (chunk) => {
|
||||
sshStderr += String(chunk);
|
||||
sshStderrAcc.write(chunk);
|
||||
});
|
||||
|
||||
tar.on("error", fail);
|
||||
|
|
@ -1484,8 +1534,8 @@ export async function syncDirectoryFromSsh(input: {
|
|||
env: tarSpawnEnv(),
|
||||
});
|
||||
|
||||
let sshStderr = "";
|
||||
let tarStderr = "";
|
||||
const sshStderrAcc = createStreamingUtf8Accumulator();
|
||||
const tarStderrAcc = createStreamingUtf8Accumulator();
|
||||
let settled = false;
|
||||
let sshExited = false;
|
||||
let tarExited = false;
|
||||
|
|
@ -1495,6 +1545,8 @@ export async function syncDirectoryFromSsh(input: {
|
|||
const maybeFinish = () => {
|
||||
if (settled || !sshExited || !tarExited) return;
|
||||
settled = true;
|
||||
const sshStderr = sshStderrAcc.flush();
|
||||
const tarStderr = tarStderrAcc.flush();
|
||||
if ((sshExitCode ?? 0) !== 0) {
|
||||
reject(new Error(sshStderr.trim() || `ssh exited with code ${sshExitCode ?? -1}`));
|
||||
return;
|
||||
|
|
@ -1521,10 +1573,10 @@ export async function syncDirectoryFromSsh(input: {
|
|||
ssh.stdout?.pipe(tar.stdin ?? null);
|
||||
}
|
||||
ssh.stderr?.on("data", (chunk) => {
|
||||
sshStderr += String(chunk);
|
||||
sshStderrAcc.write(chunk);
|
||||
});
|
||||
tar.stderr?.on("data", (chunk) => {
|
||||
tarStderr += String(chunk);
|
||||
tarStderrAcc.write(chunk);
|
||||
});
|
||||
|
||||
ssh.on("error", fail);
|
||||
|
|
|
|||
Loading…
Reference in New Issue