fix(environments): use incremental UTF-8 decoder in select-based drain
The first draft of the fix called `chunk.decode("utf-8")` directly on
each 4096-byte `os.read()` result, which corrupts output whenever a
multi-byte UTF-8 character straddles a read boundary:
* `UnicodeDecodeError` fires on the valid-but-truncated byte sequence.
* The except handler clears ALL previously-decoded output and replaces
the whole buffer with `[binary output detected ...]`.
Empirically: 10000 '日' chars (30001 bytes) through the wrapper loses
all 10000 characters on the first draft; the baseline TextIOWrapper
drain (which uses `encoding='utf-8', errors='replace'` on Popen)
preserves them all. This regression affects any command emitting
non-ASCII output larger than one chunk — CJK/Arabic/emoji in
`npm install`, `pip install`, `docker logs`, `kubectl logs`, etc.
Fix: swap to `codecs.getincrementaldecoder('utf-8')(errors='replace')`,
which buffers partial multi-byte sequences across chunks and substitutes
U+FFFD for genuinely invalid bytes. Flush on drain exit via
`decoder.decode(b'', final=True)` to emit any trailing replacement
character for a dangling partial sequence.
Adds two regression tests:
* test_utf8_multibyte_across_read_boundary — 10000 U+65E5 chars,
verifies count round-trips and no fallback fires.
* test_invalid_utf8_uses_replacement_not_fallback — deliberate
\xff\xfe between valid ASCII, verifies surrounding text survives.
This commit is contained in:
@@ -109,3 +109,46 @@ class TestBackgroundChildDoesNotHang:
|
||||
assert "日本語" in result["output"]
|
||||
assert "café" in result["output"]
|
||||
assert "résumé" in result["output"]
|
||||
|
||||
def test_utf8_multibyte_across_read_boundary(self, local_env):
|
||||
"""Multibyte UTF-8 characters straddling a 4096-byte ``os.read()`` boundary
|
||||
must be decoded correctly via the incremental decoder — not lost to a
|
||||
``UnicodeDecodeError`` fallback. Regression for a bug in the first draft
|
||||
of the fix where a strict ``bytes.decode('utf-8')`` on each raw chunk
|
||||
wiped the entire buffer as soon as any chunk split a multi-byte char.
|
||||
"""
|
||||
# 10000 "日" chars = 30000 bytes — guaranteed to cross multiple 4096
|
||||
# read boundaries, and most boundaries will land in the middle of the
|
||||
# 3-byte UTF-8 encoding of U+65E5.
|
||||
cmd = (
|
||||
'python3 -c \'import sys; '
|
||||
'sys.stdout.buffer.write(chr(0x65e5).encode("utf-8") * 10000); '
|
||||
'sys.stdout.buffer.write(b"\\n")\''
|
||||
)
|
||||
result = local_env.execute(cmd, timeout=10)
|
||||
assert result["returncode"] == 0
|
||||
# All 10000 characters must survive the round-trip
|
||||
assert result["output"].count("\u65e5") == 10000, (
|
||||
f"lost multibyte chars across read boundaries: got "
|
||||
f"{result['output'].count(chr(0x65e5))} / 10000"
|
||||
)
|
||||
# And the "[binary output detected ...]" fallback must NOT fire
|
||||
assert "binary output detected" not in result["output"]
|
||||
|
||||
def test_invalid_utf8_uses_replacement_not_fallback(self, local_env):
|
||||
"""Truly invalid byte sequences must be substituted with U+FFFD (matching
|
||||
the pre-fix ``errors='replace'`` behaviour of the old ``TextIOWrapper``
|
||||
drain), not clobber the entire buffer with a fallback placeholder.
|
||||
"""
|
||||
# Write a deliberate invalid UTF-8 lead byte sandwiched between valid ASCII
|
||||
cmd = (
|
||||
'python3 -c \'import sys; '
|
||||
'sys.stdout.buffer.write(b"before "); '
|
||||
'sys.stdout.buffer.write(b"\\xff\\xfe"); '
|
||||
'sys.stdout.buffer.write(b" after\\n")\''
|
||||
)
|
||||
result = local_env.execute(cmd, timeout=5)
|
||||
assert result["returncode"] == 0
|
||||
assert "before" in result["output"]
|
||||
assert "after" in result["output"]
|
||||
assert "binary output detected" not in result["output"]
|
||||
|
||||
Reference in New Issue
Block a user