T36: review round 1: every pair of a known key is checked before the last of each key wins, a clear's too, so a report that breaks a rule anywhere drops whole; a sequence counts its length from OSC through ST with the controls its body skips; the scanners pass DEL and high bytes by in an escape as vte does, so ESC DEL c resets the records too
CI / platforms (ubuntu-24.04) (push) Successful in 1m6s
CI / platforms (windows-latest) (push) Successful in 1m33s
CI / macos (push) Successful in 14m17s

This commit is contained in:
ZacharyZhang-NY committed 2026-10-17 00:54:03 -04:00
1 parent 42b272ece6
commit 4a269cb57a
8 files changed
+133 -56

No files matched your search

+8
View File
@@ -105,6 +105,14 @@ impl Prompts {
self.out.push(byte);
State::Escape { sent }
}
// vte passes DEL and high bytes by in the escape: the ESC goes on, then they.
(State::Escape { sent }, 0x7f..=0xff) => {
if !sent {
self.release();
}
self.out.push(byte);
State::Escape { sent: true }
}
(State::Escape { .. }, _) => {
// A full reset clears the link the parser writes with.
if byte == b'c' {
+8
View File
@@ -211,6 +211,14 @@ fn controls_after_an_osc_s_closing_escape_run_where_vte_runs_them() {
assert_eq!(prompt(&marked.grid()[Line(1)]), Some(1));
}
#[test]
fn del_and_high_bytes_in_an_escape_go_on_after_it() {
assert_eq!(
passed(&[b"\x1b\x7f]133;A\x07$\x1b\xff", b"]133;B\x07"]),
b"\x1b\x7f]8;id=\x7f1;\x7f\x07$\x1b\xff]8;;\x07"
);
}
#[test]
fn controls_vte_runs_inside_an_escape_go_on_before_the_link() {
assert_eq!(
+25 -33
View File
@@ -64,9 +64,9 @@ const BASE64: GeneralPurpose = GeneralPurpose::new(
GeneralPurposeConfig::new().with_decode_padding_mode(DecodePaddingMode::Indifferent),
);
/// The sequence in an OSC 7501 body past `7501;`; none for one the spec drops, logged with why.
pub fn read(body: &[u8]) -> Option<Status> {
parse(body)
/// The sequence in an OSC 7501 body past `7501;`, `length` bytes from OSC through ST; none for one the spec drops, logged with why.
pub fn read(body: &[u8], length: usize) -> Option<Status> {
parse(body, length)
.inspect_err(|why| log::warn!("pty: OSC 7501 dropped: {why}"))
.ok()
}
@@ -81,15 +81,16 @@ fn named(byte: u8) -> bool {
byte.is_ascii_alphanumeric() || b"_.+-".contains(&byte)
}
fn parse(body: &[u8]) -> Result<Status, String> {
// ESC ] 7501 ; and an ST of two bytes frame the body.
if body.len() + 9 > SEQUENCE {
return Err(format!("over {SEQUENCE} bytes"));
fn parse(body: &[u8], length: usize) -> Result<Status, String> {
if length > SEQUENCE {
return Err(format!("{length} bytes, over {SEQUENCE}"));
}
if body.trim_ascii() == b"?" {
return Ok(Status::Query);
}
let mut pairs = Vec::new();
// Every pair is checked before the last of each key wins.
let (mut state, mut id, mut kind, mut progress, mut app, mut title, mut msg) =
(None, None, None, None, None, None, None);
for pair in body.split(|&byte| byte == b':') {
// A malformed pair is skipped: no `=`, no key, or a byte outside the set.
let Some(at) = pair.iter().position(|&byte| byte == b'=') else {
@@ -105,19 +106,20 @@ fn parse(body: &[u8]) -> Result<Status, String> {
if key.len() > KEY {
return Err(format!("a key over {KEY} bytes"));
}
let ascii = |bytes| String::from_utf8(Vec::from(bytes)).expect("ASCII");
pairs.push((ascii(key), ascii(value)));
let value = String::from_utf8(value.to_vec()).expect("ASCII");
match key {
b"state" => state = Some(value),
b"id" => id = Some(self::id(&value)?),
b"kind" => kind = Some(value),
b"progress" => progress = Some(value),
b"app" if value.len() > APP => return Err(format!("an app over {APP} bytes")),
b"app" => app = Some(value),
b"title" => title = Some(text(&value, "title", 256, 192)?),
b"msg" => msg = Some(text(&value, "msg", 2732, 2048)?),
_ => {}
}
}
// The last of a key wins.
let get = |key: &str| {
pairs
.iter()
.rev()
.find(|(name, _)| name == key)
.map(|(_, value)| value.as_str())
};
let id = get("id").map(id).transpose()?;
let state = match get("state").ok_or("no state")? {
let state = match state.as_deref().ok_or("no state")? {
"idle" => State::Idle,
"working" => State::Working,
"done" => State::Done,
@@ -126,30 +128,20 @@ fn parse(body: &[u8]) -> Result<Status, String> {
"clear" => return Ok(Status::Clear(id)),
other => return Err(format!("state {other} is not the spec's")),
};
let kind = match (state, get("kind")) {
let kind = match (state, kind.as_deref()) {
(State::Blocked, Some("permission")) => Some(Kind::Permission),
(State::Blocked, Some("question")) => Some(Kind::Question),
(State::Blocked, Some("auth")) => Some(Kind::Auth),
_ => None,
};
let progress = match state {
State::Working | State::Blocked => get("progress")
State::Working | State::Blocked => progress
.filter(|value| value.bytes().all(|byte| byte.is_ascii_digit()))
.and_then(|value| value.parse::<u8>().ok())
.filter(|&value| value <= 100),
_ => None,
};
let app = match get("app") {
Some(app) if app.len() > APP => return Err(format!("an app over {APP} bytes")),
Some(app) if !app.is_empty() && app.bytes().all(named) => Some(app.to_string()),
_ => None,
};
let title = get("title")
.map(|value| text(value, "title", 256, 192))
.transpose()?;
let msg = get("msg")
.map(|value| text(value, "msg", 2732, 2048))
.transpose()?;
let app = app.filter(|app| !app.is_empty() && app.bytes().all(named));
let record = Record {
state,
kind,
+51 -8
View File
@@ -4,6 +4,11 @@ fn encoded(text: &[u8]) -> String {
BASE64.encode(text)
}
/// What `body` says when its sequence is as long as its frame makes it.
fn said(body: &[u8]) -> Option<Status> {
read(body, body.len() + 9)
}
fn record(state: State, app: Option<&str>) -> Record {
Record {
state,
@@ -16,7 +21,7 @@ fn record(state: State, app: Option<&str>) -> Record {
}
fn reported(body: &str) -> Record {
match read(body.as_bytes()) {
match said(body.as_bytes()) {
Some(Status::Report(_, record)) => record,
other => panic!("{body}: {other:?}"),
}
@@ -30,7 +35,7 @@ fn the_spec_s_examples_read_as_it_means_them() {
msg: Some("Apply 3 to add, 1 to change, 0 to destroy?".into()),
..record(State::Blocked, Some("terraform"))
};
assert_eq!(read(body), Some(Status::Report(String::new(), blocked)));
assert_eq!(said(body), Some(Status::Report(String::new(), blocked)));
let body = b"state=working:id=us-east:title=VVMgRWFzdA==:progress=40:msg=UHVzaGluZyBpbWFnZQ==";
let working = Record {
progress: Some(40),
@@ -38,13 +43,13 @@ fn the_spec_s_examples_read_as_it_means_them() {
msg: Some("Pushing image".into()),
..record(State::Working, None)
};
assert_eq!(read(body), Some(Status::Report("us-east".into(), working)));
assert_eq!(said(body), Some(Status::Report("us-east".into(), working)));
assert_eq!(
read(b"state=clear:id=us-east"),
said(b"state=clear:id=us-east"),
Some(Status::Clear(Some("us-east".into())))
);
assert_eq!(read(b"state=clear"), Some(Status::Clear(None)));
assert_eq!(read(b"?"), Some(Status::Query));
assert_eq!(said(b"state=clear"), Some(Status::Clear(None)));
assert_eq!(said(b"?"), Some(Status::Query));
assert_eq!(
reported("state=done:msg=VXBncmFkZWQgMTIgcGFja2FnZXM")
.msg
@@ -82,9 +87,9 @@ fn a_sequence_that_breaks_a_rule_drops_whole() {
format!("state=done:x={}", "a".repeat(4096 - 9 - 12)),
];
for body in drops {
assert_eq!(read(body.as_bytes()), None, "{body}");
assert_eq!(said(body.as_bytes()), None, "{body}");
}
assert!(read(format!("state=done:x={}", "a".repeat(4096 - 9 - 14)).as_bytes()).is_some());
assert!(said(format!("state=done:x={}", "a".repeat(4096 - 9 - 14)).as_bytes()).is_some());
}
#[test]
@@ -166,3 +171,41 @@ fn past_the_cap_the_record_updated_longest_ago_goes() {
);
assert!(records.list.iter().any(|(id, ..)| id == "r0"));
}
#[test]
fn a_clear_that_breaks_a_rule_drops_whole() {
let drops = [
"state=clear:msg=Cg==".to_string(),
"state=clear:msg=A".into(),
format!("state=clear:app={}", "a".repeat(33)),
format!("state=clear:title={}", "a".repeat(257)),
"state=clear:id=a//b".into(),
];
for body in drops {
assert_eq!(said(body.as_bytes()), None, "{body}");
}
}
#[test]
fn every_pair_counts_though_the_last_of_a_key_wins() {
let drops = [
format!("state=done:app={}:app=ok", "a".repeat(33)),
"state=done:title=A:title=VVMgRWFzdA==".into(),
"state=done:id=a//b:id=ok".into(),
format!("state=done:msg={}:msg=b2s=", encoded(b"a\nb")),
];
for body in drops {
assert_eq!(said(body.as_bytes()), None, "{body}");
}
assert_eq!(reported("state=done:app=x:app=y").app.as_deref(), Some("y"));
assert_eq!(
reported("state=done:app=a,b:app=ok").app.as_deref(),
Some("ok")
);
}
#[test]
fn the_sequence_counts_from_osc_through_st() {
assert_eq!(read(b"state=done", 4097), None);
assert!(read(b"state=done", 4096).is_some());
}
+22 -12
View File
@@ -77,11 +77,14 @@ fn report(found: Found, events: &UnboundedSender<PtyEvent>, pid: u32, last: &mut
log::debug!("pty {pid}: a full reset");
PtyEvent::Reset
}
Found::Osc(body) => {
Found::Osc(body, length) => {
if let Some(mark) = body.strip_prefix(b"133;").and_then(marks::read) {
log::debug!("pty {pid}: {mark:?}");
PtyEvent::Mark(mark)
} else if let Some(status) = body.strip_prefix(b"7501;").and_then(status::read) {
} else if let Some(status) = body
.strip_prefix(b"7501;")
.and_then(|rest| status::read(rest, length))
{
log::debug!("pty {pid}: {status:?}");
PtyEvent::Status(status)
} else if let Some(folder) = body.strip_prefix(b"7;").and_then(folder) {
@@ -152,9 +155,9 @@ enum State {
Osc,
}
/// What the scanner finds: an OSC's body, or a full reset.
/// What the scanner finds: an OSC's body with the sequence's length from OSC through ST, or a full reset.
pub(crate) enum Found<'a> {
Osc(&'a [u8]),
Osc(&'a [u8], usize),
Reset,
}
@@ -163,6 +166,8 @@ pub(crate) enum Found<'a> {
pub(crate) struct Osc {
state: State,
body: Vec<u8>,
/// The sequence's bytes so far, the controls the body leaves out too.
length: usize,
overflow: bool,
}
@@ -185,6 +190,7 @@ impl Osc {
self.state = match (self.state, byte) {
(State::Escape, b']') => {
self.body.clear();
self.length = 2;
self.overflow = false;
State::Osc
}
@@ -192,20 +198,24 @@ impl Osc {
found(Found::Reset);
State::Ground
}
// vte runs controls in an escape and stays there.
(State::Escape, 0x00..=0x17 | 0x19 | 0x1b..=0x1f) => State::Escape,
// vte runs controls in an escape, passes DEL and high bytes by, and stays there.
(State::Escape, 0x00..=0x17 | 0x19 | 0x1b..=0x1f | 0x7f..=0xff) => State::Escape,
(State::Escape, _) => State::Ground,
(State::Osc, 0x07 | 0x18 | 0x1a) => {
self.finish(found);
self.finish(1, found);
State::Ground
}
// ESC ends the body, then reads as an escape: `\` makes ST.
// ESC ends the body, then reads as an escape: `\` makes ST, counted so.
(State::Osc, 0x1b) => {
self.finish(found);
self.finish(2, found);
State::Escape
}
(State::Osc, 0x00..=0x06 | 0x08..=0x17 | 0x19 | 0x1c..=0x1f) => State::Osc,
(State::Osc, 0x00..=0x06 | 0x08..=0x17 | 0x19 | 0x1c..=0x1f) => {
self.length += 1;
State::Osc
}
(State::Osc, byte) => {
self.length += 1;
if self.body.len() < MOST {
self.body.push(byte);
} else {
@@ -218,11 +228,11 @@ impl Osc {
}
}
fn finish(&mut self, found: &mut impl FnMut(Found)) {
fn finish(&mut self, end: usize, found: &mut impl FnMut(Found)) {
if self.overflow {
log::warn!("pty: an OSC over {MOST} bytes dropped");
} else {
found(Found::Osc(&self.body));
found(Found::Osc(&self.body, self.length + end));
}
self.body.clear();
}
+17 -1
View File
@@ -7,7 +7,7 @@ fn scan(chunks: &[&[u8]]) -> Vec<String> {
for chunk in chunks {
scanner.feed(chunk, &mut |thing| {
found.push(match thing {
Found::Osc(body) => String::from_utf8(body.to_vec()).expect("utf-8"),
Found::Osc(body, _) => String::from_utf8(body.to_vec()).expect("utf-8"),
Found::Reset => "RESET".into(),
})
});
@@ -25,6 +25,22 @@ fn finds_a_full_reset_between_bodies() {
scan(&[b"\x1b[c\x1b]0;c"]).is_empty(),
"DA1 and an unfinished body"
);
// vte passes DEL and high bytes by in an escape, and the reset still comes.
assert_eq!(scan(&[b"\x1b\x7fc\x1b\xff", b"c"]), ["RESET", "RESET"]);
}
#[test]
fn a_sequence_counts_from_osc_through_st_skipped_controls_too() {
let mut lengths = Vec::new();
let mut osc = b"\x1b]7501;".to_vec();
osc.extend([0; 5000]);
osc.extend_from_slice(b"state=done\x07\x1b]0;t\x1b\\");
Osc::default().feed(&osc, &mut |found| {
if let Found::Osc(body, length) = found {
lengths.push((body.len(), length));
}
});
assert_eq!(lengths, [(15, 5018), (3, 7)]);
}
#[test]