mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(sandbox): decode Git Bash output as UTF-8 so CJK output is not garbled (#6548)
* fix(sandbox): decode Git Bash output as UTF-8 so CJK tools are not garbled Git Bash/MSYS always writes UTF-8 to its pipes, but the MSYS branch of LocalSandbox.execute_command called _run_windows_command without an encoding, so the capture fell back to locale.getpreferredencoding(). On a non-UTF-8 host (GBK on zh-CN Windows) that decodes UTF-8 bytes with the host code page and mangles every non-ASCII tool result. The pipe decoder replaces instead of raising, so the corruption is silent. The PowerShell branch right next to it has pinned UTF-8 since #5440. Decode the MSYS branch as UTF-8 too, and pin the behaviour in tests/test_local_sandbox_encoding.py: a unit test for the decoder choice and a live round-trip through a real Git Bash (skipped when it is absent). * fix(sandbox): keep locale fallback for native-child output under Git Bash Git Bash/MSYS writes its own output as UTF-8, but Windows-native children (python.exe, CLI shims) inherit the pipe and keep writing the host code page. Forcing UTF-8 with errors=replace turned that output into U+FFFD. Decode the captured buffer as UTF-8 first and retry with the locale when it is not valid UTF-8, so both MSYS UTF-8 output and native-child code-page output survive. The PowerShell branch is unchanged (its own output is pinned to UTF-8 by the preamble); the fallback is only wired on the MSYS path. Adds native-Python CJK regressions through execute_command. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Jason Cat
GitHub
parent
927ecb756f
commit
cee0d93b60
@ -40,16 +40,34 @@ class _BoundedPipeCapture:
|
||||
*,
|
||||
limit_bytes: int = _COMMAND_CAPTURE_LIMIT_BYTES,
|
||||
encoding: str = "utf-8",
|
||||
fallback_encoding: str | None = None,
|
||||
normalize_newlines: bool = False,
|
||||
) -> None:
|
||||
self._limit_bytes = limit_bytes
|
||||
self._encoding = encoding
|
||||
self._fallback_encoding = fallback_encoding
|
||||
self._normalize_newlines = normalize_newlines
|
||||
self._chunks: list[bytes] = []
|
||||
self._kept_bytes = 0
|
||||
self._total_bytes = 0
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _decode(self, data: bytes) -> str:
|
||||
"""Decode captured bytes, preferring the primary encoding when valid.
|
||||
|
||||
A forced ``encoding`` describes the shell's *own* output. Windows-native
|
||||
children (``python.exe``, CLI shims) inherit the pipe and keep writing the
|
||||
host code page, which is not valid in the forced encoding. Rather than
|
||||
replacing every such byte with U+FFFD, retry the whole buffer with
|
||||
``fallback_encoding`` (the locale) when the primary decode fails.
|
||||
"""
|
||||
if self._fallback_encoding is None:
|
||||
return data.decode(self._encoding, errors="replace")
|
||||
try:
|
||||
return data.decode(self._encoding)
|
||||
except UnicodeDecodeError:
|
||||
return data.decode(self._fallback_encoding, errors="replace")
|
||||
|
||||
def append(self, chunk: bytes) -> None:
|
||||
with self._lock:
|
||||
self._total_bytes += len(chunk)
|
||||
@ -67,7 +85,7 @@ class _BoundedPipeCapture:
|
||||
total_bytes = self._total_bytes
|
||||
kept_bytes = self._kept_bytes
|
||||
|
||||
output = data.decode(self._encoding, errors="replace")
|
||||
output = self._decode(data)
|
||||
if self._normalize_newlines:
|
||||
# Match ``subprocess.run(..., text=True)``: text streams use universal
|
||||
# newlines, translating both CRLF and bare CR to LF.
|
||||
@ -191,9 +209,10 @@ class LocalSandbox(Sandbox):
|
||||
name: str,
|
||||
*,
|
||||
encoding: str = "utf-8",
|
||||
fallback_encoding: str | None = None,
|
||||
normalize_newlines: bool = False,
|
||||
) -> tuple[_BoundedPipeCapture, threading.Thread]:
|
||||
capture = _BoundedPipeCapture(encoding=encoding, normalize_newlines=normalize_newlines)
|
||||
capture = _BoundedPipeCapture(encoding=encoding, fallback_encoding=fallback_encoding, normalize_newlines=normalize_newlines)
|
||||
thread = threading.Thread(target=LocalSandbox._drain_pipe, args=(fd, capture), name=name, daemon=True)
|
||||
thread.start()
|
||||
return capture, thread
|
||||
@ -561,7 +580,24 @@ class LocalSandbox(Sandbox):
|
||||
"MSYS2_ARG_CONV_EXCL": exclusions,
|
||||
}
|
||||
|
||||
if self._is_powershell(shell):
|
||||
if self._is_msys_shell(shell):
|
||||
# Git Bash/MSYS writes its own output as UTF-8, so the host code
|
||||
# page (GBK on zh-CN) must not decode it. Windows-native children
|
||||
# it spawns (python.exe, CLI shims) inherit the pipe and keep
|
||||
# writing the host code page, so a buffer that is not valid UTF-8
|
||||
# falls back to the locale instead of being replaced wholesale.
|
||||
stdout, stderr, returncode, timed_out = self._run_windows_command(
|
||||
args,
|
||||
timeout,
|
||||
sandbox_env,
|
||||
encoding="utf-8",
|
||||
fallback_encoding=locale.getpreferredencoding(False),
|
||||
)
|
||||
elif self._is_powershell(shell):
|
||||
# PowerShell is pinned to UTF-8 by the preamble above, so its own
|
||||
# output must be decoded as UTF-8: any other code page (GBK on
|
||||
# zh-CN) mangles non-ASCII output because the pipe decoder
|
||||
# replaces instead of raising.
|
||||
stdout, stderr, returncode, timed_out = self._run_windows_command(args, timeout, sandbox_env, encoding="utf-8")
|
||||
else:
|
||||
stdout, stderr, returncode, timed_out = self._run_windows_command(args, timeout, sandbox_env)
|
||||
@ -593,6 +629,7 @@ class LocalSandbox(Sandbox):
|
||||
env: dict[str, str] | None = None,
|
||||
*,
|
||||
encoding: str | None = None,
|
||||
fallback_encoding: str | None = None,
|
||||
) -> tuple[str, str, int, bool]:
|
||||
"""Run with bounded capture, a process-tree timeout, and locale decoding unless overridden."""
|
||||
timed_out = False
|
||||
@ -630,12 +667,14 @@ class LocalSandbox(Sandbox):
|
||||
stdout_read_fd,
|
||||
"deerflow-bash-stdout-drain",
|
||||
encoding=encoding,
|
||||
fallback_encoding=fallback_encoding,
|
||||
normalize_newlines=True,
|
||||
)
|
||||
stderr_capture, stderr_thread = LocalSandbox._start_pipe_drain(
|
||||
stderr_read_fd,
|
||||
"deerflow-bash-stderr-drain",
|
||||
encoding=encoding,
|
||||
fallback_encoding=fallback_encoding,
|
||||
normalize_newlines=True,
|
||||
)
|
||||
|
||||
|
||||
@ -5,6 +5,7 @@ import subprocess
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
from support.shell import require_posix_sh
|
||||
|
||||
import deerflow.sandbox.local.local_sandbox as local_sandbox
|
||||
from deerflow.sandbox.local.local_sandbox import LocalSandbox, PathMapping, _BoundedPipeCapture
|
||||
@ -23,6 +24,31 @@ def test_bounded_pipe_capture_decodes_non_utf8_output_with_configured_encoding()
|
||||
assert capture.read() == "caf\u00e9"
|
||||
|
||||
|
||||
def test_bounded_pipe_capture_falls_back_to_locale_for_non_utf8_bytes():
|
||||
"""A forced UTF-8 decode keeps the locale as a guarded fallback so output
|
||||
from native children (which still use the host code page) survives."""
|
||||
capture = _BoundedPipeCapture(encoding="utf-8", fallback_encoding="cp936")
|
||||
capture.append("\u4f60\u597d".encode("cp936"))
|
||||
|
||||
assert capture.read() == "\u4f60\u597d"
|
||||
|
||||
|
||||
def test_bounded_pipe_capture_prefers_primary_encoding_when_valid():
|
||||
capture = _BoundedPipeCapture(encoding="utf-8", fallback_encoding="cp936")
|
||||
capture.append("\u4f60\u597d".encode("utf-8"))
|
||||
|
||||
assert capture.read() == "\u4f60\u597d"
|
||||
|
||||
|
||||
def test_bounded_pipe_capture_without_fallback_replaces_non_utf8_bytes():
|
||||
capture = _BoundedPipeCapture(encoding="utf-8")
|
||||
capture.append("\u4f60\u597d".encode("cp936"))
|
||||
|
||||
decoded = capture.read()
|
||||
assert "\ufffd" in decoded
|
||||
assert "\u4f60\u597d" not in decoded
|
||||
|
||||
|
||||
def test_bounded_pipe_capture_applies_text_mode_newline_normalization_when_enabled():
|
||||
capture = _BoundedPipeCapture(normalize_newlines=True)
|
||||
capture.append(b"crlf\r\nbare-cr\rlf\n")
|
||||
@ -92,6 +118,76 @@ assert output == expected + "\n\nStd Error:\n" + expected + "\n\nExit Code: 3",
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="Requires a real Windows Git Bash/MSYS shell")
|
||||
def test_windows_msys_cjk_roundtrip():
|
||||
"""Git Bash/MSYS always emits UTF-8, so the host code page must not decode it."""
|
||||
shell = require_posix_sh()
|
||||
if not LocalSandbox._is_msys_shell(shell):
|
||||
pytest.skip(f"{shell} is not recognised as a Git Bash/MSYS shell")
|
||||
probe = r"""
|
||||
import sys
|
||||
import deerflow.sandbox.local.local_sandbox as local_sandbox
|
||||
from deerflow.sandbox.local.local_sandbox import LocalSandbox
|
||||
|
||||
# Keep this regression effective even on an English or UTF-8 Windows runner.
|
||||
local_sandbox.locale.getpreferredencoding = lambda _: "cp936"
|
||||
LocalSandbox._get_shell = staticmethod(lambda: sys.argv[1])
|
||||
expected = "\u4f60\u597d \u65e5\u672c\u8a9e"
|
||||
output = LocalSandbox("encoding-probe").execute_command(f"printf '%s\n' '{expected}'", timeout=15)
|
||||
assert output == expected + "\n", ascii(output)
|
||||
"""
|
||||
env = {**os.environ, "PYTHONUTF8": "0"}
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", probe, shell],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
env=env,
|
||||
timeout=30,
|
||||
check=False,
|
||||
)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="Requires a real Windows Git Bash/MSYS shell")
|
||||
def test_windows_msys_native_child_cjk_roundtrip():
|
||||
"""A Windows-native child of Git Bash (python.exe) inherits the pipe and keeps
|
||||
writing the host code page; the forced UTF-8 decode must fall back to the
|
||||
locale for those bytes instead of replacing the text with U+FFFD."""
|
||||
shell = require_posix_sh()
|
||||
if not LocalSandbox._is_msys_shell(shell):
|
||||
pytest.skip(f"{shell} is not recognised as a Git Bash/MSYS shell")
|
||||
probe = r"""
|
||||
import sys
|
||||
import deerflow.sandbox.local.local_sandbox as local_sandbox
|
||||
from deerflow.sandbox.local.local_sandbox import LocalSandbox
|
||||
|
||||
# Pin the fallback code page so the regression holds on non-zh-CN runners.
|
||||
local_sandbox.locale.getpreferredencoding = lambda _: "cp936"
|
||||
LocalSandbox._get_shell = staticmethod(lambda: sys.argv[1])
|
||||
expected = "\u4f60\u597d \u65e5\u672c\u8a9e"
|
||||
interpreter = sys.executable.replace("\\", "/")
|
||||
command = f"'{interpreter}' -c \"print('{expected}')\""
|
||||
# PYTHONIOENCODING makes the native child emit CP936 even on a UTF-8 host,
|
||||
# mirroring a zh-CN host's native Python writing to a redirected pipe.
|
||||
output = LocalSandbox("encoding-probe").execute_command(command, timeout=30, env={"PYTHONIOENCODING": "cp936"})
|
||||
assert output == expected + "\n", ascii(output)
|
||||
"""
|
||||
env = {**os.environ, "PYTHONUTF8": "0"}
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", probe, shell],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
encoding="utf-8",
|
||||
errors="replace",
|
||||
env=env,
|
||||
timeout=60,
|
||||
check=False,
|
||||
)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="POSIX capture semantics")
|
||||
def test_posix_command_capture_preserves_newlines():
|
||||
stdout, stderr, returncode, timed_out = LocalSandbox._run_posix_command(
|
||||
@ -293,10 +389,37 @@ def test_execute_command_forces_utf8_console_for_powershell_cjk_output(monkeypat
|
||||
assert calls[0][3] == "utf-8"
|
||||
|
||||
|
||||
def test_execute_command_forces_utf8_decoding_with_locale_fallback_for_msys(monkeypatch):
|
||||
"""Git Bash/MSYS writes UTF-8 to its pipes; decoding with the host code page
|
||||
(GBK on zh-CN) mangles CJK output silently because the pipe decoder replaces
|
||||
instead of raising. The native children Git Bash spawns still write the host
|
||||
code page, so the forced UTF-8 decode keeps the locale as a guarded fallback."""
|
||||
calls: list[tuple[list[str], float, dict[str, str], str | None, str | None]] = []
|
||||
|
||||
def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None):
|
||||
calls.append((args, timeout, env, encoding, fallback_encoding))
|
||||
return "你好", "", 0, False
|
||||
|
||||
monkeypatch.setattr(local_sandbox.os, "name", "nt")
|
||||
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Program Files\Git\bin"})
|
||||
monkeypatch.setattr(local_sandbox.locale, "getpreferredencoding", lambda _: "cp936")
|
||||
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Program Files\Git\bin\sh.exe"))
|
||||
monkeypatch.setattr(LocalSandbox, "_run_windows_command", staticmethod(fake_run))
|
||||
|
||||
output = LocalSandbox("t").execute_command("echo 你好")
|
||||
|
||||
assert output == "你好"
|
||||
assert calls[0][0] == [r"C:\Program Files\Git\bin\sh.exe", "-c", "echo 你好"]
|
||||
assert calls[0][3] == "utf-8"
|
||||
# Native children of Git Bash keep writing the host code page; the locale is
|
||||
# retained as a guarded fallback instead of forcing every byte through UTF-8.
|
||||
assert calls[0][4] == "cp936"
|
||||
|
||||
|
||||
def test_execute_command_keeps_msys_path_conversion_for_host_commands_on_windows(monkeypatch):
|
||||
calls: list[tuple[list[str], float, dict[str, str]]] = []
|
||||
|
||||
def fake_run(args, timeout, env):
|
||||
def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None):
|
||||
calls.append((args, timeout, env))
|
||||
return "ok", "", 0, False
|
||||
|
||||
@ -324,7 +447,7 @@ def test_execute_command_keeps_msys_path_conversion_for_host_commands_on_windows
|
||||
def test_execute_command_scopes_msys_path_conversion_exclusions_on_windows(monkeypatch):
|
||||
calls: list[tuple[list[str], float, dict[str, str]]] = []
|
||||
|
||||
def fake_run(args, timeout, env):
|
||||
def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None):
|
||||
calls.append((args, timeout, env))
|
||||
return "ok", "", 0, False
|
||||
|
||||
@ -346,7 +469,7 @@ def test_execute_command_scopes_msys_path_conversion_exclusions_on_windows(monke
|
||||
def test_execute_command_ignores_root_msys_mapping_for_host_commands_on_windows(monkeypatch):
|
||||
calls: list[tuple[list[str], float, dict[str, str]]] = []
|
||||
|
||||
def fake_run(args, timeout, env):
|
||||
def fake_run(args, timeout, env, *, encoding=None, fallback_encoding=None):
|
||||
calls.append((args, timeout, env))
|
||||
return "ok", "", 0, False
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user