Skip to content

fix(input): discard unrecognised CSI sequences instead of typing them - #6

Open
basil-k-aji-dev wants to merge 1 commit into
rylena:mainfrom
basil-k-aji-dev:fix/discard-unrecognised-csi
Open

basil-k-aji-dev wants to merge 1 commit into
rylena:mainfrom
basil-k-aji-dev:fix/discard-unrecognised-csi

Conversation

@basil-k-aji-dev

Copy link
Copy Markdown

Fixes #5

TerminalEventParser recognised only a small set of CSI sequences. SGR colour sequences — \x1b[31m, \x1b[0m — matched none of them, and failed two different ways depending on timing.

Typed at the remote host. After ESCAPE_DELAY the ESC fallback did del self.buffer[0], removing only the ESC byte and emitting an ESCAPE key. The remaining [31m was then parsed as ordinary characters, so a single colour sequence produced five keystrokes:

ESCAPE, '[', '3', '1', 'm'

Ended the session. Before the delay elapsed the loop breaks and left the sequence buffered, stalling every byte behind it. The read loop in session/direct.py only calls flush() when select() times out, so during a paste feed() runs back to back and the residue is never cleared. Once it crossed the 8192 guard:

raise ValueError("terminal input sequence exceeds 8192 bytes")

That propagates out of _input_worker, which forwards it via self._controls.put(exc), and the main loop re-raises it with raise control. About 8 KB of coloured text is enough — git diff output, coloured logs, anything copied from another terminal.

Change

CSI_ANY_RE matches a complete CSI sequence (ESC [, parameter bytes 0x30–0x3f, intermediate bytes 0x20–0x2f, final byte 0x40–0x7e). It is consulted only after every specific handler has declined, so a sequence this parser does not act on is discarded rather than buffered or retyped.

It sits after the existing incomplete-sequence check, so partial sequences still buffer and split reads keep working — feed(b"\x1b[12;") then feed(b"40R") still yields one cursor report.

Tests

Four added to tests/test_input.py. The first three fail on main:

FAILED test_unrecognised_csi_is_discarded_not_typed
FAILED test_coloured_text_yields_only_its_visible_characters
FAILED test_bulk_coloured_input_does_not_exhaust_the_buffer

with the first showing exactly the spurious keystrokes:

- [KeyEvent(action=2, modifiers=0, key_code=2, unicode=0),
-  KeyEvent(action=2, modifiers=0, key_code=0, unicode=91),    # '['
-  KeyEvent(action=2, modifiers=0, key_code=0, unicode=51),    # '3'
-  KeyEvent(action=2, modifiers=0, key_code=0, unicode=49),    # '1'
-  KeyEvent(action=2, modifiers=0, key_code=0, unicode=109)]   # 'm'

The fourth guards against regressing arrow keys, cursor reports and partial-sequence buffering.

pytest -q tests/ — 92 passed (88 before). ruff check clean on both changed files. Python 3.13.3, Linux aarch64.

Note

The 8192 guard is left as it is. It still measures the whole buffer before parsing rather than the residue after it, so it can in principle fire on input that is merely unconsumed rather than on one oversized sequence — but with unknown CSI sequences no longer accumulating, I could not reach it with realistic input. Tightening it to check after the parse loop would match the wording of its message; happy to include that here if you would prefer.

TerminalEventParser recognised only a small set of CSI sequences. SGR
colour sequences such as \x1b[31m matched none of them, which failed two
ways depending on timing.

Before ESCAPE_DELAY elapsed the parse loop broke and left the sequence
buffered, stalling every byte behind it. The session read loop only calls
flush() when select() times out, so during a paste feed() runs back to
back and the residue is never cleared; once it crossed the 8192 guard the
ValueError propagated out of the input worker and ended the session.
Roughly 8KB of coloured text — git diff output, coloured logs — was enough.

After the delay the ESC fallback removed only the ESC byte and emitted an
ESCAPE key, leaving "[31m" to be parsed as ordinary characters and typed
at the remote host.

Add CSI_ANY_RE, matching a complete CSI sequence, and consume it once every
specific handler has declined. Partial sequences still buffer as before, so
split reads keep working.

Fixes rylena#5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unrecognised CSI sequences are typed as literal keystrokes, and kill the session when pasted in bulk

1 participant