Parser: stop OSC at the first BEL or ST instead of reading into the next sequence#353
Conversation
parseOsc tried skipUntilST before checking for BEL, and skipUntilST grabs
the first ESC byte from index 2 and assumes the next byte is \. If an OSC
ends with BEL and another escape sequence follows it in the same buffer,
that first ESC belongs to the next sequence, so the OSC reads past its own
BEL and into it. Depending on the OSC type this dropped the next sequence
or, for a color or paste reply, returned an error out of parse() that
Loop.zig throws away with _ttyRun(...) catch {}, quietly killing the read
thread.
Scan for the first real terminator instead: BEL (0x07) or ST (ESC \). A
bare ESC that is not part of an ST means the OSC was never terminated and a
new sequence started, so return {null, 0} rather than reading past it.
Adds tests for an OSC 11 color reply followed by DA1 and a BEL-terminated
OSC followed by a CSI cursor-up.
ESC is an anywhere transition in the DEC parser state machine. When an OSC sees ESC followed by a byte other than backslash, the OSC has been interrupted by a new escape sequence rather than remaining incomplete. Consume the malformed OSC prefix and leave the ESC byte for the next parse call, so the following sequence can still be parsed. Keep waiting when the buffer ends at ESC because it may still become ST on the next read. Add regression tests for an OSC interrupted by CSI and for the trailing-ESC case.
|
I added the second commit to handle the case where an OSC is interrupted by another escape sequence. The DEC ANSI parser treats The first version of this PR correctly avoided consuming into the following sequence, but it returned could wedge the input loop waiting for the OSC to complete, even though the The follow-up commit changes that path to consume only the malformed OSC prefix and leave the The added tests cover both cases:
|
|
Thanks for the follow-up, good catch. Returning n == 0 on the interrupted case would wedge the loop waiting for the OSC to finish, and consuming the prefix while leaving the ESC for the next call is the right fix. The ESC-as-anywhere-transition framing makes it click. Looks good to me. |
|
Thanks! Good find. |
|
By good find I mean you @LordAizen1, not the ai 😂...not many parser bugs in libvaxis these days! |
|
Appreciate that, and thanks for the quick merge and the follow-up commit. Can't take all the credit though, libvaxis is clean enough that the bug actually stood out once I sat down and read Parser.zig against the spec. Your interrupted-OSC fix was the part I'd have missed. Fun one to chase, thanks for being so responsive. 😄 |
Fixes #352.
Right now
parseOsctriesskipUntilSTbefore it looks for BEL, andskipUntilSTgrabs the first ESC byte from index 2 and assumes the next byte is\. If an OSC ends with BEL and there is another escape sequence right after it, that first ESC is the next sequence's, so the OSC reads into it. Depending on the OSC type that either drops the next sequence or, for a color or paste reply, returns an error out ofparse(). Loop.zig throws that error away with_ttyRun(...) catch {}, which quietly kills the read thread and the app stops getting input.This swaps the scan for a single pass that looks for the first real terminator: BEL (0x07) or ST (
ESC \). A bare ESC that is not part of an ST means the OSC never got terminated and a new sequence started, so it returns{null, 0}instead of reading past it.Two tests:
zig build test: 169/170 (the one skip was already there), nothing else changed.