#1060
Issues/crossterm-rs/crossterm/#1060

Bracketed paste parser hangs event stream indefinitely if `\x1B[201~` never arrives

OpenDannyStoll1opened 5mo ago

Describe the bug

When bracketed paste mode is enabled, crossterm's internal Parser (src/event/source/unix/{tty,mio}.rs) accumulates every byte received between the opening ESC [ 200 ~ and closing ESC [ 201 ~ markers into a single in-flight Event::Paste. The accumulator buffer has no upper bound, neither in time nor in bytes:

https://github.com/crossterm-rs/crossterm/blob/0.29/src/event/sys/unix/parse.rs#L817-L822

if !buffer.ends_with(b"\x1b[201~") {
    Ok(None)
} else {
    let paste = String::from_utf8_lossy(&buffer[6..buffer.len() - 6]).to_string();
    Ok(Some(InternalEvent::Event(Event::Paste(paste))))
}

If the closing marker never arrives, the parser silently swallows every subsequent byte read from the tty. The application keeps reading keystrokes from the OS, but it never receives an Event. From the user's perspective, the application appears to have stopped listening to their input.

There is currently no public API to flush or reset the parser's internal state, so there is no good recovery option. At present, applications/users are essentially limited to three options:

  • attempt a graceful shutdown (only possible if the application has a watchdog in place to detect the issue),
  • try to hack the event stream back into a valid state by emulating a \x1B[201~ input or issuing another bracketed pasted (hard for a user to know this is what the problem is, and will emit a ton of garbage input like mouse movements), or
  • kill the process, losing any unsaved state.

This condition is reachable under normal operation:

  • tmux pane swap: I initially encountered this bug when the text I was pasting into the application triggered a tmux swap to a different pane. This meant the closing marker never reached the application's pane.
  • Misbehaving terminal emulator: Some terminal emulators (kitty, iTerm2 under certain configurations) can drop the close marker if focus is moved away mid-paste or if the host renegotiates terminal modes between the opening and closing markers.
  • Adversarial input: A program that prints \x1B[2004h\x1B[200~ to a terminal hosting a crossterm application can wedge it by never sending the close marker. See "Failure modes worth flagging" below.

To Reproduce

Minimal repro using the existing (private) parser tests:

use crossterm::event::sys::unix::parse::parse_event;
assert_eq!(parse_event(b"\x1B[200~aaaaaaaaaaaaaaa", false).unwrap(), None);
//                                                         ^^^^^^
//                    Buffer keeps growing on every subsequent call;
//                    there is no time- or byte-bound that ever yields
//                    Ok(Some(...)) until the literal `\x1B[201~` arrives.

To reproduce end-to-end:

  1. Run a crossterm application with EnableBracketedPaste set.
  2. Print \x1B[200~ to the application's stdin from any source (another process via the host tty, a malicious tmux paste buffer, etc.)
  3. Type keys or attempt mouse input.
  4. Nothing happens. The application is wedged.

Expected behavior

After a configurable cap (e.g. wall-clock duration since the opening marker or total accumulated bytes), the parser should abandon the inflight paste, discard or surface the buffered bytes, and resume normal event parsing so the application regains control of input.

Failure modes worth flagging

Crossterm runs on a tty owned by the host user, so input bytes are typically trusted and crossterm is not advertised as a security boundary. Still, the open-ended accumulator interacts badly with two non-malicious operational patterns:

Wedge via untrusted terminal output

Any process whose stdout reaches the controlling tty of a crossterm application using EnableBracketedPaste can wedge it indefinitely by emitting \x1B[2004h\x1B[200~ without the matching close marker. This can include:

  • cat-ing a file that happens to contain those bytes,
  • stdout from a sandboxed child process that the parent doesn't strip,
  • output from an ssh subprocess piped back to the originating terminal.

None of these require a malicious attacker -- a logfile with embedded escape sequences is enough to wedge the TUI. For long running applications, the wedge is total and recovery is out of band.

Unbounded memory growth

The accumulator is an unbounded Vec<u8>. A process that streams large amounts of data inside an unfinished paste will consume the host's RAM. Crossterm currently has no mechanism to bound or evict this buffer. On constrained systems (CI runners, containers), this can crash the host application before the OOM killer arrives.

Environment

OS

Linux 4.18. Should reproduce under any unix-like OS.

Terminal/Console

kitty 0.31 wrapping ssh wrapping tmux 3.3. Should reproduce in any environment that has a path produce bracketed paste open sequences without a paired close sequence.

Activity

9
joshka5mo ago

Following up on my comment on the PR, I wanted to move the design discussion back here, since I think the main question is bigger than the parser fix itself.

The thing I’m most hesitant about is not the recovery logic, it is adding a new public Event variant.

crossterm::event::Event is a very high-fanout API surface. Most crossterm consumers have at least one match event point somewhere in their stack, and many of those matches are exhaustive. So adding a new Event variant creates broad compile-time breakage even for downstreams that do not care about bracketed paste at all.

That makes changes to Event unusually expensive, and it makes me want to separate:

  1. fixing the orphaned bracketed-paste wedge
  2. deciding whether Event should grow a new public variant

I do think the wedge described here is real and worth fixing.

If limits are added, the two that seem most defensible to me are wall-clock duration and accumulated bytes, since those map directly to the two concrete failure modes in this issue:

  • indefinite input wedging
  • unbounded buffer growth

My hesitation is with surfacing that recovery as Event::PasteAborted.

As one concrete downstream example, from my past work on Codex’s terminal input stack, bracketed paste was not a marginal path for us. We had explicit logic around crossterm::event::Event::Paste(String), and then additional app-level machinery layered on top of it for cases where terminals do not reliably provide bracketed paste.

In the current Codex tree, that still appears to be true:

  • the crossterm event stream is mapped explicitly from Event::Paste(...) into an app-level TuiEvent::Paste(String)
  • the main TUI routes TuiEvent::Paste through dedicated paste handling rather than treating it like ordinary key input
  • the bottom-pane composer has substantial paste-specific state machine logic, including a separate “paste burst” fallback for terminals that do not behave well
  • at least one other terminal path (cloud-tasks) also matches directly on Some(Ok(Event::Paste(...)))

The reason I bring that up is not because I think OpenAI specifically needs to approve the design. It is just one downstream I know well enough to say that, once an application has explicit paste handling, Event::Paste(String) usually becomes a semantic signal, not just another enum arm.

At that point, a downstream has often made product decisions like:

  • should pasted input be inserted as one unit or interpreted character-by-character?
  • should large pasted content trigger different UX or validation behavior?
  • should pasted newlines be normalized differently from typed input?
  • should paste handling bypass logic that exists for ordinary keystrokes?

So Event::PasteAborted is not just “one more variant to match on”. It asks downstreams to decide whether an aborted paste should behave more like:

  • a normal paste
  • a failed paste that should be ignored
  • partially recoverable input
  • or something that should surface to users as an error/state transition

For downstreams with substantial paste-specific behavior, that is a real design and testing burden, not just a mechanical compile fix.

I also think this should be considered as precedent for other stateful terminal interactions where crossterm observes the start of a protocol sequence and the corresponding completion never arrives. That makes me more inclined to keep the initial recovery design simple and conservative, and to be careful about establishing “new public event variant for timeout/recovery” as the general pattern unless we are confident that is the right direction more broadly.

So my current preference is:

  • fix the wedge
  • add opt-in limits if that is the chosen recovery mechanism
  • avoid changing the public Event shape if possible

The non-breaking direction I would want explored first is aborting the in-flight paste and recovering via the existing Event::Paste(String) surface, rather than introducing a distinct Event::PasteAborted variant.

More broadly, if crossterm expects Event to grow over time, it may be worth thinking separately about how to migrate from an exhaustive to a non-exhaustive public enum, possibly over multiple releases. It may also be perfectly reasonable to do that as a clean breaking change in a major release. Either way, that feels like a larger API-policy decision than this one bugfix.

That said, this is just my opinion based on significant prior work in this area, not a hard no. I’d defer heavily to people actively maintaining and using these paths in real applications.

From the current Codex checkout, the main active owners I see around the relevant paste/TUI paths are @etraut-openai and @fcoury-oai, so I’d be interested in their take on whether a distinct “failed paste” event is actually useful enough downstream to justify the break. I’d also value comments from other downstream users of crossterm who have this in their input stack.

joshka5mo ago

Following up on my comment on the PR, I wanted to move the design discussion back here, since I think the main question is bigger than the parser fix itself.

The thing I’m most hesitant about is not the recovery logic, it is adding a new public Event variant.

crossterm::event::Event is a very high-fanout API surface. Most crossterm consumers have at least one match event point somewhere in their stack, and many of those matches are exhaustive. So adding a new Event variant creates broad compile-time breakage even for downstreams that do not care about bracketed paste at all.

That makes changes to Event unusually expensive, and it makes me want to separate:

  1. fixing the orphaned bracketed-paste wedge
  2. deciding whether Event should grow a new public variant

I do think the wedge described here is real and worth fixing.

If limits are added, the two that seem most defensible to me are wall-clock duration and accumulated bytes, since those map directly to the two concrete failure modes in this issue:

  • indefinite input wedging
  • unbounded buffer growth

My hesitation is with surfacing that recovery as Event::PasteAborted.

As one concrete downstream example, from my past work on Codex’s terminal input stack, bracketed paste was not a marginal path for us. We had explicit logic around crossterm::event::Event::Paste(String), and then additional app-level machinery layered on top of it for cases where terminals do not reliably provide bracketed paste.

In the current Codex tree, that still appears to be true:

  • the crossterm event stream is mapped explicitly from Event::Paste(...) into an app-level TuiEvent::Paste(String)
  • the main TUI routes TuiEvent::Paste through dedicated paste handling rather than treating it like ordinary key input
  • the bottom-pane composer has substantial paste-specific state machine logic, including a separate “paste burst” fallback for terminals that do not behave well
  • at least one other terminal path (cloud-tasks) also matches directly on Some(Ok(Event::Paste(...)))

The reason I bring that up is not because I think OpenAI specifically needs to approve the design. It is just one downstream I know well enough to say that, once an application has explicit paste handling, Event::Paste(String) usually becomes a semantic signal, not just another enum arm.

At that point, a downstream has often made product decisions like:

  • should pasted input be inserted as one unit or interpreted character-by-character?
  • should large pasted content trigger different UX or validation behavior?
  • should pasted newlines be normalized differently from typed input?
  • should paste handling bypass logic that exists for ordinary keystrokes?

So Event::PasteAborted is not just “one more variant to match on”. It asks downstreams to decide whether an aborted paste should behave more like:

  • a normal paste
  • a failed paste that should be ignored
  • partially recoverable input
  • or something that should surface to users as an error/state transition

For downstreams with substantial paste-specific behavior, that is a real design and testing burden, not just a mechanical compile fix.

I also think this should be considered as precedent for other stateful terminal interactions where crossterm observes the start of a protocol sequence and the corresponding completion never arrives. That makes me more inclined to keep the initial recovery design simple and conservative, and to be careful about establishing “new public event variant for timeout/recovery” as the general pattern unless we are confident that is the right direction more broadly.

So my current preference is:

  • fix the wedge
  • add opt-in limits if that is the chosen recovery mechanism
  • avoid changing the public Event shape if possible

The non-breaking direction I would want explored first is aborting the in-flight paste and recovering via the existing Event::Paste(String) surface, rather than introducing a distinct Event::PasteAborted variant.

More broadly, if crossterm expects Event to grow over time, it may be worth thinking separately about how to migrate from an exhaustive to a non-exhaustive public enum, possibly over multiple releases. It may also be perfectly reasonable to do that as a clean breaking change in a major release. Either way, that feels like a larger API-policy decision than this one bugfix.

That said, this is just my opinion based on significant prior work in this area, not a hard no. I’d defer heavily to people actively maintaining and using these paths in real applications.

From the current Codex checkout, the main active owners I see around the relevant paste/TUI paths are @etraut-openai and @fcoury-oai, so I’d be interested in their take on whether a distinct “failed paste” event is actually useful enough downstream to justify the break. I’d also value comments from other downstream users of crossterm who have this in their input stack.

joshka5mo ago

Following up on my comment on the PR, I wanted to move the design discussion back here, since I think the main question is bigger than the parser fix itself.

The thing I’m most hesitant about is not the recovery logic, it is adding a new public Event variant.

crossterm::event::Event is a very high-fanout API surface. Most crossterm consumers have at least one match event point somewhere in their stack, and many of those matches are exhaustive. So adding a new Event variant creates broad compile-time breakage even for downstreams that do not care about bracketed paste at all.

That makes changes to Event unusually expensive, and it makes me want to separate:

  1. fixing the orphaned bracketed-paste wedge
  2. deciding whether Event should grow a new public variant

I do think the wedge described here is real and worth fixing.

If limits are added, the two that seem most defensible to me are wall-clock duration and accumulated bytes, since those map directly to the two concrete failure modes in this issue:

  • indefinite input wedging
  • unbounded buffer growth

My hesitation is with surfacing that recovery as Event::PasteAborted.

As one concrete downstream example, from my past work on Codex’s terminal input stack, bracketed paste was not a marginal path for us. We had explicit logic around crossterm::event::Event::Paste(String), and then additional app-level machinery layered on top of it for cases where terminals do not reliably provide bracketed paste.

In the current Codex tree, that still appears to be true:

  • the crossterm event stream is mapped explicitly from Event::Paste(...) into an app-level TuiEvent::Paste(String)
  • the main TUI routes TuiEvent::Paste through dedicated paste handling rather than treating it like ordinary key input
  • the bottom-pane composer has substantial paste-specific state machine logic, including a separate “paste burst” fallback for terminals that do not behave well
  • at least one other terminal path (cloud-tasks) also matches directly on Some(Ok(Event::Paste(...)))

The reason I bring that up is not because I think OpenAI specifically needs to approve the design. It is just one downstream I know well enough to say that, once an application has explicit paste handling, Event::Paste(String) usually becomes a semantic signal, not just another enum arm.

At that point, a downstream has often made product decisions like:

  • should pasted input be inserted as one unit or interpreted character-by-character?
  • should large pasted content trigger different UX or validation behavior?
  • should pasted newlines be normalized differently from typed input?
  • should paste handling bypass logic that exists for ordinary keystrokes?

So Event::PasteAborted is not just “one more variant to match on”. It asks downstreams to decide whether an aborted paste should behave more like:

  • a normal paste
  • a failed paste that should be ignored
  • partially recoverable input
  • or something that should surface to users as an error/state transition

For downstreams with substantial paste-specific behavior, that is a real design and testing burden, not just a mechanical compile fix.

I also think this should be considered as precedent for other stateful terminal interactions where crossterm observes the start of a protocol sequence and the corresponding completion never arrives. That makes me more inclined to keep the initial recovery design simple and conservative, and to be careful about establishing “new public event variant for timeout/recovery” as the general pattern unless we are confident that is the right direction more broadly.

So my current preference is:

  • fix the wedge
  • add opt-in limits if that is the chosen recovery mechanism
  • avoid changing the public Event shape if possible

The non-breaking direction I would want explored first is aborting the in-flight paste and recovering via the existing Event::Paste(String) surface, rather than introducing a distinct Event::PasteAborted variant.

More broadly, if crossterm expects Event to grow over time, it may be worth thinking separately about how to migrate from an exhaustive to a non-exhaustive public enum, possibly over multiple releases. It may also be perfectly reasonable to do that as a clean breaking change in a major release. Either way, that feels like a larger API-policy decision than this one bugfix.

That said, this is just my opinion based on significant prior work in this area, not a hard no. I’d defer heavily to people actively maintaining and using these paths in real applications.

From the current Codex checkout, the main active owners I see around the relevant paste/TUI paths are @etraut-openai and @fcoury-oai, so I’d be interested in their take on whether a distinct “failed paste” event is actually useful enough downstream to justify the break. I’d also value comments from other downstream users of crossterm who have this in their input stack.

DannyStoll15mo ago

Thanks for the detailed review @joshka -- I agree with your point that there is a high cost to updating Event due to it being exhaustive. I'd hope to some day make it #[non_exhaustive] (which will have an equivalent one-time cost if and when it lands), but that's certainly outside the scope here.

Regarding reusing Event::Paste(String) specifically: my hesitation is that the salvaged buffer at abort time isn't really a paste. It's \x1B[200~ plus whatever bytes happened to arrive in the configured window, which is presumably a mix of intended paste content and post-wedge keystrokes the user typed thinking the paste went through. This is probably fine in most use cases, but it has the drawback that downstream code doesn't know to distinguish this from a regular paste (I suppose callers could manually check if the paste starts with "\x1B[200~", but this seems rather clunky). So I'm not sure Event::Paste is quite the ideal venue either, though it has the major advantage of being non-breaking.

I can think of a few alternatives, ordered roughly by API impact:

  1. Silent drop: abort, clear buffer, no event emitted. Minimal, non-breaking, and fixes the wedge. The app won't be able to distinguish "no paste happened" from "we recovered from one," and if the limit takes seconds to fire (which it will, depending on the timeout), the user will see a freeze-then-recover with no signal to the app or its logs.
  2. Silent drop by default, but layer in a pub fn set_paste_abort_handler(impl Fn(PasteAbortReason, Vec<u8>) + Send + Sync + 'static) (or a channel-style sink). Non-breaking/opt-in, and gives interested downstreams a signal. This signal would be orthogonal to the event stream, which is the main API smell.
  3. (0) with tracing::warn! on abort. Non-breaking and allows for retroactive diagnosis, but does not allow downstreams to handle aborts. Main cost is this adds tracing as a new runtime dependency.
  4. Event::PasteAborted as proposed in #1061, bundled with a deliberate #[non_exhaustive] migration on Event. Breaking, but it's a separable API-policy decision that crossterm probably wants to make once rather than per-variant. Probably too much scope for the immediate fix.

My preference would be to land (0) on its own as an immediate fix, decoupled from the API question entirely. This would let downstream code opt in to limits immediately, while leaving the recovery surface decision as a separate followup. If you'd rather bundle the recovery surface in, (1) seems to me like the most reasonable option among the above.

Either way, happy to revise the PR once we decide on the right direction.

DannyStoll15mo ago

Thanks for the detailed review @joshka -- I agree with your point that there is a high cost to updating Event due to it being exhaustive. I'd hope to some day make it #[non_exhaustive] (which will have an equivalent one-time cost if and when it lands), but that's certainly outside the scope here.

Regarding reusing Event::Paste(String) specifically: my hesitation is that the salvaged buffer at abort time isn't really a paste. It's \x1B[200~ plus whatever bytes happened to arrive in the configured window, which is presumably a mix of intended paste content and post-wedge keystrokes the user typed thinking the paste went through. This is probably fine in most use cases, but it has the drawback that downstream code doesn't know to distinguish this from a regular paste (I suppose callers could manually check if the paste starts with "\x1B[200~", but this seems rather clunky). So I'm not sure Event::Paste is quite the ideal venue either, though it has the major advantage of being non-breaking.

I can think of a few alternatives, ordered roughly by API impact:

  1. Silent drop: abort, clear buffer, no event emitted. Minimal, non-breaking, and fixes the wedge. The app won't be able to distinguish "no paste happened" from "we recovered from one," and if the limit takes seconds to fire (which it will, depending on the timeout), the user will see a freeze-then-recover with no signal to the app or its logs.
  2. Silent drop by default, but layer in a pub fn set_paste_abort_handler(impl Fn(PasteAbortReason, Vec<u8>) + Send + Sync + 'static) (or a channel-style sink). Non-breaking/opt-in, and gives interested downstreams a signal. This signal would be orthogonal to the event stream, which is the main API smell.
  3. (0) with tracing::warn! on abort. Non-breaking and allows for retroactive diagnosis, but does not allow downstreams to handle aborts. Main cost is this adds tracing as a new runtime dependency.
  4. Event::PasteAborted as proposed in #1061, bundled with a deliberate #[non_exhaustive] migration on Event. Breaking, but it's a separable API-policy decision that crossterm probably wants to make once rather than per-variant. Probably too much scope for the immediate fix.

My preference would be to land (0) on its own as an immediate fix, decoupled from the API question entirely. This would let downstream code opt in to limits immediately, while leaving the recovery surface decision as a separate followup. If you'd rather bundle the recovery surface in, (1) seems to me like the most reasonable option among the above.

Either way, happy to revise the PR once we decide on the right direction.

joshka5mo ago

This is probably fine in most use cases, but it has the drawback that downstream code doesn't know to distinguish this from a regular paste

My bar for this (which could be wrong) Is is it worse than what's there now or better. I think that would be generally better, but what problems would this cause that are non obvious. It's definitely something worth getting a few eyes on.

I still lean to mainly doing a Event::Paste("whatever was captured up to the limit") here. Sort of a break in a reasonable way that allows recovery. The problem with aborting generally is that you're arbitrarily choosing which keystrokes are thrown away and which are kept. So if there are destructive commands after the paste that would have been interpreted as pasted instead of pressed then you're in for a world of hurt if you dump the buffer. I think you really still just need to have a linear set of events here (I could be wrong about this as well - 👀 needed :))

joshka5mo ago

This is probably fine in most use cases, but it has the drawback that downstream code doesn't know to distinguish this from a regular paste

My bar for this (which could be wrong) Is is it worse than what's there now or better. I think that would be generally better, but what problems would this cause that are non obvious. It's definitely something worth getting a few eyes on.

I still lean to mainly doing a Event::Paste("whatever was captured up to the limit") here. Sort of a break in a reasonable way that allows recovery. The problem with aborting generally is that you're arbitrarily choosing which keystrokes are thrown away and which are kept. So if there are destructive commands after the paste that would have been interpreted as pasted instead of pressed then you're in for a world of hurt if you dump the buffer. I think you really still just need to have a linear set of events here (I could be wrong about this as well - 👀 needed :))

DannyStoll15mo ago

I'm not sure I understand how sending a truncated Paste event gets around the destructive-keystroke concern. If the abort fires at byte N, bytes 0..=N are handled by whatever callback we decide on (dropping, moving into a PasteAborted.buffered_bytes, moving into an Event::Paste, etc), so they are not reparsed as keystrokes. But bytes that arrive after the trigger are parsed as normal events in all cases.

The only library-side way to avoid this would be to stay in paste-mode and keep accumulating until a close marker (i.e. the current wedging behavior).

We could leave it up to callers to mitigate the concern by somehow notifying them that an abort occurred so they can decide how to handle potential uncaptured trailing bytes. This is the idea of #1061 and the approach (1) I suggest above.

If we do go with Event::Paste, it's also worth considering whether or not to strip the \x1B[200~ prefix from the surfaced string. Stripping matches the shape of a normal Paste, but leaving it in gives apps a way (albeit hacky) to detect an abort occurred.

joshka5mo ago

I think you’re right that Event::Paste(truncated) doesn’t solve the post-abort trailing-keystroke concern. After the abort fires, anything that arrives later is normal input unless we keep waiting for ESC [ 201 ~, which is the wedge we’re trying to escape.

The reason I still prefer it over silent drop is narrower: crossterm has already consumed bytes 0..=N, so surfacing them as a paste preserves a linear event stream better than dropping them. It does not make any guarantee about later bytes.

If we use Event::Paste, I would strip the ESC [ 200 ~ prefix. Leaving it in turns Paste into a hidden abort signal and changes the semantic shape of the variant. If downstreams need to distinguish aborts, that points back toward an explicit abort signal or handler rather than overloading Paste.

Before choosing the public API shape, I’d like to hear from affected users and see a bit more downstream evidence. Since you’re running into this in practice, could you look at a few real crossterm downstreams that enable bracketed paste and summarize how they currently handle Event::Paste? The useful questions are whether they treat paste as a semantic event, whether they appear to use exhaustive match Event handling, and what they would plausibly do with an aborted paste.

The obvious compatibility issue with Event::PasteAborted is that it breaks downstream code with exhaustive match Event handling, but that is hard to measure from GitHub search because the break depends on local match structure. A few concrete examples would help us decide whether a distinct abort signal is worth the API cost.