fix(connection): tolerate non-UTF-8 command output when decoding#191
Open
plusky wants to merge 1 commit into
Open
fix(connection): tolerate non-UTF-8 command output when decoding#191plusky wants to merge 1 commit into
plusky wants to merge 1 commit into
Conversation
Connection.run() accumulated raw bytes from the channel and returned them via stdout.decode()/stderr.decode() with the default strict UTF-8 error mode. Any non-UTF-8 byte in a remote command's output (locale-encoded package descriptions, binary noise) raised UnicodeDecodeError out of run(), which is neither CommandTimeout nor the expected result tuple, so it escaped unhandled and killed the host worker mid-run. The streaming debug path already decoded with a tolerant mode, making the strict final decode an oversight. The asyncssh backend had the same defect one layer down: asyncssh decodes session output with encoding='utf-8' and errors='strict' by default, and its channel wraps the UnicodeDecodeError in a ProtocolError that tears down the whole connection on the first bad byte. Decode the paramiko backend's final output with errors="replace" and pass errors="replace" through AsyncConnection.run() to asyncssh, so undecodable bytes come back as U+FFFD on both backends instead of raising. The paramiko regression test feeds b"ok\xff\xfe"-style output through run()'s decode and asserts the replaced text is returned; the asyncssh regression test asserts errors="replace" is passed through to asyncssh, which performs the decode in its channel layer.
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.
What
Connection.run() accumulated raw bytes from the channel and returned
them via stdout.decode()/stderr.decode() with the default strict
UTF-8 error mode. Any non-UTF-8 byte in a remote command's output
(locale-encoded package descriptions, binary noise) raised
UnicodeDecodeError out of run(), which is neither CommandTimeout nor
the expected result tuple, so it escaped unhandled and killed the
host worker mid-run. The streaming debug path already decoded with a
tolerant mode, making the strict final decode an oversight.
The asyncssh backend had the same defect one layer down: asyncssh
decodes session output with encoding='utf-8' and errors='strict' by
default, and its channel wraps the UnicodeDecodeError in a
ProtocolError that tears down the whole connection on the first bad
byte.
Decode the paramiko backend's final output with errors="replace" and
pass errors="replace" through AsyncConnection.run() to asyncssh, so
undecodable bytes come back as U+FFFD on both backends instead of
raising. The paramiko regression test feeds b"ok\xff\xfe"-style
output through run()'s decode and asserts the replaced text is
returned; the asyncssh regression test asserts errors="replace" is
passed through to asyncssh, which performs the decode in its channel
layer.
Note: this is one of five single-concern fixes in this batch touching
repose/connection.py(several alsorepose/aiossh.py), on top of the four already-open connection PRs #182–#185. They conflict pairwise; any merge order works and later ones get a trivial rebase, which I'll provide.Verification
Full gate green (ruff format/check, ty, pytest: 549 passed, coverage 83.49%). The paramiko regression test feeds non-UTF-8 bytes through the real decode; the asyncssh test pins the errors="replace" pass-through (the decode happens inside asyncssh's channel layer). Both verified to fail on the pre-fix code. Reviewed by a fan-out adversarial code review (two finder lenses per branch, two verifier lenses per finding); all confirmed findings were addressed before opening.
AI-assisted.