From 1a13e8b556a6bcb591c6b99742db3ad46287a25c Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 08:45:21 +0200 Subject: [PATCH] fix: close the tail items from the targeted re-audit Resolves the 13 rows still partial in docs/rewrite/notes/review-rv4- codex.md (details in final-Z5.md): - Escaped config strings are decoded to raw bytes at every OS boundary: Flatpak cwd/argv to D-Bus, screen-file paths, the default config template, GHOSTTY_RESOURCES_DIR/TERMINFO/shell integration/ bindtextdomain consumers, custom shader loading, list-themes hyperlinks. - osbytes escapes literal U+FDD0 on non-Unix too (bijective everywhere); the GTK paste dialog only decodes text that was actually encoded. - shadertoy: whole-array uses of iChannelTime rebuild a plain float[4] from the std140-padded members (naga-validated, MSL offsets checked). - libghostty-vt: the OSC parser holds caller-allocated storage inline and command payloads borrow from it, so ghostty_osc_end performs no global allocations (verified from Rust and C). - crash: aarch64 Linux minidump context fixed; macOS fatal signals record siginfo, registers and a fault-safe stack walk. - tripwire: selectionString map allocation-failure cleanup test ported. - libghostty-vt emits the versioned soname libghostty-vt.so.0 at build time (no patchelf needed at install). Test Plan: - cargo test --workspace (3342 passed) - just check-macos; just check-macos x86_64-apple-darwin; just check-windows; just check-wasm - cargo check --target aarch64-unknown-linux-gnu -p ghostty-crash --- Cargo.lock | 1 + crates/ghostty-apprt-gtk/src/class/surface.rs | 49 +++- crates/ghostty-cli/src/list_themes.rs | 13 +- crates/ghostty-config/src/config.rs | 37 ++- crates/ghostty-core/src/surface/binding.rs | 34 ++- crates/ghostty-crash/Cargo.toml | 3 + crates/ghostty-crash/src/darwin.rs | 135 +++++++++ crates/ghostty-crash/src/lib.rs | 6 + crates/ghostty-crash/src/minidump.rs | 88 +++++- crates/ghostty-crash/src/native.rs | 231 ++++++++++++++++ crates/ghostty-crash/src/sentry.rs | 12 +- crates/ghostty-crash/src/signal.rs | 15 + crates/ghostty-os/src/env.rs | 8 + crates/ghostty-os/src/flatpak.rs | 37 ++- crates/ghostty-os/src/i18n.rs | 13 +- crates/ghostty-os/src/osbytes.rs | 21 +- crates/ghostty-renderer/Cargo.toml | 2 +- crates/ghostty-renderer/src/shadertoy.rs | 51 +++- crates/ghostty-terminal/src/screen.rs | 48 +++- .../ghostty-terminal/src/screen/tests/p5.rs | 28 +- .../ghostty-terminal/src/terminal/tests/p8.rs | 6 +- crates/ghostty-termio/src/exec.rs | 47 ++-- .../ghostty-termio/src/shell_integration.rs | 24 +- crates/ghostty-vt-capi/build.rs | 47 +++- crates/ghostty-vt-capi/src/alloc.rs | 13 +- crates/ghostty-vt-capi/src/osc.rs | 160 ++++++++++- crates/ghostty-vt-capi/tests/c/alloc_test.c | 43 +++ crates/ghostty-vt-capi/tests/common/mod.rs | 16 ++ crates/ghostty-vt-capi/tests/soname.rs | 29 ++ crates/ghostty-vt/src/osc/mod.rs | 261 ++++++++++-------- .../src/osc/parsers/change_window_icon.rs | 13 +- .../src/osc/parsers/change_window_title.rs | 33 ++- .../src/osc/parsers/clipboard_operation.rs | 60 ++-- crates/ghostty-vt/src/osc/parsers/color.rs | 18 +- .../src/osc/parsers/context_signal.rs | 148 +++++----- .../ghostty-vt/src/osc/parsers/hyperlink.rs | 116 ++++---- crates/ghostty-vt/src/osc/parsers/iterm2.rs | 24 +- .../osc/parsers/kitty_clipboard_protocol.rs | 36 ++- .../ghostty-vt/src/osc/parsers/kitty_color.rs | 21 +- .../src/osc/parsers/kitty_text_sizing.rs | 78 ++++-- .../ghostty-vt/src/osc/parsers/mouse_shape.rs | 15 +- crates/ghostty-vt/src/osc/parsers/osc9.rs | 81 ++++-- .../ghostty-vt/src/osc/parsers/report_pwd.rs | 19 +- .../src/osc/parsers/rxvt_extension.rs | 58 ++-- .../src/osc/parsers/semantic_prompt.rs | 90 +++--- crates/ghostty-vt/src/parser.rs | 8 +- crates/ghostty-vt/src/stream.rs | 4 +- docs/rewrite/notes/final-Z5.md | 22 ++ scripts/install-lib.sh | 20 +- 49 files changed, 1785 insertions(+), 557 deletions(-) create mode 100644 crates/ghostty-crash/src/darwin.rs create mode 100644 crates/ghostty-crash/src/native.rs create mode 100644 crates/ghostty-vt-capi/tests/soname.rs create mode 100644 docs/rewrite/notes/final-Z5.md diff --git a/Cargo.lock b/Cargo.lock index 3e80d09..17d125b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1496,6 +1496,7 @@ dependencies = [ "ghostty-os", "libc", "log", + "mach2", "minidump", "minidump-writer", "serde_json", diff --git a/crates/ghostty-apprt-gtk/src/class/surface.rs b/crates/ghostty-apprt-gtk/src/class/surface.rs index fad1cf6..947532f 100644 --- a/crates/ghostty-apprt-gtk/src/class/surface.rs +++ b/crates/ghostty-apprt-gtk/src/class/surface.rs @@ -2542,7 +2542,12 @@ mod clipboard { return; } - show_clipboard_confirmation(surface, ClipboardRequest::Osc52Write(clipboard_type), &text); + show_clipboard_confirmation( + surface, + ClipboardRequest::Osc52Write(clipboard_type), + &text, + false, + ); } fn clipboard_to_u32(c: Clipboard) -> u32 { @@ -2596,7 +2601,7 @@ mod clipboard { match core.complete_clipboard_request(state, &text, false) { Ok(()) => {} Err(ClipboardError::UnsafePaste | ClipboardError::UnauthorizedPaste) => { - show_clipboard_confirmation(&this, state, &text); + show_clipboard_confirmation(&this, state, &text, false); return; } Err(err) => { @@ -2622,7 +2627,7 @@ mod clipboard { match core.complete_clipboard_request(ClipboardRequest::Paste, text, false) { Ok(()) => {} Err(ClipboardError::UnsafePaste | ClipboardError::UnauthorizedPaste) => { - show_clipboard_confirmation(surface, ClipboardRequest::Paste, text); + show_clipboard_confirmation(surface, ClipboardRequest::Paste, text, false); } Err(err) => log::warn!("failed to complete clipboard request err={err}"), } @@ -2642,7 +2647,7 @@ mod clipboard { Ok(()) => {} Err(ClipboardError::UnsafePaste | ClipboardError::UnauthorizedPaste) => { let text = ghostty_os::osbytes::bytes_to_config_string(bytes); - show_clipboard_confirmation(surface, ClipboardRequest::Paste, &text); + show_clipboard_confirmation(surface, ClipboardRequest::Paste, &text, true); } Err(err) => log::warn!("failed to complete clipboard request err={err}"), } @@ -2656,7 +2661,24 @@ mod clipboard { } } - fn show_clipboard_confirmation(surface: &Surface, req: ClipboardRequest, text: &str) { + /// Bytes to paste once a paste confirmation was accepted. Only text that + /// was byte-escaped for the dialog (`encoded`, from `paste_bytes`) is + /// decoded; ordinary text is pasted verbatim, even if it happens to + /// contain the escape character. + pub(super) fn confirmed_paste_bytes(text: &str, encoded: bool) -> Vec { + if encoded { + ghostty_os::osbytes::config_str_to_bytes(text) + } else { + text.as_bytes().to_vec() + } + } + + fn show_clipboard_confirmation( + surface: &Surface, + req: ClipboardRequest, + text: &str, + encoded: bool, + ) { // Build a text buffer for our contents let contents_buf = gtk::TextBuffer::new(None); contents_buf.insert_at_cursor(text); @@ -2688,8 +2710,8 @@ mod clipboard { // Get our text let text = buf.text(&buf.start_iter(), &buf.end_iter(), false); let result = if matches!(req, ClipboardRequest::Paste) { - // Undo the byte escape used for non-UTF-8 pastes. - core.complete_paste_bytes(&ghostty_os::osbytes::config_str_to_bytes(&text), true) + // Undo the byte escape only for non-UTF-8 pastes. + core.complete_paste_bytes(&confirmed_paste_bytes(&text, encoded), true) } else { core.complete_clipboard_request(req, &text, true) }; @@ -3874,6 +3896,19 @@ mod imp { mod tests { use super::*; + #[test] + fn confirmed_paste_decodes_only_encoded_text() { + // Ordinary text containing the escape char + U+00FF stays intact. + let text = "a\u{fdd0}\u{ff}b"; + assert_eq!( + clipboard::confirmed_paste_bytes(text, false), + text.as_bytes() + ); + // Text escaped by `paste_bytes` is decoded back to the raw bytes. + let enc = ghostty_os::osbytes::bytes_to_config_string(b"a\xffb"); + assert_eq!(clipboard::confirmed_paste_bytes(&enc, true), b"a\xffb"); + } + #[test] fn compute_fraction_clamps() { assert!((compute_fraction(100) - 1.0).abs() < f64::EPSILON); diff --git a/crates/ghostty-cli/src/list_themes.rs b/crates/ghostty-cli/src/list_themes.rs index f9631ab..9f0bd39 100644 --- a/crates/ghostty-cli/src/list_themes.rs +++ b/crates/ghostty-cli/src/list_themes.rs @@ -76,7 +76,7 @@ impl ThemeListElement { #[must_use] pub fn to_uri(&self) -> String { let mut out = String::from("file://"); - for b in self.path.bytes() { + for b in ghostty_os::osbytes::config_str_to_bytes(&self.path) { // std.Uri path component escaping: unreserved + sub-delims + ":@/". if b.is_ascii_alphanumeric() || matches!( @@ -399,6 +399,17 @@ mod tests { assert_eq!("file:///a%20b/c%25d", t.to_uri()); } + #[test] + fn uri_escaping_percent_encodes_original_bytes() { + let t = ThemeListElement { + location: Location::User, + path: ghostty_os::osbytes::bytes_to_config_string(b"/a/\xff\xc3x"), + theme: "x".into(), + rank: None, + }; + assert_eq!("file:///a/%FF%C3x", t.to_uri()); + } + #[test] fn options_color() { let mut opts = Options::default(); diff --git a/crates/ghostty-config/src/config.rs b/crates/ghostty-config/src/config.rs index 6f90072..c44af9a 100644 --- a/crates/ghostty-config/src/config.rs +++ b/crates/ghostty-config/src/config.rs @@ -3940,13 +3940,29 @@ impl Config { } } + /// The default template with the (decoded, byte-exact) path substituted. + fn render_config_template(path: &str) -> Vec { + let tmpl = include_str!("config-template"); + let path_bytes = ghostty_os::osbytes::config_str_to_bytes(path); + let mut out = Vec::with_capacity(tmpl.len() + path_bytes.len()); + for (i, part) in tmpl.split("{[path]s}").enumerate() { + if i > 0 { + out.extend_from_slice(&path_bytes); + } + out.extend_from_slice(part.as_bytes()); + } + out + } + fn write_config_template(path: &str) -> std::io::Result<()> { log::info!("creating template config file: path={path}"); - if let Some(dir) = std::path::Path::new(path).parent() { + // `path` carries the reversible byte encoding; decode it for the OS. + let os_path = ghostty_os::osbytes::config_str_to_path(path); + if let Some(dir) = os_path.parent() { std::fs::create_dir_all(dir)?; } - let template = include_str!("config-template").replace("{[path]s}", path); - std::fs::write(path, template) + let template = Self::render_config_template(path); + std::fs::write(os_path, template) } /// Load configurations from the default configuration files. The default @@ -4928,6 +4944,21 @@ impl CliTarget for Config { clippy::assert_is_empty )] mod tests { + #[cfg(unix)] + #[test] + fn config_template_written_at_non_utf8_path() { + use std::os::unix::ffi::OsStrExt; + let td = ghostty_os::TempDir::init().unwrap(); + let dir = td.path().join(std::ffi::OsStr::from_bytes(b"cfg\xff")); + let file = dir.join("config.ghostty"); + let enc = ghostty_os::osbytes::path_to_config_string(&file); + Config::write_config_template(&enc).unwrap(); + let data = std::fs::read(&file).unwrap(); + let raw = file.as_os_str().as_bytes(); + assert!(data.windows(raw.len()).any(|w| w == raw)); + assert!(!data.windows(3).any(|w| w == "\u{fdd0}".as_bytes())); + } + use std::io::Cursor; use ghostty_os::TempDir; diff --git a/crates/ghostty-core/src/surface/binding.rs b/crates/ghostty-core/src/surface/binding.rs index ce865bc..1c70687 100644 --- a/crates/ghostty-core/src/surface/binding.rs +++ b/crates/ghostty-core/src/surface/binding.rs @@ -440,9 +440,11 @@ pub fn write_screen_temp_file( let mut file = opts.open(&file_path)?; file.write_all(contents.as_bytes())?; file.flush()?; - Ok(std::fs::canonicalize(&file_path)? - .to_string_lossy() - .into_owned()) + // Reversible config-string encoding: a non-UTF-8 temp dir keeps its + // bytes (decode with `config_str_to_path` at OS boundaries). + Ok(ghostty_os::osbytes::path_to_config_string( + &std::fs::canonicalize(&file_path)?, + )) })(); match result { @@ -628,7 +630,11 @@ impl Surface { WriteScreenAction::Copy => { self.rt_surface.set_clipboard( Clipboard::Standard, - &[("text/plain".to_owned(), path)], + &[( + "text/plain".to_owned(), + String::from_utf8_lossy(&ghostty_os::osbytes::config_str_to_bytes(&path)) + .into_owned(), + )], false, ); } @@ -639,7 +645,9 @@ impl Surface { }, url: path, })?, - WriteScreenAction::Paste => self.queue_io(IoMessage::write_req(path.as_bytes())), + WriteScreenAction::Paste => self.queue_io(IoMessage::write_req( + &ghostty_os::osbytes::config_str_to_bytes(&path), + )), } Ok(()) } @@ -1282,6 +1290,22 @@ mod tests { let _ = std::fs::remove_dir_all(std::path::Path::new(&path).parent().unwrap()); } + #[cfg(unix)] + #[test] + fn screen_temp_file_returns_encoded_non_utf8_path() { + use std::os::unix::ffi::OsStrExt; + let td = ghostty_os::temp_dir::TempDir::init().unwrap(); + let bad = td.path().join(std::ffi::OsStr::from_bytes(b"d\xff")); + std::fs::create_dir_all(&bad).unwrap(); + // Use TMPDIR to force the non-UTF-8 directory. + let file = bad.join("x.txt"); + std::fs::write(&file, b"x").unwrap(); + let canon = std::fs::canonicalize(&file).unwrap(); + let enc = ghostty_os::osbytes::path_to_config_string(&canon); + assert!(enc.contains('\u{fdd0}')); + assert_eq!(ghostty_os::osbytes::config_str_to_path(&enc), canon); + } + #[test] fn adjust_selection_without_selection() { let mut t = term(10, 3); diff --git a/crates/ghostty-crash/Cargo.toml b/crates/ghostty-crash/Cargo.toml index 96513ae..300b401 100644 --- a/crates/ghostty-crash/Cargo.toml +++ b/crates/ghostty-crash/Cargo.toml @@ -19,6 +19,9 @@ thiserror = "2.0.21" [target.'cfg(unix)'.dependencies] libc = "0.2" +[target.'cfg(target_vendor = "apple")'.dependencies] +mach2 = "0.6" + [target.'cfg(target_os = "linux")'.dependencies] minidump-writer = "0.13.0" diff --git a/crates/ghostty-crash/src/darwin.rs b/crates/ghostty-crash/src/darwin.rs new file mode 100644 index 0000000..df8bc38 --- /dev/null +++ b/crates/ghostty-crash/src/darwin.rs @@ -0,0 +1,135 @@ +//! Darwin glue for [`crate::native`]: fill the native crash context from the +//! `siginfo_t` and `ucontext_t` passed to the fatal-signal handler. +//! +//! The handler runs on the faulting thread, so the `ucontext` holds the +//! thread state at the fault (the equivalent of `thread_get_state` on the +//! crashed thread that breakpad's Mach exception handler does). Memory is read +//! with `mach_vm_read_overwrite`, which returns an error instead of faulting +//! for unmapped addresses, so walking a corrupted stack is safe. +#![allow(unsafe_code)] + +use crate::native::Native; + +/// `struct __darwin_ucontext` (``, 64-bit). +#[repr(C)] +#[allow(clippy::struct_field_names)] +struct Ucontext { + uc_onstack: libc::c_int, + uc_sigmask: u32, + uc_stack: libc::stack_t, + uc_link: *mut Ucontext, + uc_mcsize: usize, + /// `struct __darwin_mcontext64 *`: exception state (16 bytes on both + /// x86_64 and arm64) followed by the thread state. + uc_mcontext: *const u8, +} + +#[cfg(target_arch = "aarch64")] +const X_NAMES: [&str; 29] = [ + "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8", "x9", "x10", "x11", "x12", "x13", "x14", + "x15", "x16", "x17", "x18", "x19", "x20", "x21", "x22", "x23", "x24", "x25", "x26", "x27", + "x28", +]; + +/// Size of `__darwin_x86_exception_state64` / `__darwin_arm_exception_state64`, +/// which precedes the thread state in `__darwin_mcontext64`. +const EXCEPTION_STATE_SIZE: usize = 16; + +fn read_mem(addr: u64) -> Option<[u8; 16]> { + let mut buf = [0u8; 16]; + let mut out: mach2::vm_types::mach_vm_size_t = 0; + // SAFETY: `buf` is a valid 16 byte destination; the call validates the + // source address and fails instead of faulting. + let kr = unsafe { + mach2::vm::mach_vm_read_overwrite( + mach2::traps::mach_task_self(), + addr, + 16, + buf.as_mut_ptr() as mach2::vm_types::mach_vm_address_t, + &raw mut out, + ) + }; + (kr == mach2::kern_return::KERN_SUCCESS && out == 16).then_some(buf) +} + +/// Fill `n` from the handler arguments. +/// +/// # Safety +/// `info` and `uc` must be the pointers the kernel passed to an `SA_SIGINFO` +/// handler (or null). +pub(crate) unsafe fn capture( + n: &mut Native, + info: *const libc::siginfo_t, + uc: *const libc::c_void, +) { + // SAFETY: per the contract (a non-null siginfo is valid for reads). + if let Some(info) = unsafe { info.as_ref() } { + n.signo = info.si_signo; + n.code = info.si_code; + n.addr = info.si_addr as u64; + } + // SAFETY: per the contract. + let Some(uc) = (unsafe { uc.cast::().as_ref() }) else { + return; + }; + if uc.uc_mcontext.is_null() { + return; + } + // SAFETY: the mcontext holds the exception state followed by the thread + // state; the kernel built it for this signal frame. + let ss = unsafe { uc.uc_mcontext.add(EXCEPTION_STATE_SIZE) }; + + #[cfg(target_arch = "x86_64")] + { + // SAFETY: as above; the layout is `x86_thread_state64_t`. + let s = unsafe { &*ss.cast::() }; + for (name, v) in [ + ("rax", s.__rax), + ("rbx", s.__rbx), + ("rcx", s.__rcx), + ("rdx", s.__rdx), + ("rdi", s.__rdi), + ("rsi", s.__rsi), + ("rbp", s.__rbp), + ("rsp", s.__rsp), + ("r8", s.__r8), + ("r9", s.__r9), + ("r10", s.__r10), + ("r11", s.__r11), + ("r12", s.__r12), + ("r13", s.__r13), + ("r14", s.__r14), + ("r15", s.__r15), + ("rip", s.__rip), + ("rflags", s.__rflags), + ("cs", s.__cs), + ("fs", s.__fs), + ("gs", s.__gs), + ] { + n.push_reg(name, v); + } + n.walk(s.__rip, s.__rbp, read_mem); + } + + #[cfg(target_arch = "aarch64")] + { + // SAFETY: as above; the layout is `arm_thread_state64_t`. + let s = unsafe { &*ss.cast::() }; + for (name, v) in X_NAMES.iter().zip(s.__x) { + n.push_reg(name, v); + } + n.push_reg("fp", s.__fp); + n.push_reg("lr", s.__lr); + n.push_reg("sp", s.__sp); + n.push_reg("pc", s.__pc); + n.push_reg("cpsr", u64::from(s.__cpsr)); + // arm64 may sign pointers (PAC): strip the high bits before use. + let strip = |v: u64| v & 0x0000_007f_ffff_ffff; + // The pc is exact; the faulting leaf function did not necessarily + // push a frame record, so the link register is the caller. + n.walk(s.__pc, strip(s.__fp), read_mem); + if n.nframes == 1 && s.__lr != 0 { + n.push_frame(strip(s.__lr)); + } + } +} diff --git a/crates/ghostty-crash/src/lib.rs b/crates/ghostty-crash/src/lib.rs index cc0683d..66748f6 100644 --- a/crates/ghostty-crash/src/lib.rs +++ b/crates/ghostty-crash/src/lib.rs @@ -9,10 +9,16 @@ #![cfg_attr(windows, allow(dead_code))] pub mod context; +#[cfg(target_vendor = "apple")] +mod darwin; pub mod dir; pub mod envelope; #[cfg(target_os = "linux")] mod minidump; +// Pure (platform independent) event-context formatting; only used by the +// Darwin signal path but unit-tested everywhere. +#[cfg_attr(not(target_vendor = "apple"), allow(dead_code))] +mod native; pub mod sentry; #[cfg(unix)] mod signal; diff --git a/crates/ghostty-crash/src/minidump.rs b/crates/ghostty-crash/src/minidump.rs index 231896f..79b05fe 100644 --- a/crates/ghostty-crash/src/minidump.rs +++ b/crates/ghostty-crash/src/minidump.rs @@ -92,7 +92,7 @@ pub(crate) unsafe fn crash_context( unsafe { std::ptr::copy_nonoverlapping(uc.cast::(), &raw mut cc.context, 1); } - #[cfg(not(target_arch = "arm"))] + #[cfg(any(target_arch = "x86_64", target_arch = "x86"))] { let fp = cc.context.uc_mcontext.fpregs; if !fp.is_null() { @@ -101,10 +101,61 @@ pub(crate) unsafe fn crash_context( unsafe { std::ptr::copy_nonoverlapping(fp, &raw mut cc.float_state, 1) }; } } + // AArch64 has no `fpregs` pointer: the FP/SIMD state is the + // `fpsimd_context` record in the chain of context extension records + // stored in `mcontext_t::__reserved`. + #[cfg(target_arch = "aarch64")] + { + // SAFETY: `__reserved` is plain initialized memory inside `cc`. + let reserved: &[u8] = unsafe { + std::slice::from_raw_parts( + (&raw const cc.context.uc_mcontext.__reserved).cast::(), + size_of_val(&cc.context.uc_mcontext.__reserved), + ) + }; + if let Some(off) = fpsimd_offset(reserved) + && off + size_of::() <= reserved.len() + { + // SAFETY: bounds checked above; both sides are plain old + // data and the destination is our static (unaligned source). + unsafe { + std::ptr::copy_nonoverlapping( + reserved.as_ptr().add(off), + (&raw mut cc.float_state).cast::(), + size_of::(), + ); + } + } + } } cc } +/// Magic of the AArch64 `fpsimd_context` record (`FPSIMD_MAGIC`). +#[cfg_attr(not(target_arch = "aarch64"), allow(dead_code))] +const FPSIMD_MAGIC: u32 = 0x4650_8001; + +/// Walk the AArch64 context extension records (`struct _aarch64_ctx`: u32 +/// magic, u32 size, native endian) in `mcontext_t::__reserved` and return the +/// byte offset of the `fpsimd_context` record, if present. The chain ends at +/// a zero magic. Pure, so it is testable on every architecture. +#[cfg_attr(not(target_arch = "aarch64"), allow(dead_code))] +fn fpsimd_offset(reserved: &[u8]) -> Option { + let mut off = 0usize; + while off + 8 <= reserved.len() { + let magic = u32::from_ne_bytes(reserved[off..off + 4].try_into().ok()?); + let size = u32::from_ne_bytes(reserved[off + 4..off + 8].try_into().ok()?) as usize; + if magic == FPSIMD_MAGIC { + return Some(off); + } + if magic == 0 || size < 8 { + return None; + } + off += size; + } + None +} + /// Fork the dumping child and wait for it. Returns true if the envelope with /// the minidump has been written to `cap.path`. pub(crate) fn capture(cap: &Capture<'_>, cc: &CrashContext) -> bool { @@ -324,3 +375,38 @@ fn write_envelope(cap: &Capture<'_>, dump: &[u8]) -> bool { } ok } + +#[cfg(test)] +mod tests { + use super::{FPSIMD_MAGIC, fpsimd_offset}; + + fn record(magic: u32, size: u32, out: &mut Vec) { + out.extend_from_slice(&magic.to_ne_bytes()); + out.extend_from_slice(&size.to_ne_bytes()); + out.resize(out.len() + size as usize - 8, 0); + } + + #[test] + fn finds_fpsimd_record_in_extension_chain() { + let mut buf = Vec::new(); + // An unrelated record (e.g. esr_context) first, then fpsimd. + record(0x4553_5201, 16, &mut buf); + let at = buf.len(); + record(FPSIMD_MAGIC, 528, &mut buf); + record(0, 8, &mut buf); + assert_eq!(fpsimd_offset(&buf), Some(at)); + } + + #[test] + fn missing_or_corrupt_chain_yields_none() { + let mut buf = Vec::new(); + record(0, 8, &mut buf); + assert_eq!(fpsimd_offset(&buf), None); + assert_eq!(fpsimd_offset(&[]), None); + // A record with a bogus size must not loop forever. + let mut bad = Vec::new(); + bad.extend_from_slice(&1u32.to_ne_bytes()); + bad.extend_from_slice(&0u32.to_ne_bytes()); + assert_eq!(fpsimd_offset(&bad), None); + } +} diff --git a/crates/ghostty-crash/src/native.rs b/crates/ghostty-crash/src/native.rs new file mode 100644 index 0000000..b4debb6 --- /dev/null +++ b/crates/ghostty-crash/src/native.rs @@ -0,0 +1,231 @@ +//! Native crash context for the event JSON: signal info, CPU registers and a +//! stack trace of the faulting thread. +//! +//! This is what sentry-native's signal handler records on macOS (where +//! Ghostty's Zig build links sentry with the breakpad backend, whose handler +//! captures the faulting thread's machine state): the `siginfo`, the thread +//! state (registers) and the stack. We have no minidump writer that can run +//! from a signal handler on Darwin (a Mach exception port handler thread +//! would be required), so the same information goes into the event itself as +//! `exception.values[0].stacktrace` (`frames` + `registers`) and +//! `mechanism.meta.signal`, following the Sentry event schema. +//! +//! Everything here is allocation free and architecture independent (the +//! Darwin glue in `darwin.rs` only fills [`Native`] from the `ucontext`), so +//! it is unit-tested on every platform. + +use std::fmt; + +/// Maximum number of stack frames recorded. +pub(crate) const MAX_FRAMES: usize = 64; +/// Maximum number of registers recorded (x86_64: 21, arm64: 34). +pub(crate) const MAX_REGS: usize = 40; + +/// Machine/signal context of the faulting thread. +pub(crate) struct Native { + pub signo: i32, + pub code: i32, + /// Fault address (`si_addr`). + pub addr: u64, + pub reg_names: [&'static str; MAX_REGS], + pub reg_vals: [u64; MAX_REGS], + pub nregs: usize, + /// Return addresses, innermost first (`frames[0]` is the pc). + pub frames: [u64; MAX_FRAMES], + pub nframes: usize, +} + +impl Native { + pub(crate) const fn new() -> Native { + Native { + signo: 0, + code: 0, + addr: 0, + reg_names: [""; MAX_REGS], + reg_vals: [0; MAX_REGS], + nregs: 0, + frames: [0; MAX_FRAMES], + nframes: 0, + } + } + + pub(crate) fn push_reg(&mut self, name: &'static str, val: u64) { + if self.nregs < MAX_REGS { + self.reg_names[self.nregs] = name; + self.reg_vals[self.nregs] = val; + self.nregs += 1; + } + } + + pub(crate) fn push_frame(&mut self, addr: u64) { + if self.nframes < MAX_FRAMES { + self.frames[self.nframes] = addr; + self.nframes += 1; + } + } + + /// Record the pc and then walk the frame-pointer chain starting at `fp`. + /// + /// Both x86_64 and arm64 Darwin keep a frame record `[prev_fp, + /// return_address]` at `fp`. `read` copies 16 bytes of the crashed + /// process' memory, returning false instead of faulting when the address + /// is unmapped. The walk stops at a null/misaligned/non-increasing frame + /// pointer, a failed read or a null return address. + pub(crate) fn walk( + &mut self, + pc: u64, + mut fp: u64, + mut read: impl FnMut(u64) -> Option<[u8; 16]>, + ) { + self.push_frame(pc); + let mut prev = 0u64; + while self.nframes < MAX_FRAMES && fp != 0 && fp.is_multiple_of(8) && fp > prev { + let Some(rec) = read(fp) else { break }; + let next = u64::from_ne_bytes([ + rec[0], rec[1], rec[2], rec[3], rec[4], rec[5], rec[6], rec[7], + ]); + let ret = u64::from_ne_bytes([ + rec[8], rec[9], rec[10], rec[11], rec[12], rec[13], rec[14], rec[15], + ]); + if ret == 0 { + break; + } + self.push_frame(ret); + prev = fp; + fp = next; + } + } +} + +/// `si_code` names for the fault signals (POSIX / Darwin ``). +fn code_name(signo: i32, code: i32) -> &'static str { + match (signo, code) { + (11, 1) => "SEGV_MAPERR", + (11, 2) => "SEGV_ACCERR", + (10, 1) => "BUS_ADRALN", + (10, 2) => "BUS_ADRERR", + (10, 3) => "BUS_OBJERR", + (4, 1) => "ILL_ILLOPC", + (4, 2) => "ILL_ILLTRP", + (4, 3) => "ILL_PRVOPC", + (8, 1) => "FPE_INTDIV", + (8, 2) => "FPE_INTOVF", + (8, 3) => "FPE_FLTDIV", + (8, 4) => "FPE_FLTOVF", + (8, 5) => "FPE_FLTUND", + (8, 6) => "FPE_FLTRES", + (8, 7) => "FPE_FLTINV", + (_, 0) => "SI_USER", + _ => "", + } +} + +/// Write `"meta":{...}` (inside the mechanism object) and, after the +/// mechanism, the `stacktrace` of the exception value. The caller has just +/// written `"mechanism":{"type":..,"handled":false`; this appends the +/// remaining mechanism fields and closes the mechanism object. +pub(crate) fn write_mechanism_rest_and_stacktrace( + w: &mut W, + n: &Native, +) -> fmt::Result { + write!( + w, + ",\"meta\":{{\"signal\":{{\"number\":{},\"code\":{}", + n.signo, n.code + )?; + let name = code_name(n.signo, n.code); + if !name.is_empty() { + write!(w, ",\"code_name\":\"{name}\"")?; + } + write!( + w, + "}}}},\"data\":{{\"relevant_address\":\"0x{:x}\"}}}}", + n.addr + )?; + + // Sentry wants frames oldest first. + w.write_str(",\"stacktrace\":{\"frames\":[")?; + for (i, addr) in n.frames[..n.nframes].iter().rev().enumerate() { + if i > 0 { + w.write_char(',')?; + } + write!(w, "{{\"instruction_addr\":\"0x{addr:x}\"}}")?; + } + w.write_str("],\"registers\":{")?; + for i in 0..n.nregs { + if i > 0 { + w.write_char(',')?; + } + write!(w, "\"{}\":\"0x{:x}\"", n.reg_names[i], n.reg_vals[i])?; + } + w.write_str("}}") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample() -> Native { + let mut n = Native::new(); + n.signo = 11; + n.code = 1; + n.addr = 0xdead_beef; + n.push_reg("pc", 0x1000); + n.push_reg("sp", 0x7000); + n.walk(0x1000, 0x7000, |fp| { + // Two frames: [0x7100, 0x2000], [0x7200, 0x3000], then end. + let (next, ret) = match fp { + 0x7000 => (0x7100u64, 0x2000u64), + 0x7100 => (0x7200, 0x3000), + _ => return None, + }; + let mut b = [0u8; 16]; + b[..8].copy_from_slice(&next.to_ne_bytes()); + b[8..].copy_from_slice(&ret.to_ne_bytes()); + Some(b) + }); + n + } + + #[test] + fn walks_frame_pointer_chain() { + let n = sample(); + assert_eq!(&n.frames[..n.nframes], &[0x1000, 0x2000, 0x3000]); + } + + #[test] + fn walk_stops_on_cycles_and_bad_pointers() { + let mut n = Native::new(); + // fp chain pointing at itself: must terminate. + n.walk(1, 0x7000, |_| { + let mut b = [0u8; 16]; + b[..8].copy_from_slice(&0x7000u64.to_ne_bytes()); + b[8..].copy_from_slice(&0x42u64.to_ne_bytes()); + Some(b) + }); + assert_eq!(&n.frames[..n.nframes], &[1, 0x42]); + // Misaligned fp: no read at all. + let mut m = Native::new(); + m.walk(1, 0x7001, |_| panic!("read")); + assert_eq!(m.nframes, 1); + } + + #[test] + fn event_json_is_valid_with_registers_and_frames() { + let n = sample(); + let mut s = String::from("{\"mechanism\":{\"type\":\"signalhandler\",\"handled\":false"); + write_mechanism_rest_and_stacktrace(&mut s, &n).expect("format"); + s.push('}'); + let v: serde_json::Value = serde_json::from_str(&s).expect("valid json"); + let mech = &v["mechanism"]; + assert_eq!(mech["meta"]["signal"]["number"], 11); + assert_eq!(mech["meta"]["signal"]["code_name"], "SEGV_MAPERR"); + assert_eq!(mech["data"]["relevant_address"], "0xdeadbeef"); + let frames = v["stacktrace"]["frames"].as_array().expect("array"); + // Oldest first. + assert_eq!(frames[0]["instruction_addr"], "0x3000"); + assert_eq!(frames[2]["instruction_addr"], "0x1000"); + assert_eq!(v["stacktrace"]["registers"]["pc"], "0x1000"); + assert_eq!(v["stacktrace"]["registers"]["sp"], "0x7000"); + } +} diff --git a/crates/ghostty-crash/src/sentry.rs b/crates/ghostty-crash/src/sentry.rs index 5701faa..245318b 100644 --- a/crates/ghostty-crash/src/sentry.rs +++ b/crates/ghostty-crash/src/sentry.rs @@ -242,6 +242,9 @@ pub(crate) struct Crash<'a> { pub value: &'a str, /// Sentry mechanism type: `panic` or `signalhandler`. pub mechanism: &'a str, + /// Signal info, registers and stack of the faulting thread (Darwin + /// signal path; see `native.rs`). + pub native: Option<&'a crate::native::Native>, } /// Write the event JSON (single line). Mirrors what Sentry produces after @@ -281,7 +284,13 @@ pub(crate) fn write_event(w: &mut W, st: &State, c: &Crash<'_>) - json_str(w, c.value)?; w.write_str(",\"mechanism\":{\"type\":")?; json_str(w, c.mechanism)?; - w.write_str(",\"handled\":false}}]}}") + w.write_str(",\"handled\":false")?; + if let Some(n) = c.native { + crate::native::write_mechanism_rest_and_stacktrace(w, n)?; + w.write_str("}]}}") + } else { + w.write_str("}}]}}") + } } /// Write the envelope header line and the event item header line (the event @@ -362,6 +371,7 @@ fn write_report( kind, value, mechanism, + native: None, }; let mut event = String::new(); diff --git a/crates/ghostty-crash/src/signal.rs b/crates/ghostty-crash/src/signal.rs index 13e7310..035a234 100644 --- a/crates/ghostty-crash/src/signal.rs +++ b/crates/ghostty-crash/src/signal.rs @@ -34,6 +34,8 @@ struct Scratch { event: [u8; EVENT_CAP], head: [u8; HEAD_CAP], path: [u8; PATH_CAP], + #[cfg(target_vendor = "apple")] + native: crate::native::Native, } struct ScratchCell(UnsafeCell); @@ -44,6 +46,8 @@ static SCRATCH: ScratchCell = ScratchCell(UnsafeCell::new(Scratch { event: [0; EVENT_CAP], head: [0; HEAD_CAP], path: [0; PATH_CAP], + #[cfg(target_vendor = "apple")] + native: crate::native::Native::new(), })); static BUSY: AtomicBool = AtomicBool::new(false); @@ -150,6 +154,16 @@ fn write_signal_report(name: &str, desc: &str, info: *mut libc::siginfo_t, uc: * return; }; let (secs, micros) = sentry::now(); + // Darwin: record the faulting thread's registers and stack in the event + // (no minidump there; see `native.rs`). + #[cfg(target_vendor = "apple")] + let native_ctx = { + // SAFETY: `info`/`uc` are the pointers the kernel gave the handler. + unsafe { crate::darwin::capture(&mut sc.native, info, uc) }; + Some(&sc.native) + }; + #[cfg(not(target_vendor = "apple"))] + let native_ctx = None; let crash = Crash { uuid, secs, @@ -157,6 +171,7 @@ fn write_signal_report(name: &str, desc: &str, info: *mut libc::siginfo_t, uc: * kind: name, value: desc, mechanism: "signalhandler", + native: native_ctx, }; let mut ev = SliceWriter { diff --git a/crates/ghostty-os/src/env.rs b/crates/ghostty-os/src/env.rs index 1ab261c..6aa3867 100644 --- a/crates/ghostty-os/src/env.rs +++ b/crates/ghostty-os/src/env.rs @@ -203,6 +203,14 @@ pub fn append_env_always(current: &str, value: &str) -> String { format!("{current}{DELIMITER}{value}") } +/// Byte-preserving [`append_env_always`]. +pub fn append_env_always_os(current: &OsStr, value: &OsStr) -> OsString { + let mut out = current.to_owned(); + out.push(DELIMITER.to_string()); + out.push(value); + out +} + /// Byte-preserving [`prepend_env`]. pub fn prepend_env_os(current: &OsStr, value: &OsStr) -> OsString { if current.is_empty() { diff --git a/crates/ghostty-os/src/flatpak.rs b/crates/ghostty-os/src/flatpak.rs index ef40ce7..240c06b 100644 --- a/crates/ghostty-os/src/flatpak.rs +++ b/crates/ghostty-os/src/flatpak.rs @@ -296,6 +296,14 @@ impl FlatpakSignalHandle { } } +/// Decode a reversible config string to the original bytes plus NUL, the +/// D-Bus `ay` form of a path/argument. +fn nul_terminated_bytes(s: &str) -> Vec { + let mut v = crate::osbytes::config_str_to_bytes(s); + v.push(0); + v +} + struct Request { argv: Vec, cwd: Option, @@ -346,22 +354,26 @@ fn start( .argv .iter() .map(|a| { - let mut v = a.as_bytes().to_vec(); + let mut v = crate::osbytes::config_str_to_bytes(a); v.push(0); v }) .collect(); // Get the cwd in case we don't have ours set. - let cwd = match request.cwd { - Some(c) => c, - None => std::env::current_dir() - .map_err(|_| Error::FlatpakSetupFail)? - .to_string_lossy() - .into_owned(), + // D-Bus `ay`: the original bytes (request.cwd carries the reversible + // config-string encoding). + let cwd = if let Some(c) = request.cwd { + nul_terminated_bytes(&c) + } else { + let mut v = crate::osbytes::os_bytes( + std::env::current_dir() + .map_err(|_| Error::FlatpakSetupFail)? + .as_os_str(), + ); + v.push(0); + v }; - let mut cwd = cwd.into_bytes(); - cwd.push(0); // Subscribe to exit notifications (before the call so we can't miss it). let rule = MatchRule::builder() @@ -429,6 +441,13 @@ fn wait_for_exit(shared: &Shared, iter: MessageIterator, pid: u32) { #[cfg(test)] mod tests { + #[test] + fn dbus_bytes_are_decoded() { + let enc = crate::osbytes::bytes_to_config_string(b"/tmp/\xff\xfe"); + assert_ne!(enc.as_bytes(), b"/tmp/\xff\xfe"); + assert_eq!(super::nul_terminated_bytes(&enc), b"/tmp/\xff\xfe\0"); + } + use super::*; #[test] diff --git a/crates/ghostty-os/src/i18n.rs b/crates/ghostty-os/src/i18n.rs index a311c05..36503ba 100644 --- a/crates/ghostty-os/src/i18n.rs +++ b/crates/ghostty-os/src/i18n.rs @@ -1,6 +1,3 @@ -#[cfg(not(windows))] -use std::path::Path; - use crate::i18n_locales::LOCALES; /// Application bundle ID, used as the gettext domain @@ -42,16 +39,18 @@ pub fn init(resources_dir: &str) -> Result<(), InitError> { // is standard for translations. // (`std.fs.path.dirname` returns null for a bare relative name, where // `Path::parent` returns an empty path.) - let share_dir = Path::new(resources_dir) + let resources_dir = crate::osbytes::config_str_to_path(resources_dir); + let share_dir = resources_dir .parent() .filter(|p| !p.as_os_str().is_empty()) .ok_or(InitError::InvalidResourcesDir)?; // Build our locale path - let path = format!("{}/locale", share_dir.display()); + let path = share_dir.join("locale"); - // Bind our bundle ID to the given locale path - log::debug!("binding domain={BUNDLE_ID} path={path}"); + // Bind our bundle ID to the given locale path (gettextrs takes a + // `Into`, so non-UTF-8 bytes are preserved). + log::debug!("binding domain={BUNDLE_ID} path={}", path.display()); gettextrs::bindtextdomain(BUNDLE_ID, path).map_err(|_| InitError::OutOfMemory)?; Ok(()) } diff --git a/crates/ghostty-os/src/osbytes.rs b/crates/ghostty-os/src/osbytes.rs index ad68666..fec4db4 100644 --- a/crates/ghostty-os/src/osbytes.rs +++ b/crates/ghostty-os/src/osbytes.rs @@ -100,10 +100,18 @@ pub fn os_to_config_string(s: &OsStr) -> String { } #[cfg(not(unix))] { - s.to_string_lossy().into_owned() + valid_str_to_config_string(&s.to_string_lossy()) } } +/// Encode a string that is already valid UTF-8 (e.g. the WTF-8 to UTF-8 +/// conversion of a non-Unix OS string). The only thing that needs escaping is +/// a literal escape character, so decoding stays a bijection on every +/// platform. Shared (not cfg gated) so it is testable everywhere. +pub fn valid_str_to_config_string(s: &str) -> String { + bytes_to_config_string(s.as_bytes()) +} + /// Inverse of [`os_to_config_string`]. pub fn config_str_to_os(s: &str) -> OsString { os_from_bytes(config_str_to_bytes(s)) @@ -142,6 +150,17 @@ mod tests { assert_eq!(config_str_to_bytes(icons), icons.as_bytes()); } + #[test] + fn valid_str_encoder_escapes_literal_escape_char() { + // Shared logic of the non-Unix encoder: U+FDD0 U+00FF must not be + // mistaken for the escape of byte 0xFF. + let s = "a\u{fdd0}\u{ff}"; + let e = valid_str_to_config_string(s); + assert_eq!(e, "a\u{fdd0}\u{fdd0}\u{ff}"); + assert_eq!(config_str_to_bytes(&e), s.as_bytes()); + assert_eq!(valid_str_to_config_string("plain"), "plain"); + } + #[test] fn literal_escape_char_is_doubled() { let s = "a\u{fdd0}\u{ff}"; diff --git a/crates/ghostty-renderer/Cargo.toml b/crates/ghostty-renderer/Cargo.toml index 75110ce..bc3a51d 100644 --- a/crates/ghostty-renderer/Cargo.toml +++ b/crates/ghostty-renderer/Cargo.toml @@ -9,6 +9,7 @@ bytemuck = { version = "1.25.2", features = ["derive"] } crossbeam-channel = "0.5.17" ghostty-config.workspace = true ghostty-crash.workspace = true +ghostty-os.workspace = true ghostty-datastruct.workspace = true ghostty-font.workspace = true ghostty-input.workspace = true @@ -61,7 +62,6 @@ inline_always = "allow" [target.'cfg(target_vendor = "apple")'.dependencies] block2 = "0.6.2" dispatch2 = "0.3.1" -ghostty-os.workspace = true objc2 = "0.6.5" objc2-core-foundation = "0.3.2" objc2-core-graphics = "0.3.2" diff --git a/crates/ghostty-renderer/src/shadertoy.rs b/crates/ghostty-renderer/src/shadertoy.rs index 1611e73..dc63bac 100644 --- a/crates/ghostty-renderer/src/shadertoy.rs +++ b/crates/ghostty-renderer/src/shadertoy.rs @@ -124,7 +124,7 @@ pub fn load_from_files(paths: &[PathItem], target: Target) -> Result PathItem::Required(p) => (p, false), }; - let shader = match load_from_file(Path::new(path), target) { + let shader = match load_from_file(&ghostty_os::osbytes::config_str_to_path(path), target) { Ok(s) => s, Err(Error::Io(e)) if optional && e.kind() == std::io::ErrorKind::NotFound => { continue; @@ -262,6 +262,22 @@ fn adapt_for_naga(full: &str) -> String { .replacen("#version 430 core", "#version 460 core", 1) .replace(" uniform ", " ") .replacen("layout(location = 0) in vec4 gl_FragCoord;", "", 1) + // std140 pads `float iChannelTime[4]` to stride 16, which naga types + // differently from a plain local `float[4]`, so whole-array loads and + // copies (`float t[4] = iChannelTime;`, passing it to a function) do + // not validate. The block member is renamed and `iChannelTime` becomes + // a function that reconstructs the plain array from the padded + // elements; element reads (`iChannelTime[i]`) keep working. + .replacen( + " float iChannelTime[4];\n", + " float iChannelTimeUbo[4];\n", + 1, + ) + .replacen( + "\n#define CURSORSTYLE_BLOCK ", + "\nfloat[4] iChannelTime_plain() {\n return float[4](iChannelTimeUbo[0], iChannelTimeUbo[1], iChannelTimeUbo[2], iChannelTimeUbo[3]);\n}\n#define iChannelTime iChannelTime_plain()\n\n#define CURSORSTYLE_BLOCK ", + 1, + ) .replacen( "layout(binding = 0) uniform sampler2D iChannel0;", "layout(set = 0, binding = 0) uniform texture2D iChannel0_tex;\n\ @@ -803,10 +819,32 @@ mod tests { #[test] fn msl_uniform_block_matches_std140_upload_layout() { - use std::mem::offset_of as o; // A shader that reads the scalar array so the accessor path is used. - let user = "void mainImage(out vec4 c, in vec2 p) { \ - int i = int(p.x) & 3; c = vec4(iChannelTime[i], iChannelTime[2], iTime, iSampleRate); }"; + check_msl_layout( + "void mainImage(out vec4 c, in vec2 p) { \ + int i = int(p.x) & 3; c = vec4(iChannelTime[i], iChannelTime[2], iTime, iSampleRate); }", + ); + } + + #[test] + fn whole_array_copy_of_padded_uniform_validates_and_keeps_layout() { + // Whole-array loads/copies of the std140-padded `iChannelTime` must + // validate (naga types the padded array differently from a plain + // local `float[4]`) and not disturb the MSL block layout. + let user = "float sum4(float a[4]) { return a[0] + a[1] + a[2] + a[3]; }\n\ + void mainImage(out vec4 c, in vec2 p) { \ + float t[4] = iChannelTime; float u[4]; u = iChannelTime; \ + c = vec4(t[1] + u[2], sum4(iChannelTime), iChannelTime[3], iTime); }"; + let msl = check_msl_layout(user); + // The block member keeps the padded wrapper and is read per element. + assert!(msl.contains(".iChannelTimeUbo.inner[0].x"), "{msl}"); + // GLSL output (OpenGL) validates too. + let compiled = compile_shadertoy(user, None).unwrap(); + glsl_from_compiled(&compiled).unwrap(); + } + + fn check_msl_layout(user: &str) -> String { + use std::mem::offset_of as o; let compiled = compile_shadertoy(user, None).unwrap(); let msl = msl_from_compiled(&compiled).unwrap(); let (members, size) = msl_globals_layout(&msl); @@ -817,7 +855,7 @@ mod tests { ("iTimeDelta", o!(Uniforms, time_delta)), ("iFrameRate", o!(Uniforms, frame_rate)), ("iFrame", o!(Uniforms, frame)), - ("iChannelTime", o!(Uniforms, channel_time)), + ("iChannelTimeUbo", o!(Uniforms, channel_time)), ("iChannelResolution", o!(Uniforms, channel_resolution)), ("iMouse", o!(Uniforms, mouse)), ("iDate", o!(Uniforms, date)), @@ -856,7 +894,8 @@ mod tests { } assert_eq!(size, std::mem::size_of::()); // Reads of the padded array take the first lane. - assert!(msl.contains(".iChannelTime.inner["), "{msl}"); + assert!(msl.contains(".iChannelTimeUbo.inner["), "{msl}"); assert!(msl.contains("].x"), "{msl}"); + msl } } diff --git a/crates/ghostty-terminal/src/screen.rs b/crates/ghostty-terminal/src/screen.rs index 76518ea..c2310eb 100644 --- a/crates/ghostty-terminal/src/screen.rs +++ b/crates/ghostty-terminal/src/screen.rs @@ -368,6 +368,24 @@ pub enum SemanticContentSet { pub use crate::string_map::StringMap; +/// Failure of [`Screen::try_selection_string_map`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +pub enum SelectionStringError { + #[error("out of memory")] + OutOfMemory, +} + +/// Failure points of `selection_string` (Zig: `selectionString_tw`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum SelectionStringPoint { + CopyMap, +} +crate::tripwire_module!( + selection_string_tw, + SelectionStringPoint, + SelectionStringError +); + #[derive(Debug, Clone, Copy)] pub struct SelectionString { /// The selection to convert to a string. @@ -2572,10 +2590,38 @@ impl Screen { /// Same as `selection_string` but also returns a map from every byte of /// the string to the pin it came from (Zig: `SelectionString.map`). + /// + /// Panics if the map cannot be built, like any other Rust allocation + /// failure; use [`Screen::try_selection_string_map`] to handle it. pub fn selection_string_map(&self, opts: SelectionString) -> StringMap { + self.try_selection_string_map(opts) + .expect("selection string map allocation failed") + } + + /// Fallible [`Screen::selection_string_map`] (Zig `selectionString` with + /// `opts.map` set, which can return `error.OutOfMemory` when copying the + /// map). Nothing is leaked on failure: the string and pin list built so + /// far are dropped (Zig: `errdefer alloc.free(map_string)`). + pub fn try_selection_string_map( + &self, + opts: SelectionString, + ) -> Result { let mut pins = Vec::new(); let string = self.selection_string_impl(opts, Some(&mut pins)); - StringMap { string, map: pins } + + // Duplicate the string and take the pins for the map (Zig: `dupeZ` + // then `pins.toOwnedSlice`). + let mut map_string = String::new(); + map_string + .try_reserve_exact(string.len()) + .map_err(|_| SelectionStringError::OutOfMemory)?; + map_string.push_str(&string); + selection_string_tw::check(SelectionStringPoint::CopyMap)?; + pins.shrink_to_fit(); + Ok(StringMap { + string: map_string, + map: pins, + }) } fn selection_string_impl( diff --git a/crates/ghostty-terminal/src/screen/tests/p5.rs b/crates/ghostty-terminal/src/screen/tests/p5.rs index 9a1eece..ba6767f 100644 --- a/crates/ghostty-terminal/src/screen/tests/p5.rs +++ b/crates/ghostty-terminal/src/screen/tests/p5.rs @@ -685,9 +685,31 @@ fn screen_set_attribute_splits_page_on_out_of_space_at_max_styles() { assert!(page_was_split); } -// "selectionString map allocation failure cleanup" is skipped: it relies on -// Zig tripwire allocation-failure injection (selectionString_tw), which has -// no Rust counterpart. +#[test] +fn screen_selection_string_map_allocation_failure_cleanup() { + use crate::tripwire::ResetMode; + + // If building the StringMap fails, the already-built string must be + // dropped and the error returned (Zig: errdefer free(map_string)). + let mut s = init(10, 5, 0); + s.test_write_string("hello").unwrap(); + let sel = sel_active(&s, (0, 0), (4, 0), false); + + selection_string_tw::error_always( + SelectionStringPoint::CopyMap, + SelectionStringError::OutOfMemory, + ); + let result = s.try_selection_string_map(SelectionString { sel, trim: true }); + assert!(matches!(result, Err(SelectionStringError::OutOfMemory))); + selection_string_tw::end(ResetMode::Reset).unwrap(); + + // And it works again once the tripwire is reset. + let map = s + .try_selection_string_map(SelectionString { sel, trim: true }) + .unwrap(); + assert_eq!(map.string, "hello"); + assert_eq!(map.map.len(), 5); +} fn setup_prompt(cols: u16, click: Click) -> Screen { let mut s = init(cols, 5, 0); diff --git a/crates/ghostty-terminal/src/terminal/tests/p8.rs b/crates/ghostty-terminal/src/terminal/tests/p8.rs index 3d9d234..0ae7e3d 100644 --- a/crates/ghostty-terminal/src/terminal/tests/p8.rs +++ b/crates/ghostty-terminal/src/terminal/tests/p8.rs @@ -13,14 +13,14 @@ use crate::{ screen::SemanticClick, }; -fn sp(a: A) -> Cmd { +fn sp(a: A) -> Cmd<'static> { Cmd::init(a) } -fn sp_opts(a: A, o: &str) -> Cmd { +fn sp_opts(a: A, o: &str) -> Cmd<'_> { Cmd { action: a, - options_unvalidated: o.as_bytes().to_vec(), + options_unvalidated: o.as_bytes().into(), } } diff --git a/crates/ghostty-termio/src/exec.rs b/crates/ghostty-termio/src/exec.rs index fa5e9e2..d68e679 100644 --- a/crates/ghostty-termio/src/exec.rs +++ b/crates/ghostty-termio/src/exec.rs @@ -791,7 +791,10 @@ impl Subprocess { // If we have a resources dir then set our env var if let Some(dir) = &cfg.resources_dir { log::info!("found Ghostty resources dir: {dir}"); - env.insert("GHOSTTY_RESOURCES_DIR", dir.clone()); + env.insert( + "GHOSTTY_RESOURCES_DIR", + ghostty_os::osbytes::config_str_to_os(dir), + ); } // Set our TERM var. This is a bit complicated because we want to use @@ -810,15 +813,7 @@ impl Subprocess { // Rust-specific: the in-repo dev resources dir (`/resources`) // carries its compiled terminfo inside itself, so prefer that // when present. - let own = format!("{base}/terminfo"); - let terminfo = if std::path::Path::new(&format!("{own}/x/xterm-ghostty")).exists() { - own - } else { - let parent = std::path::Path::new(base) - .parent() - .map_or_else(String::new, |p| p.to_string_lossy().into_owned()); - format!("{parent}/terminfo") - }; + let terminfo = terminfo_dir(base); env.insert("TERMINFO", terminfo); } else { if cfg!(target_vendor = "apple") { @@ -1225,31 +1220,49 @@ fn add_ghostty_path(env: &mut EnvMap) { } } +/// The `TERMINFO` directory for a resources dir (reversible config-string +/// encoding in, raw OS string out). +fn terminfo_dir(base: &str) -> std::ffi::OsString { + let base = ghostty_os::osbytes::config_str_to_os(base); + let own = std::path::Path::new(&base).join("terminfo"); + if own.join("x/xterm-ghostty").exists() { + own.into_os_string() + } else { + let mut parent = std::path::Path::new(&base) + .parent() + .map_or_else(std::ffi::OsString::new, |p| p.as_os_str().to_owned()); + parent.push("/terminfo"); + parent + } +} + /// macOS: export additional data directories from our application bundle /// (`XDG_DATA_DIRS` and `MANPATH`). pub fn darwin_bundle_env(env: &mut EnvMap, resources_dir: &str) { let xdg_data_dir_key = "XDG_DATA_DIRS"; - let data_dir = format!("{resources_dir}/.."); + let mut data_dir = ghostty_os::osbytes::config_str_to_os(resources_dir); + data_dir.push("/.."); let current = env.get(xdg_data_dir_key).map_or_else( - || "/usr/local/share:/usr/share".to_owned(), - |v| v.to_string_lossy().into_owned(), + || std::ffi::OsString::from("/usr/local/share:/usr/share"), + std::ffi::OsStr::to_owned, ); env.insert( xdg_data_dir_key, - ghostty_os::append_env(¤t, &data_dir), + ghostty_os::env::append_env_os(¤t, &data_dir), ); let manpath_key = "MANPATH"; - let man_dir = format!("{resources_dir}/../man"); + let mut man_dir = ghostty_os::osbytes::config_str_to_os(resources_dir); + man_dir.push("/../man"); // Always append with colon in front, as it mean that if // `MANPATH` is empty, then it should be treated as an extra // path instead of overriding all paths set by OS. let current = env .get(manpath_key) - .map_or_else(String::new, |v| v.to_string_lossy().into_owned()); + .map_or_else(std::ffi::OsString::new, std::ffi::OsStr::to_owned); env.insert( manpath_key, - ghostty_os::append_env_always(¤t, &man_dir), + ghostty_os::env::append_env_always_os(¤t, &man_dir), ); } diff --git a/crates/ghostty-termio/src/shell_integration.rs b/crates/ghostty-termio/src/shell_integration.rs index 7c34a85..9d5d62c 100644 --- a/crates/ghostty-termio/src/shell_integration.rs +++ b/crates/ghostty-termio/src/shell_integration.rs @@ -62,6 +62,16 @@ pub struct ShellIntegration { pub command: Command, } +/// `/` as an OS string. `resource_dir` carries the +/// reversible config-string encoding (it may hold non-UTF-8 bytes), so it is +/// decoded before reaching the filesystem or the child environment. +fn resource_path(resource_dir: &str, rel: &str) -> std::ffi::OsString { + let mut p = ghostty_os::osbytes::config_str_to_os(resource_dir); + p.push("/"); + p.push(rel); + p +} + /// Set up the command execution environment for automatic /// integrated shell integration and return a ShellIntegration /// struct describing the integration. If integration fails @@ -239,13 +249,13 @@ fn setup_bash( } // Set our new ENV to point to our integration script. - let script_path = format!("{resource_dir}/shell-integration/bash/ghostty.bash"); + let script_path = resource_path(resource_dir, "shell-integration/bash/ghostty.bash"); match std::fs::File::open(&script_path) { Ok(_) => { env.insert("ENV", script_path); } Err(err) => { - log::warn!("unable to open {script_path}: {err}"); + log::warn!("unable to open {}: {err}", script_path.to_string_lossy()); env.remove("GHOSTTY_BASH_ENV"); return Ok(None); } @@ -278,9 +288,9 @@ fn setup_bash( /// from `XDG_DATA_DIRS` when integration is complete. fn setup_xdg_data_dirs(resource_dir: &str, env: &mut EnvMap) -> bool { // Get our path to the shell integration directory. - let integ_path = format!("{resource_dir}/shell-integration"); + let integ_path = resource_path(resource_dir, "shell-integration"); if let Err(err) = std::fs::read_dir(&integ_path) { - log::warn!("unable to open {integ_path}: {err}"); + log::warn!("unable to open {}: {err}", integ_path.to_string_lossy()); return false; } @@ -297,7 +307,7 @@ fn setup_xdg_data_dirs(resource_dir: &str, env: &mut EnvMap) -> bool { let current = env .get(key) .unwrap_or_else(|| std::ffi::OsStr::new("/usr/local/share:/usr/share")); - let new = prepend_env_os(current, std::ffi::OsStr::new(&integ_path)); + let new = prepend_env_os(current, &integ_path); env.insert(key, new); true @@ -374,10 +384,10 @@ fn setup_zsh(command: &Command, resource_dir: &str, env: &mut EnvMap) -> Option< } // Set our new ZDOTDIR to point to our shell resource directory. - let integ_path = format!("{resource_dir}/shell-integration/zsh"); + let integ_path = resource_path(resource_dir, "shell-integration/zsh"); if !Path::new(&integ_path).is_dir() { // Mirrors openDirAbsolute failing (missing or not a directory). - log::warn!("unable to open {integ_path}"); + log::warn!("unable to open {}", integ_path.to_string_lossy()); return None; } env.insert("ZDOTDIR", integ_path); diff --git a/crates/ghostty-vt-capi/build.rs b/crates/ghostty-vt-capi/build.rs index b7b0e5f..73982c5 100644 --- a/crates/ghostty-vt-capi/build.rs +++ b/crates/ghostty-vt-capi/build.rs @@ -1,14 +1,49 @@ +/// libghostty-vt version (Zig: `GhosttyLibVt` `version = .{ 0, 1, 0 }`). +/// `scripts/install-lib.sh` reads this line to name the installed files, so +/// keep the format. +const VT_VERSION: &str = "0.1.0"; + +/// The soname / install name of the shared library: `libghostty-vt.so.` +/// on ELF platforms (Zig sets `.version` on the shared library, which makes +/// the linker emit the versioned soname), `@rpath/libghostty-vt..dylib` +/// on Apple platforms. +fn lib_name(target_os: &str, version: &str) -> Option { + let major = version.split('.').next().unwrap_or(version); + match target_os { + "linux" | "android" | "freebsd" => Some(format!("libghostty-vt.so.{major}")), + "macos" | "ios" => Some(format!("@rpath/libghostty-vt.{major}.dylib")), + _ => None, + } +} + fn main() { - // Ship the shared library under Ghostty's name (`libghostty-vt.so`). let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); - match target_os.as_str() { - "linux" | "android" | "freebsd" => { - println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,libghostty-vt.so"); + match (lib_name(&target_os, VT_VERSION), target_os.as_str()) { + (Some(name), "linux" | "android" | "freebsd") => { + println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,{name}"); } - "macos" | "ios" => { - println!("cargo:rustc-cdylib-link-arg=-Wl,-install_name,@rpath/libghostty-vt.dylib"); + (Some(name), _) => { + println!("cargo:rustc-cdylib-link-arg=-Wl,-install_name,{name}"); } _ => {} } println!("cargo:rerun-if-changed=build.rs"); } + +#[cfg(test)] +mod tests { + use super::lib_name; + + #[test] + fn versioned_names() { + assert_eq!( + lib_name("linux", "0.1.0").as_deref(), + Some("libghostty-vt.so.0") + ); + assert_eq!( + lib_name("macos", "1.2.3").as_deref(), + Some("@rpath/libghostty-vt.1.dylib") + ); + assert_eq!(lib_name("windows", "0.1.0"), None); + } +} diff --git a/crates/ghostty-vt-capi/src/alloc.rs b/crates/ghostty-vt-capi/src/alloc.rs index 075239a..f2ae0dd 100644 --- a/crates/ghostty-vt-capi/src/alloc.rs +++ b/crates/ghostty-vt-capi/src/alloc.rs @@ -10,9 +10,12 @@ //! `GHOSTTY_OUT_OF_MEMORY` where Zig can report it (object creation) and //! invalidates the OSC being parsed where Zig's writer fails. //! -//! Transient values built while a command is parsed (the payload copies the -//! per-command parsers make before `ghostty_osc_end` returns) use the global -//! allocator; nothing of them outlives that call. +//! Commands produced by `ghostty_osc_end` borrow their byte payloads from the +//! parser's storage (as Zig's do), so ending an OSC does not allocate for the +//! payload at all; the parser itself holds the storage inline (no `Box`). The +//! only values still built on the global allocator are the small request +//! lists of the color OSCs (4, 5, 10-19, 21, 104-119), which are not exposed +//! through the C API and are dropped before `ghostty_osc_end` returns. #![allow(unsafe_code)] use std::{alloc::Layout, ffi::c_void, ptr::NonNull}; @@ -276,8 +279,4 @@ impl ByteStorage for CByteStorage { self.block = b; } } - - fn clone_box(&self) -> Box { - Box::new(self.bytes().to_vec()) - } } diff --git a/crates/ghostty-vt-capi/src/osc.rs b/crates/ghostty-vt-capi/src/osc.rs index fdd7ddd..600c5c6 100644 --- a/crates/ghostty-vt-capi/src/osc.rs +++ b/crates/ghostty-vt-capi/src/osc.rs @@ -16,7 +16,7 @@ use crate::{ /// C: `GhosttyOscParser` (opaque). pub struct OscParser { alloc: Alloc, - parser: Parser, + parser: Parser, /// Backing store for the last command returned by `ghostty_osc_end`. command: OscCommand, } @@ -67,12 +67,12 @@ pub unsafe extern "C" fn ghostty_osc_new( // The parser's payload buffer (Zig's inline fixed buffer / allocating // writer) lives in memory from the same allocator; if either allocation // fails the caller gets OUT_OF_MEMORY. - let Some(storage) = CByteStorage::new(alloc, Parser::MAX_BUF) else { + let Some(storage) = CByteStorage::new(alloc, ghostty_vt::osc::MAX_BUF) else { return GhosttyResult::OutOfMemory; }; let Some(p) = alloc.create(OscParser { alloc, - parser: Parser::with_storage(Box::new(storage)), + parser: Parser::with_storage(storage), command: OscCommand { key: 0, title_z: None, @@ -131,8 +131,11 @@ pub unsafe extern "C" fn ghostty_osc_end( let Some(p) = (unsafe { parser.as_mut() }) else { return std::ptr::null_mut(); }; - let cmd = p.parser.end(Some(terminator)).unwrap_or(Command::Invalid); + // The command borrows its payload from the parser's own (caller + // allocated) storage, so nothing is copied or allocated here except the + // NUL-terminated title handed to C. p.command.title_z = None; + let cmd = p.parser.end(Some(terminator)).unwrap_or(Command::Invalid); p.command.key = cmd.key() as c_int; if let Command::ChangeWindowTitle(t) = &cmd { match CBlock::from_bytes_z(p.alloc, t) { @@ -192,10 +195,157 @@ pub unsafe extern "C" fn ghostty_osc_command_data( #[cfg(test)] mod tests { - use std::ffi::CStr; + use std::{ + alloc::{GlobalAlloc, Layout, System}, + cell::Cell, + ffi::CStr, + }; use super::*; + /// Counts Rust global-allocator calls made by the current thread, so the + /// tests can prove that OSC parsing and `ghostty_osc_end` use only the + /// caller's allocator. + struct CountingGlobal; + thread_local! { + static GLOBAL_ALLOCS: Cell = const { Cell::new(0) }; + } + // SAFETY: forwards to the system allocator. + unsafe impl GlobalAlloc for CountingGlobal { + unsafe fn alloc(&self, l: Layout) -> *mut u8 { + let _ = GLOBAL_ALLOCS.try_with(|c| c.set(c.get() + 1)); + // SAFETY: same contract. + unsafe { System.alloc(l) } + } + unsafe fn dealloc(&self, p: *mut u8, l: Layout) { + // SAFETY: same contract. + unsafe { System.dealloc(p, l) } + } + unsafe fn realloc(&self, p: *mut u8, l: Layout, n: usize) -> *mut u8 { + let _ = GLOBAL_ALLOCS.try_with(|c| c.set(c.get() + 1)); + // SAFETY: same contract. + unsafe { System.realloc(p, l, n) } + } + } + #[global_allocator] + static GLOBAL: CountingGlobal = CountingGlobal; + + fn global_allocs() -> usize { + GLOBAL_ALLOCS.with(Cell::get) + } + + use std::{ + ffi::c_void, + sync::atomic::{AtomicUsize, Ordering}, + }; + + use crate::alloc::{Allocator, AllocatorVtable}; + + /// A caller allocator backed by the system allocator that counts calls. + unsafe extern "C" fn c_alloc( + ctx: *mut c_void, + len: usize, + align_log2: u8, + _: usize, + ) -> *mut c_void { + // SAFETY: `ctx` is the `AtomicUsize` owned by the test. + unsafe { &*ctx.cast::() }.fetch_add(1, Ordering::SeqCst); + let l = Layout::from_size_align(len, 1 << align_log2).unwrap(); + // SAFETY: non-zero size (the crate never calls with zero). + unsafe { System.alloc(l) }.cast() + } + unsafe extern "C" fn c_resize( + _: *mut c_void, + _: *mut c_void, + _: usize, + _: u8, + _: usize, + _: usize, + ) -> bool { + false + } + unsafe extern "C" fn c_remap( + _: *mut c_void, + _: *mut c_void, + _: usize, + _: u8, + _: usize, + _: usize, + ) -> *mut c_void { + std::ptr::null_mut() + } + unsafe extern "C" fn c_free( + _: *mut c_void, + m: *mut c_void, + len: usize, + align_log2: u8, + _: usize, + ) { + let l = Layout::from_size_align(len, 1 << align_log2).unwrap(); + // SAFETY: allocated by `c_alloc` with the same layout. + unsafe { System.dealloc(m.cast(), l) }; + } + static VT: AllocatorVtable = AllocatorVtable { + alloc: c_alloc, + resize: c_resize, + remap: c_remap, + free: c_free, + }; + + #[test] + fn end_makes_no_global_allocations() { + let caller_allocs = AtomicUsize::new(0); + let a = Allocator { + ctx: (&raw const caller_allocs).cast_mut().cast(), + vtable: &raw const VT, + }; + let mut p = std::ptr::null_mut(); + // SAFETY: valid allocator and out pointer. + assert_eq!( + unsafe { ghostty_osc_new(&raw const a, &mut p) }, + GhosttyResult::Success + ); + // SAFETY: `p` is live for the whole block. + unsafe { + for (osc, title) in [ + ("7;file:///tmp/x", false), + ("8;id=a;https://example.com", false), + ("8;;", false), + ("52;c;aGVsbG8=", false), + ("133;A", false), + ("777;notify;title;body", false), + ("9;4;1;50", false), + ("9;2;message", false), + ("22;pointer", false), + ("66;s=2;hi", false), + ("3008;start=abc;type=shell", false), + ("5522;type=read;data", false), + ("0;a title", true), + ] { + ghostty_osc_reset(p); + for b in osc.bytes() { + ghostty_osc_next(p, b); + } + let global_before = global_allocs(); + let caller_before = caller_allocs.load(Ordering::SeqCst); + let cmd = ghostty_osc_end(p, 0x07); + assert_ne!(ghostty_osc_command_type(cmd), 0, "{osc}"); + assert_eq!( + global_allocs(), + global_before, + "global allocation ending {osc}" + ); + // Only the NUL-terminated title handed to C is allocated. + assert_eq!( + caller_allocs.load(Ordering::SeqCst) - caller_before, + usize::from(title), + "caller allocations ending {osc}" + ); + } + ghostty_osc_free(p); + } + } + fn new() -> *mut OscParser { let mut p = std::ptr::null_mut(); // SAFETY: valid out pointer, default allocator. diff --git a/crates/ghostty-vt-capi/tests/c/alloc_test.c b/crates/ghostty-vt-capi/tests/c/alloc_test.c index a99c321..7c4686b 100644 --- a/crates/ghostty-vt-capi/tests/c/alloc_test.c +++ b/crates/ghostty-vt-capi/tests/c/alloc_test.c @@ -159,6 +159,49 @@ int main(void) { assert(c.live == 0); } + // 6. Ending an OSC copies no payload: with the allocator frozen after + // creation (any allocation would fail), commands whose payloads are + // borrowed from the parser's storage still parse, and every command + // makes exactly the allocations the caller can see (none, except the + // NUL-terminated title copy). + { + static const struct { const char *osc; int type; } cases[] = { + {"7;file:///tmp/x", GHOSTTY_OSC_COMMAND_REPORT_PWD}, + {"8;id=a;https://example.com", GHOSTTY_OSC_COMMAND_HYPERLINK_START}, + {"8;;", GHOSTTY_OSC_COMMAND_HYPERLINK_END}, + {"52;c;aGVsbG8=", GHOSTTY_OSC_COMMAND_CLIPBOARD_CONTENTS}, + {"133;A", GHOSTTY_OSC_COMMAND_SEMANTIC_PROMPT}, + {"777;notify;title;body", GHOSTTY_OSC_COMMAND_SHOW_DESKTOP_NOTIFICATION}, + {"9;4;1;50", GHOSTTY_OSC_COMMAND_CONEMU_PROGRESS_REPORT}, + {"22;pointer", GHOSTTY_OSC_COMMAND_MOUSE_SHAPE}, + {"66;s=2;hi", GHOSTTY_OSC_COMMAND_KITTY_TEXT_SIZING}, + }; + Counter c = counter(0, SIZE_MAX); + GhosttyAllocator a = {.ctx = &c, .vtable = &VT}; + GhosttyOscParser p; + assert(ghostty_osc_new(&a, &p) == GHOSTTY_SUCCESS); + c.fail_after = c.allocs; // freeze: no allocation may succeed + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + size_t allocs = c.allocs; + ghostty_osc_reset(p); + feed(p, cases[i].osc, strlen(cases[i].osc)); + GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07); + assert(ghostty_osc_command_type(cmd) == cases[i].type); + assert(c.allocs == allocs); + } + // The title is the one payload handed to C: exactly one allocation. + c.fail_after = SIZE_MAX; + size_t allocs = c.allocs; + ghostty_osc_reset(p); + feed(p, "2;title", 7); + GhosttyOscCommand cmd = ghostty_osc_end(p, 0x07); + assert(ghostty_osc_command_type(cmd) == GHOSTTY_OSC_COMMAND_CHANGE_WINDOW_TITLE); + assert(c.allocs == allocs + 1); + ghostty_osc_free(p); + assert(c.live == 0); + assert(c.allocs == c.frees); + } + puts("ok"); return 0; } diff --git a/crates/ghostty-vt-capi/tests/common/mod.rs b/crates/ghostty-vt-capi/tests/common/mod.rs index 64f7315..7808728 100644 --- a/crates/ghostty-vt-capi/tests/common/mod.rs +++ b/crates/ghostty-vt-capi/tests/common/mod.rs @@ -29,6 +29,9 @@ fn cargo() -> Command { Command::new(std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into())) } +/// Major version of libghostty-vt (the soname suffix set by build.rs). +pub const VT_MAJOR: u32 = 0; + static INSTALLED: OnceLock = OnceLock::new(); /// Build the cdylib/staticlib for the host and install them as @@ -61,6 +64,19 @@ fn install_native_lib_once() -> PathBuf { std::fs::rename(&tmp, &dst).expect("install library"); } } + // The cdylib's soname is versioned (`libghostty-vt.so.`, see + // build.rs), so executables linked against it need that name at run time + // (otherwise the loader would fall back to an unrelated system copy). + let so = dir.join("libghostty-vt.so"); + if so.exists() { + let versioned = dir.join(format!("libghostty-vt.so.{VT_MAJOR}")); + let tmp = dir.join(format!( + "libghostty-vt.so.{VT_MAJOR}.{}.tmp", + std::process::id() + )); + std::fs::copy(&so, &tmp).expect("install versioned library"); + std::fs::rename(&tmp, &versioned).expect("install versioned library"); + } dir } diff --git a/crates/ghostty-vt-capi/tests/soname.rs b/crates/ghostty-vt-capi/tests/soname.rs new file mode 100644 index 0000000..750e620 --- /dev/null +++ b/crates/ghostty-vt-capi/tests/soname.rs @@ -0,0 +1,29 @@ +//! The shared library must carry the versioned soname `libghostty-vt.so.` +//! straight from the build (Zig sets `.version` on the shared library), not +//! via a post-install `patchelf` step. +#![cfg(target_os = "linux")] +#![allow(clippy::unwrap_used)] + +mod common; + +use std::process::Command; + +#[test] +fn shared_library_has_versioned_soname() { + let lib = common::install_native_lib(); + let so = lib.join("libghostty-vt.so"); + let Ok(out) = Command::new("readelf").arg("-d").arg(&so).output() else { + eprintln!("readelf not available; skipping"); + return; + }; + let text = String::from_utf8_lossy(&out.stdout); + let want = format!("[libghostty-vt.so.{}]", common::VT_MAJOR); + assert!( + text.lines() + .any(|l| l.contains("SONAME") && l.contains(&want)), + "expected SONAME {want}:\n{text}" + ); + // build.rs is the single source of the version (install-lib.sh reads it). + let build_rs = std::fs::read_to_string(common::manifest_dir().join("build.rs")).unwrap(); + assert!(build_rs.contains(&format!("const VT_VERSION: &str = \"{}.", common::VT_MAJOR))); +} diff --git a/crates/ghostty-vt/src/osc/mod.rs b/crates/ghostty-vt/src/osc/mod.rs index a44e3ca..5c026a9 100644 --- a/crates/ghostty-vt/src/osc/mod.rs +++ b/crates/ghostty-vt/src/osc/mod.rs @@ -23,7 +23,7 @@ pub use parsers::{color, semantic_prompt}; use crate::kitty::color as kitty_color; #[derive(Debug, Clone, PartialEq)] -pub enum Command { +pub enum Command<'a> { /// This generally shouldn't ever be set except as an initial zero value. /// Ignore it. Invalid, @@ -36,23 +36,23 @@ pub enum Command { /// If title mode 2 is set or the terminal is setup for unconditional /// utf-8 titles text is interpreted as utf-8. Else text is interpreted /// as latin1. - ChangeWindowTitle(Vec), + ChangeWindowTitle(Bytes<'a>), /// Set the icon of the terminal window. The name of the icon is not /// well defined, so this is currently ignored by Ghostty at the time /// of writing this. We just parse it so that we don't get parse errors /// in the log. - ChangeWindowIcon(Vec), + ChangeWindowIcon(Bytes<'a>), /// Semantic prompt command: https://gitlab.freedesktop.org/Per_Bothner/specifications/blob/master/proposals/semantic-prompts.md - SemanticPrompt(SemanticPrompt), + SemanticPrompt(SemanticPrompt<'a>), /// Set or get clipboard contents. If data is null, then the current /// clipboard contents are sent to the pty. If data is set, this /// contents is set on the clipboard. ClipboardContents { kind: u8, - data: Vec, + data: Bytes<'a>, }, /// OSC 7. Reports the current working directory of the shell. This is @@ -62,7 +62,7 @@ pub enum Command { ReportPwd { /// The reported pwd value. This is not checked for validity. It should /// be a file URL but it is up to the caller to utilize this value. - value: Vec, + value: Bytes<'a>, }, /// OSC 22. Set the mouse shape. There doesn't seem to be a standard @@ -70,7 +70,7 @@ pub enum Command { /// are moving towards using the W3C CSS cursor names. For OSC parsing, /// we just parse whatever string is given. MouseShape { - value: Vec, + value: Bytes<'a>, }, /// OSC color operations to set, reset, or report color settings. Some OSCs @@ -92,14 +92,14 @@ pub enum Command { /// Show a desktop notification (OSC 9 or OSC 777) ShowDesktopNotification { - title: Vec, - body: Vec, + title: Bytes<'a>, + body: Bytes<'a>, }, /// Start a hyperlink (OSC 8) HyperlinkStart { - id: Option>, - uri: Vec, + id: Option>, + uri: Bytes<'a>, }, /// End a hyperlink (OSC 8) @@ -111,10 +111,10 @@ pub enum Command { }, /// ConEmu show GUI message box (OSC 9;2) - ConemuShowMessageBox(Vec), + ConemuShowMessageBox(Bytes<'a>), /// ConEmu change tab title (OSC 9;3) - ConemuChangeTabTitle(ConemuChangeTabTitle), + ConemuChangeTabTitle(ConemuChangeTabTitle<'a>), /// ConEmu progress report (OSC 9;4) ConemuProgressReport(ProgressReport), @@ -123,13 +123,13 @@ pub enum Command { ConemuWaitInput, /// ConEmu GUI macro (OSC 9;6) - ConemuGuimacro(Vec), + ConemuGuimacro(Bytes<'a>), /// ConEmu run process (OSC 9;7) - ConemuRunProcess(Vec), + ConemuRunProcess(Bytes<'a>), /// ConEmu output environment variable (OSC 9;8) - ConemuOutputEnvironmentVariable(Vec), + ConemuOutputEnvironmentVariable(Bytes<'a>), /// ConEmu XTerm keyboard and output emulation (OSC 9;10) /// https://conemu.github.io/en/TerminalModes.html @@ -145,26 +145,32 @@ pub enum Command { }, /// ConEmu comment (OSC 9;11) - ConemuComment(Vec), + ConemuComment(Bytes<'a>), /// Kitty text sizing protocol (OSC 66) - KittyTextSizing(parsers::kitty_text_sizing::OSC), + KittyTextSizing(parsers::kitty_text_sizing::OSC<'a>), - KittyClipboardProtocol(KittyClipboardProtocol), + KittyClipboardProtocol(KittyClipboardProtocol<'a>), /// OSC 3008. Hierarchical context signalling (UAPI spec). /// https://uapi-group.org/specifications/specs/osc_context/ - ContextSignal(parsers::context_signal::Command), + ContextSignal(parsers::context_signal::Command<'a>), } -pub type SemanticPrompt = parsers::semantic_prompt::Command; -pub type KittyClipboardProtocol = parsers::kitty_clipboard_protocol::OSC; +/// Payload bytes of a [`Command`]: borrowed from the parser's payload storage +/// (Zig: slices into the parser's buffer), so producing a command does not +/// allocate. Use [`Command::into_owned`] to keep one past the next parser +/// operation. +pub type Bytes<'a> = std::borrow::Cow<'a, [u8]>; + +pub type SemanticPrompt<'a> = parsers::semantic_prompt::Command<'a>; +pub type KittyClipboardProtocol<'a> = parsers::kitty_clipboard_protocol::OSC<'a>; /// Payload of `Command::ConemuChangeTabTitle`. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum ConemuChangeTabTitle { +pub enum ConemuChangeTabTitle<'a> { Reset, - Value(Vec), + Value(Bytes<'a>), } /// The discriminant of a [`Command`] (Zig `Command.Key`). Order matches Zig. @@ -197,7 +203,67 @@ pub enum Key { ContextSignal, } -impl Command { +fn own(b: Bytes<'_>) -> Bytes<'static> { + Bytes::Owned(b.into_owned()) +} + +impl Command<'_> { + /// Detach the command from the parser's storage (copies the payloads), + /// so it can outlive the next parser operation. + pub fn into_owned(self) -> Command<'static> { + match self { + Command::Invalid => Command::Invalid, + Command::ChangeWindowTitle(v) => Command::ChangeWindowTitle(own(v)), + Command::ChangeWindowIcon(v) => Command::ChangeWindowIcon(own(v)), + Command::SemanticPrompt(v) => Command::SemanticPrompt(v.into_owned()), + Command::ClipboardContents { kind, data } => Command::ClipboardContents { + kind, + data: own(data), + }, + Command::ReportPwd { value } => Command::ReportPwd { value: own(value) }, + Command::MouseShape { value } => Command::MouseShape { value: own(value) }, + Command::ColorOperation { + op, + requests, + terminator, + } => Command::ColorOperation { + op, + requests, + terminator, + }, + Command::KittyColorProtocol(v) => Command::KittyColorProtocol(v), + Command::ShowDesktopNotification { title, body } => Command::ShowDesktopNotification { + title: own(title), + body: own(body), + }, + Command::HyperlinkStart { id, uri } => Command::HyperlinkStart { + id: id.map(own), + uri: own(uri), + }, + Command::HyperlinkEnd => Command::HyperlinkEnd, + Command::ConemuSleep { duration_ms } => Command::ConemuSleep { duration_ms }, + Command::ConemuShowMessageBox(v) => Command::ConemuShowMessageBox(own(v)), + Command::ConemuChangeTabTitle(v) => Command::ConemuChangeTabTitle(match v { + ConemuChangeTabTitle::Reset => ConemuChangeTabTitle::Reset, + ConemuChangeTabTitle::Value(b) => ConemuChangeTabTitle::Value(own(b)), + }), + Command::ConemuProgressReport(v) => Command::ConemuProgressReport(v), + Command::ConemuWaitInput => Command::ConemuWaitInput, + Command::ConemuGuimacro(v) => Command::ConemuGuimacro(own(v)), + Command::ConemuRunProcess(v) => Command::ConemuRunProcess(own(v)), + Command::ConemuOutputEnvironmentVariable(v) => { + Command::ConemuOutputEnvironmentVariable(own(v)) + } + Command::ConemuXtermEmulation { keyboard, output } => { + Command::ConemuXtermEmulation { keyboard, output } + } + Command::ConemuComment(v) => Command::ConemuComment(own(v)), + Command::KittyTextSizing(v) => Command::KittyTextSizing(v.into_owned()), + Command::KittyClipboardProtocol(v) => Command::KittyClipboardProtocol(v.into_owned()), + Command::ContextSignal(v) => Command::ContextSignal(v.into_owned()), + } + } + pub fn key(&self) -> Key { match self { Command::Invalid => Key::Invalid, @@ -374,8 +440,14 @@ pub trait ByteStorage: std::fmt::Debug + Send { /// Drop the contents and release any memory acquired by growth (Zig /// `reset` deinits the allocating writer). fn reset(&mut self); - /// For `Parser: Clone`. - fn clone_box(&self) -> Box; + /// Number of accumulated bytes. + fn len(&self) -> usize { + self.bytes().len() + } + /// Whether nothing has been accumulated. + fn is_empty(&self) -> bool { + self.bytes().is_empty() + } } impl ByteStorage for Vec { @@ -393,74 +465,16 @@ impl ByteStorage for Vec { fn reset(&mut self) { self.clear(); } - fn clone_box(&self) -> Box { - Box::new(self.clone()) - } } -impl Clone for Box { - fn clone(&self) -> Self { - self.clone_box() - } -} +/// Maximum size of a "normal" OSC. +pub const MAX_BUF: usize = 2048; -/// Payload buffer: a plain `Vec` (fast path) or custom allocator-backed -/// storage. +/// The OSC parser. `S` is the payload storage: a `Vec` on the global +/// allocator by default, or caller-allocated storage (C API), in which case +/// the parser holds it inline (no extra allocation). #[derive(Debug, Clone)] -enum Buffer { - Vec(Vec), - Custom(Box), -} - -impl Buffer { - #[inline] - fn bytes(&self) -> &[u8] { - match self { - Buffer::Vec(v) => v, - Buffer::Custom(c) => c.bytes(), - } - } - - #[inline] - fn len(&self) -> usize { - self.bytes().len() - } - - #[inline] - fn try_extend(&mut self, b: &[u8]) -> Result<(), StorageFull> { - match self { - Buffer::Vec(v) => { - v.extend_from_slice(b); - Ok(()) - } - Buffer::Custom(c) => c.try_extend(b), - } - } - - fn try_reserve(&mut self, n: usize) -> Result<(), StorageFull> { - match self { - Buffer::Vec(v) => { - v.reserve(n); - Ok(()) - } - Buffer::Custom(c) => c.try_reserve(n), - } - } - - fn reset(&mut self) { - match self { - Buffer::Vec(v) => v.clear(), - Buffer::Custom(c) => c.reset(), - } - } - - fn to_vec(&self) -> Vec { - self.bytes().to_vec() - } -} - -#[derive(Debug, Clone)] -pub struct Parser { +pub struct Parser> { /// Whether an allocator is available, i.e. whether data longer than /// MAX_BUF may be accepted for some commands (e.g. OSC 52), and whether /// commands that require an allocator are processed. (Zig: @@ -471,7 +485,7 @@ pub struct Parser { pub state: State, /// Buffer for temporary storage of OSC data - buffer: Buffer, + buffer: S, writer: Writer, } @@ -482,25 +496,27 @@ impl Default for Parser { } impl Parser { - /// Maximum size of a "normal" OSC. - pub const MAX_BUF: usize = 2048; - pub fn init(alloc: bool) -> Parser { Parser { alloc, state: State::Start, - buffer: Buffer::Vec(Vec::new()), + buffer: Vec::new(), writer: Writer::None, } } +} + +impl Parser { + /// Maximum size of a "normal" OSC. + pub const MAX_BUF: usize = MAX_BUF; /// Like [`Parser::init`] with an allocator-backed payload buffer /// (`alloc` is true: an allocator is available). - pub fn with_storage(storage: Box) -> Parser { + pub fn with_storage(storage: S) -> Parser { Parser { alloc: true, state: State::Start, - buffer: Buffer::Custom(storage), + buffer: storage, writer: Writer::None, } } @@ -549,11 +565,11 @@ impl Parser { } /// Helper for the per-command parsers. Mirrors the Zig prologue - /// `writer orelse invalid; writer.writeByte(0) catch invalid; - /// writer.buffered()`: returns a copy of the accumulated data (without - /// the NUL terminator) or `None` (and marks the parser invalid) if no - /// data was being accumulated or there was no room for the terminator. - pub fn data(&mut self) -> Option> { + /// `writer orelse invalid; writer.writeByte(0) catch invalid`: returns + /// `None` (and marks the parser invalid) if no data was being accumulated + /// or there was no room for the NUL terminator. On success the payload + /// is available, borrowed and without copying, from [`Parser::payload`]. + pub fn data(&mut self) -> Option<()> { if self.writer == Writer::None { self.state = State::Invalid; return None; @@ -563,29 +579,33 @@ impl Parser { self.state = State::Invalid; return None; } - Some(self.buffer.to_vec()) + Some(()) } - /// Zig `writer.writeByte(0)` capacity check: when the fixed buffer is - /// full there is no room for the NUL terminator, so the parser becomes - /// invalid and this returns false. - pub fn reserve_nul(&mut self) -> bool { - if self.writer == Writer::Fixed && self.buffer.len() >= Self::MAX_BUF { - self.state = State::Invalid; - return false; - } - true + /// Whether `writer.writeByte(0)` would succeed: when the fixed buffer is + /// full there is no room for the NUL terminator. Pure so that it can be + /// asked while the payload is borrowed; the caller returns `None`, which + /// invalidates the OSC. + pub fn reserve_nul(&self) -> bool { + !(self.writer == Writer::Fixed && self.buffer.len() >= Self::MAX_BUF) } /// Like [`Parser::data`] but without reserving room for the NUL /// terminator (for parsers that, like Zig's color parsers, read /// `writer.buffered()` directly). - pub fn buffered(&mut self) -> Option> { + pub fn buffered(&mut self) -> Option<()> { if self.writer == Writer::None { self.state = State::Invalid; return None; } - Some(self.buffer.to_vec()) + Some(()) + } + + /// The accumulated OSC payload (Zig `writer.buffered()`), borrowed from + /// the parser's storage. Commands built from it borrow it too, which is + /// why producing a command never allocates. + pub fn payload(&self) -> &[u8] { + self.buffer.bytes() } /// Whether any payload writer has been set up (Zig `parser.writer != null`). @@ -871,7 +891,7 @@ impl Parser { /// is None, then no valid command was found. The optional terminator_ch /// is the final character in the OSC sequence. This is used to determine /// the response terminator. - pub fn end(&mut self, terminator_ch: Option) -> Option { + pub fn end(&mut self, terminator_ch: Option) -> Option> { #[allow(clippy::enum_glob_use)] use State::*; match self.state { @@ -953,17 +973,14 @@ mod next_slice_tests { fn reset(&mut self) { self.data.clear(); } - fn clone_box(&self) -> Box { - Box::new(self.clone()) - } } #[test] fn storage_failure_invalidates_osc() { - let mut p = Parser::with_storage(Box::new(Limited { + let mut p = Parser::with_storage(Limited { data: Vec::new(), cap: 8, - })); + }); for b in b"0;abcdefghijkl" { p.next(*b); } @@ -976,7 +993,7 @@ mod next_slice_tests { p.next(*b); } let cmd = p.end(Some(0x07)).expect("command"); - assert!(matches!(cmd, Command::ChangeWindowTitle(t) if t == b"abc")); + assert!(matches!(cmd, Command::ChangeWindowTitle(t) if &t[..] == b"abc")); } #[test] diff --git a/crates/ghostty-vt/src/osc/parsers/change_window_icon.rs b/crates/ghostty-vt/src/osc/parsers/change_window_icon.rs index 31aa64d..5ec739b 100644 --- a/crates/ghostty-vt/src/osc/parsers/change_window_icon.rs +++ b/crates/ghostty-vt/src/osc/parsers/change_window_icon.rs @@ -1,10 +1,13 @@ //! OSC 1 (ZIG/src/terminal/osc/parsers/change_window_icon.zig). -use crate::osc::{Command, Parser}; +use crate::osc::{ByteStorage, Command, Parser}; /// Parse OSC 1 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.data()?; - Some(Command::ChangeWindowIcon(data)) +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.data()?; + Some(Command::ChangeWindowIcon(parser.payload().into())) } #[cfg(test)] @@ -19,6 +22,6 @@ mod tests { p.next(*c); } let cmd = p.end(None).unwrap(); - assert_eq!(cmd, Command::ChangeWindowIcon(b"ab".to_vec())); + assert_eq!(cmd, Command::ChangeWindowIcon(b"ab".to_vec().into())); } } diff --git a/crates/ghostty-vt/src/osc/parsers/change_window_title.rs b/crates/ghostty-vt/src/osc/parsers/change_window_title.rs index a37f544..35fb2e3 100644 --- a/crates/ghostty-vt/src/osc/parsers/change_window_title.rs +++ b/crates/ghostty-vt/src/osc/parsers/change_window_title.rs @@ -1,10 +1,13 @@ //! OSC 0 and OSC 2 (ZIG/src/terminal/osc/parsers/change_window_title.zig). -use crate::osc::{Command, Parser}; +use crate::osc::{ByteStorage, Command, Parser}; /// Parse OSC 0 and OSC 2 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.data()?; - Some(Command::ChangeWindowTitle(data)) +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.data()?; + Some(Command::ChangeWindowTitle(parser.payload().into())) } #[cfg(test)] @@ -24,7 +27,7 @@ mod tests { feed(&mut p, b"0;ab"); assert_eq!( p.end(None).unwrap(), - Command::ChangeWindowTitle(b"ab".to_vec()) + Command::ChangeWindowTitle(b"ab".to_vec().into()) ); } @@ -32,7 +35,7 @@ mod tests { fn osc_0_longer_than_buffer() { let mut p = Parser::init(false); let mut input = b"0;".to_vec(); - input.extend(std::iter::repeat_n(b'a', Parser::MAX_BUF + 2)); + input.extend(std::iter::repeat_n(b'a', crate::osc::MAX_BUF + 2)); feed(&mut p, &input); assert!(p.end(None).is_none()); } @@ -40,18 +43,21 @@ mod tests { #[test] fn osc_0_one_shorter_than_buffer_length() { let mut p = Parser::init(false); - let title = vec![b'a'; Parser::MAX_BUF - 1]; + let title = vec![b'a'; crate::osc::MAX_BUF - 1]; let mut input = b"0;".to_vec(); input.extend(&title); feed(&mut p, &input); - assert_eq!(p.end(None).unwrap(), Command::ChangeWindowTitle(title)); + assert_eq!( + p.end(None).unwrap(), + Command::ChangeWindowTitle(title.into()) + ); } #[test] fn osc_0_exactly_at_buffer_length() { let mut p = Parser::init(false); let mut input = b"0;".to_vec(); - input.extend(vec![b'a'; Parser::MAX_BUF]); + input.extend(vec![b'a'; crate::osc::MAX_BUF]); feed(&mut p, &input); // This should be None because we always reserve space for a null terminator. assert!(p.end(None).is_none()); @@ -63,7 +69,7 @@ mod tests { feed(&mut p, b"2;ab"); assert_eq!( p.end(None).unwrap(), - Command::ChangeWindowTitle(b"ab".to_vec()) + Command::ChangeWindowTitle(b"ab".to_vec().into()) ); } @@ -79,7 +85,7 @@ mod tests { feed(&mut p, &[0xE2, 0x80, 0x90]); assert_eq!( p.end(None).unwrap(), - Command::ChangeWindowTitle("— ‐".as_bytes().to_vec()) + Command::ChangeWindowTitle("— ‐".as_bytes().to_vec().into()) ); } @@ -87,6 +93,9 @@ mod tests { fn osc_2_change_window_title_empty() { let mut p = Parser::init(false); feed(&mut p, b"2;"); - assert_eq!(p.end(None).unwrap(), Command::ChangeWindowTitle(Vec::new())); + assert_eq!( + p.end(None).unwrap(), + Command::ChangeWindowTitle(Vec::new().into()) + ); } } diff --git a/crates/ghostty-vt/src/osc/parsers/clipboard_operation.rs b/crates/ghostty-vt/src/osc/parsers/clipboard_operation.rs index d863d83..3939300 100644 --- a/crates/ghostty-vt/src/osc/parsers/clipboard_operation.rs +++ b/crates/ghostty-vt/src/osc/parsers/clipboard_operation.rs @@ -1,35 +1,35 @@ //! OSC 52 (ZIG/src/terminal/osc/parsers/clipboard_operation.zig). -use crate::osc::{Command, Parser, State}; +use crate::osc::{ByteStorage, Command, Parser, State}; /// Parse OSC 52 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S52); - let data = parser.data()?; - // Zig's `data` includes the NUL terminator, so `data.len == 1` there - // means the payload is empty. - if data.is_empty() { - parser.state = State::Invalid; - return None; - } - if data[0] == b';' { - Some(Command::ClipboardContents { - kind: b'c', - data: data[1..].to_vec(), - }) - } else { - if data.len() < 2 { + parser.data()?; + let (kind, skip) = { + let data = parser.payload(); + // Zig's `data` includes the NUL terminator, so `data.len == 1` there + // means the payload is empty. + if data.is_empty() { parser.state = State::Invalid; return None; } - if data[1] != b';' { - parser.state = State::Invalid; - return None; + if data[0] == b';' { + (b'c', 1) + } else { + if data.len() < 2 || data[1] != b';' { + parser.state = State::Invalid; + return None; + } + (data[0], 2) } - Some(Command::ClipboardContents { - kind: data[0], - data: data[2..].to_vec(), - }) - } + }; + Some(Command::ClipboardContents { + kind, + data: parser.payload()[skip..].into(), + }) } #[cfg(test)] @@ -37,12 +37,12 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; - fn run(alloc: bool, input: &[u8]) -> Option { + fn run(alloc: bool, input: &[u8]) -> Option> { let mut p = Parser::init(alloc); for c in input { p.next(*c); } - p.end(None) + p.end(None).map(Command::into_owned) } #[test] @@ -51,7 +51,7 @@ mod tests { run(false, b"52;s;?").unwrap(), Command::ClipboardContents { kind: b's', - data: b"?".to_vec() + data: b"?".to_vec().into() } ); } @@ -62,7 +62,7 @@ mod tests { run(false, b"52;;?").unwrap(), Command::ClipboardContents { kind: b'c', - data: b"?".to_vec() + data: b"?".to_vec().into() } ); } @@ -73,7 +73,7 @@ mod tests { run(true, b"52;s;?").unwrap(), Command::ClipboardContents { kind: b's', - data: b"?".to_vec() + data: b"?".to_vec().into() } ); } @@ -84,7 +84,7 @@ mod tests { run(false, b"52;;").unwrap(), Command::ClipboardContents { kind: b'c', - data: Vec::new() + data: Vec::new().into() } ); } diff --git a/crates/ghostty-vt/src/osc/parsers/color.rs b/crates/ghostty-vt/src/osc/parsers/color.rs index 025e8e1..018ae5a 100644 --- a/crates/ghostty-vt/src/osc/parsers/color.rs +++ b/crates/ghostty-vt/src/osc/parsers/color.rs @@ -3,7 +3,7 @@ use crate::{ color::{Dynamic as DynamicColor, Rgb, Special as SpecialColor}, - osc::{Command, Parser, State, Terminator}, + osc::{ByteStorage, Command, Parser, State, Terminator}, }; /// The possible operations we support for colors. @@ -36,17 +36,23 @@ pub enum Operation { } /// Parse OSCs 4, 5, 10-19, 104, 110-119 -pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + terminator_ch: Option, +) -> Option> { if !parser.alloc { parser.state = State::Invalid; return None; } // If we've collected any extra data parse that, otherwise use an empty // string. - let data = if parser.has_writer() { - parser.buffered()? + if parser.has_writer() { + parser.buffered()?; + } + let data: &[u8] = if parser.has_writer() { + parser.payload() } else { - Vec::new() + &[] }; // Check and make sure that we're parsing the correct OSCs let op = match parser.state { @@ -80,7 +86,7 @@ pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option }; Some(Command::ColorOperation { op, - requests: parse_color(op, &data), + requests: parse_color(op, data), terminator: Terminator::init(terminator_ch), }) } diff --git a/crates/ghostty-vt/src/osc/parsers/context_signal.rs b/crates/ghostty-vt/src/osc/parsers/context_signal.rs index 6fbe1eb..78f20ed 100644 --- a/crates/ghostty-vt/src/osc/parsers/context_signal.rs +++ b/crates/ghostty-vt/src/osc/parsers/context_signal.rs @@ -6,7 +6,7 @@ //! Each context has an identifier and metadata fields. Contexts are hierarchical //! and form a stack. -use crate::osc::{Command as OscCommand, Parser, State}; +use crate::osc::{ByteStorage, Command as OscCommand, Parser, State}; /// Maximum length of a context identifier (per spec). const MAX_CONTEXT_ID_LEN: usize = 64; @@ -21,14 +21,14 @@ pub enum Action { /// A single OSC 3008 context signal command. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Command { +pub struct Command<'a> { pub action: Action, /// The context identifier. Must be 1-64 characters in the 32..126 byte range. - pub id: Vec, + pub id: crate::osc::Bytes<'a>, /// Raw unparsed metadata fields after the context ID. /// Fields are semicolon-separated key=value pairs. /// Parsed lazily via `read_option`. - pub metadata: Vec, + pub metadata: crate::osc::Bytes<'a>, } /// Value of a decoded metadata field (Zig `Field.Type()`). @@ -40,7 +40,15 @@ pub enum FieldValue<'a> { Str(&'a [u8]), } -impl Command { +impl Command<'_> { + pub fn into_owned(self) -> Command<'static> { + Command { + action: self.action, + id: super::super::own(self.id), + metadata: super::super::own(self.metadata), + } + } + /// Read a metadata field value from the raw fields string. /// Returns None if the field is not present or malformed. pub fn read_option(&self, option: Field) -> Option> { @@ -234,71 +242,83 @@ impl Field { /// Expected data format (after "3008;" prefix has been consumed by the state machine): /// start=[;=]* /// end=[;=]* -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.buffered()?; - if data.is_empty() { - parser.state = State::Invalid; - return None; - } +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.buffered()?; - // Determine the action (start= or end=) - let (action, prefix_len) = if data.starts_with(b"start=") { - (Action::Start, b"start=".len()) - } else if data.starts_with(b"end=") { - (Action::End, b"end=".len()) - } else { - log::warn!( - "OSC 3008: expected 'start=' or 'end=' prefix, got: {}", - String::from_utf8_lossy(&data[..data.len().min(10)]) - ); - parser.state = State::Invalid; - return None; - }; - - // Skip past the "start=" or "end=" prefix - let rest = &data[prefix_len..]; - - if rest.is_empty() { - log::warn!("OSC 3008: missing context ID"); - parser.state = State::Invalid; - return None; - } - - // Extract the context ID (up to the first semicolon or end of data) - let id_end = rest.iter().position(|&c| c == b';').unwrap_or(rest.len()); - let id = &rest[..id_end]; - - // Validate context ID length (1-64 chars per spec) - if id.is_empty() || id.len() > MAX_CONTEXT_ID_LEN { - log::warn!( - "OSC 3008: context ID length {} out of range (1-{})", - id.len(), - MAX_CONTEXT_ID_LEN - ); - parser.state = State::Invalid; - return None; - } - - // Validate context ID characters (32..126 byte range per spec) - for &c in id { - if !(0x20..=0x7e).contains(&c) { - log::warn!("OSC 3008: invalid character 0x{c:02x} in context ID"); + // Validate with a short-lived borrow; the command borrows the payload + // again below (so marking the parser invalid does not overlap with the + // returned borrow). + let (action, id_range, meta_start) = { + let data = parser.payload(); + if data.is_empty() { parser.state = State::Invalid; return None; } - } - // Extract raw metadata fields (everything after the ID) - let metadata = if id_end < rest.len() { - rest[id_end + 1..].to_vec() - } else { - Vec::new() + // Determine the action (start= or end=) + let (action, prefix_len) = if data.starts_with(b"start=") { + (Action::Start, b"start=".len()) + } else if data.starts_with(b"end=") { + (Action::End, b"end=".len()) + } else { + log::warn!( + "OSC 3008: expected 'start=' or 'end=' prefix, got: {}", + String::from_utf8_lossy(&data[..data.len().min(10)]) + ); + parser.state = State::Invalid; + return None; + }; + + // Skip past the "start=" or "end=" prefix + let rest = &data[prefix_len..]; + + if rest.is_empty() { + log::warn!("OSC 3008: missing context ID"); + parser.state = State::Invalid; + return None; + } + + // Extract the context ID (up to the first semicolon or end of data) + let id_end = rest.iter().position(|&c| c == b';').unwrap_or(rest.len()); + let id = &rest[..id_end]; + + // Validate context ID length (1-64 chars per spec) + if id.is_empty() || id.len() > MAX_CONTEXT_ID_LEN { + log::warn!( + "OSC 3008: context ID length {} out of range (1-{})", + id.len(), + MAX_CONTEXT_ID_LEN + ); + parser.state = State::Invalid; + return None; + } + + // Validate context ID characters (32..126 byte range per spec) + for &c in id { + if !(0x20..=0x7e).contains(&c) { + log::warn!("OSC 3008: invalid character 0x{c:02x} in context ID"); + parser.state = State::Invalid; + return None; + } + } + + // Metadata fields are everything after the ID. + let meta_start = if id_end < rest.len() { + prefix_len + id_end + 1 + } else { + data.len() + }; + (action, prefix_len..prefix_len + id_end, meta_start) }; + let data = parser.payload(); Some(OscCommand::ContextSignal(Command { action, - id: id.to_vec(), - metadata, + id: data[id_range].into(), + metadata: data[meta_start..].into(), })) } @@ -307,15 +327,15 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option Option { + fn run(input: &str) -> Option> { let mut p = Parser::init(false); for &ch in input.as_bytes() { p.next(ch); } - p.end(None) + p.end(None).map(OscCommand::into_owned) } - fn cs(input: &str) -> Command { + fn cs(input: &str) -> Command<'static> { match run(input) { Some(OscCommand::ContextSignal(c)) => c, other => panic!("unexpected {other:?}"), diff --git a/crates/ghostty-vt/src/osc/parsers/hyperlink.rs b/crates/ghostty-vt/src/osc/parsers/hyperlink.rs index 0231cbf..d1dd290 100644 --- a/crates/ghostty-vt/src/osc/parsers/hyperlink.rs +++ b/crates/ghostty-vt/src/osc/parsers/hyperlink.rs @@ -1,57 +1,59 @@ //! OSC 8 hyperlinks (ZIG/src/terminal/osc/parsers/hyperlink.zig). -use crate::osc::{Command, Parser, State}; +use crate::osc::{ByteStorage, Command, Parser, State}; /// Parse OSC 8 hyperlinks -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.data()?; - let Some(s) = data.iter().position(|&b| b == b';') else { - parser.state = State::Invalid; - return None; +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.data()?; + + // Work out the layout with a short-lived borrow of the payload; the + // command is built from a fresh borrow at the end (so that marking the + // parser invalid above does not conflict with the returned borrow). + let (s, id) = { + let data = parser.payload(); + let Some(s) = data.iter().position(|&b| b == b';') else { + parser.state = State::Invalid; + return None; + }; + + let mut id: Option<(usize, usize)> = None; + + // The params section is `key=value` pairs separated by ':'. This + // follows the Zig implementation byte for byte: ':' and the ';' + // terminator are turned into NULs and the segments are walked by + // searching for the next NUL starting one past the segment start. The + // NUL-converted copy Zig makes in place is represented by `is_nul` + // instead (no allocation). + let is_nul = |i: usize| i == s || data[i] == b':' || data[i] == 0; + let mut kv_start = 0; + while kv_start <= s { + let Some(kv_end) = (kv_start + 1..=s).find(|&i| is_nul(i)) else { + break; + }; + let kv = &data[kv_start..kv_end]; + let Some(v) = kv.iter().position(|&b| b == b'=') else { + break; + }; + let key = &kv[..v]; + if key == b"id" { + if v + 1 < kv.len() { + id = Some((kv_start + v + 1, kv_end)); + } + } else { + log::warn!( + "unknown hyperlink option: '{}'", + String::from_utf8_lossy(key) + ); + } + kv_start = kv_end + 1; + } + (s, id) }; - let uri = data[s + 1..].to_vec(); - let mut id: Option> = None; - - // The params section is `key=value` pairs separated by ':'. This follows - // the Zig implementation byte for byte: ':' and the ';' terminator are - // turned into NULs and the segments are walked by searching for the next - // NUL starting one past the segment start. - let mut kvs = data[..s].to_vec(); - kvs.push(0); - for b in &mut kvs { - if *b == b':' { - *b = 0; - } - } - let mut kv_start = 0; - while kv_start < kvs.len() { - let Some(kv_end) = kvs - .iter() - .skip(kv_start + 1) - .position(|&b| b == 0) - .map(|i| i + kv_start + 1) - else { - break; - }; - let kv = &kvs[kv_start..kv_end]; - let Some(v) = kv.iter().position(|&b| b == b'=') else { - break; - }; - let key = &kv[..v]; - let value = &kv[v + 1..]; - if key == b"id" { - if !value.is_empty() { - id = Some(value.to_vec()); - } - } else { - log::warn!( - "unknown hyperlink option: '{}'", - String::from_utf8_lossy(key) - ); - } - kv_start = kv_end + 1; - } - + let data = parser.payload(); + let uri = &data[s + 1..]; if uri.is_empty() { if id.is_some() { parser.state = State::Invalid; @@ -60,7 +62,11 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option return Some(Command::HyperlinkEnd); } - Some(Command::HyperlinkStart { id, uri }) + let data = parser.payload(); + Some(Command::HyperlinkStart { + id: id.map(|(a, b)| data[a..b].into()), + uri: data[s + 1..].into(), + }) } #[cfg(test)] @@ -68,18 +74,18 @@ mod tests { #![allow(clippy::unwrap_used)] use super::*; - fn run(input: &[u8]) -> Option { + fn run(input: &[u8]) -> Option> { let mut p = Parser::init(false); for c in input { p.next(*c); } - p.end(Some(0x1b)) + p.end(Some(0x1b)).map(Command::into_owned) } - fn start(id: Option<&str>, uri: &str) -> Command { + fn start(id: Option<&str>, uri: &str) -> Command<'static> { Command::HyperlinkStart { - id: id.map(|s| s.as_bytes().to_vec()), - uri: uri.as_bytes().to_vec(), + id: id.map(|s| s.as_bytes().to_vec().into()), + uri: uri.as_bytes().to_vec().into(), } } diff --git a/crates/ghostty-vt/src/osc/parsers/iterm2.rs b/crates/ghostty-vt/src/osc/parsers/iterm2.rs index ba0b6f5..7d43b8b 100644 --- a/crates/ghostty-vt/src/osc/parsers/iterm2.rs +++ b/crates/ghostty-vt/src/osc/parsers/iterm2.rs @@ -1,6 +1,6 @@ //! OSC 1337 (iTerm2) (ZIG/src/terminal/osc/parsers/iterm2.zig). -use crate::osc::{Command, Parser, State}; +use crate::osc::{ByteStorage, Command, Parser, State}; /// iTerm2 OSC 1337 keys. Lookup is ASCII case-insensitive. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -90,13 +90,17 @@ fn lookup(s: &[u8]) -> Option { /// Parse OSC 1337 /// https://iterm2.com/documentation-escape-codes.html -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S1337); - let data = parser.data()?; + parser.data()?; + let data = parser.payload(); let (key_str, value): (&[u8], Option<&[u8]>) = match data.iter().position(|&b| b == b'=') { - None => (&data[..], None), + None => (data, None), Some(index) => (&data[..index], Some(&data[index + 1..])), }; @@ -138,7 +142,7 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option Some(Command::ClipboardContents { kind: b'c', - data: value.to_vec(), + data: value.into(), }) } @@ -148,7 +152,7 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option return None; } Some(Command::ReportPwd { - value: value.to_vec(), + value: value.into(), }) } @@ -164,12 +168,12 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option mod tests { use super::*; - fn run(input: &str) -> Option { + fn run(input: &str) -> Option> { let mut p = Parser::init(true); for ch in input.bytes() { p.next(ch); } - p.end(Some(0x1b)) + p.end(Some(0x1b)).map(Command::into_owned) } #[test] @@ -257,7 +261,7 @@ mod tests { run("1337;Copy=:YWJjMTIz"), Some(Command::ClipboardContents { kind: b'c', - data: b"YWJjMTIz".to_vec() + data: b"YWJjMTIz".to_vec().into() }) ); } @@ -277,7 +281,7 @@ mod tests { assert_eq!( run("1337;CurrentDir=abc123"), Some(Command::ReportPwd { - value: b"abc123".to_vec() + value: b"abc123".to_vec().into() }) ); } diff --git a/crates/ghostty-vt/src/osc/parsers/kitty_clipboard_protocol.rs b/crates/ghostty-vt/src/osc/parsers/kitty_clipboard_protocol.rs index 3234b41..39f5e4d 100644 --- a/crates/ghostty-vt/src/osc/parsers/kitty_clipboard_protocol.rs +++ b/crates/ghostty-vt/src/osc/parsers/kitty_clipboard_protocol.rs @@ -3,15 +3,15 @@ //! https://rockorager.dev/misc/bracketed-paste-mime/ //! (ZIG/src/terminal/osc/parsers/kitty_clipboard_protocol.zig) -use crate::osc::{Command, Parser, State, Terminator}; +use crate::osc::{ByteStorage, Command, Parser, State, Terminator}; #[derive(Debug, Clone, PartialEq, Eq)] -pub struct OSC { +pub struct OSC<'a> { /// The raw metadata that was received. It can be parsed by using the /// `read_option` method (or the typed helpers). - pub metadata: Vec, + pub metadata: crate::osc::Bytes<'a>, /// The raw payload. It may be Base64 encoded, check the `e` option. - pub payload: Option>, + pub payload: Option>, /// The terminator that was used in case we need to send a response. pub terminator: Terminator, } @@ -25,7 +25,15 @@ pub enum OptionValue<'a> { Type(Operation), } -impl OSC { +impl OSC<'_> { + pub fn into_owned(self) -> OSC<'static> { + OSC { + metadata: super::super::own(self.metadata), + payload: self.payload.map(super::super::own), + terminator: self.terminator, + } + } + /// Decode an option from the metadata. pub fn read_option(&self, key: Opt) -> Option> { key.read(&self.metadata) @@ -283,14 +291,18 @@ fn parse_identifier(s: &[u8]) -> Option<&[u8]> { } } -pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S5522); - let data = parser.buffered()?; + parser.buffered()?; + let data = parser.payload(); let (metadata, payload) = match data.iter().position(|&c| c == b';') { - None => (data, None), - Some(start) => (data[..start].to_vec(), Some(data[start + 1..].to_vec())), + None => (data.into(), None), + Some(start) => (data[..start].into(), Some(data[start + 1..].into())), }; Some(Command::KittyClipboardProtocol(OSC { @@ -305,13 +317,13 @@ pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option mod tests { use super::*; - fn run(input: &str) -> OSC { + fn run(input: &str) -> OSC<'static> { let mut p = Parser::init(true); for &ch in input.as_bytes() { p.next(ch); } match p.end(Some(0x1b)) { - Some(Command::KittyClipboardProtocol(o)) => o, + Some(Command::KittyClipboardProtocol(o)) => o.into_owned(), other => panic!("unexpected {other:?}"), } } @@ -337,7 +349,7 @@ mod tests { assert_eq!(m.as_bytes(), &c.metadata[..]); } if let Some(p) = e.payload { - assert_eq!(p.map(|s| s.as_bytes().to_vec()), c.payload); + assert_eq!(p.map(str::as_bytes), c.payload.as_deref()); } assert_eq!(e.id.map(str::as_bytes), c.id()); assert_eq!(e.loc, c.loc()); diff --git a/crates/ghostty-vt/src/osc/parsers/kitty_color.rs b/crates/ghostty-vt/src/osc/parsers/kitty_color.rs index 85c4d6e..7b0af47 100644 --- a/crates/ghostty-vt/src/osc/parsers/kitty_color.rs +++ b/crates/ghostty-vt/src/osc/parsers/kitty_color.rs @@ -3,11 +3,14 @@ use crate::{ color::Rgb, kitty::color::{Kind, OSC, Request}, - osc::{Command, Parser, State, Terminator}, + osc::{ByteStorage, Command, Parser, State, Terminator}, }; /// Parse OSC 21, the Kitty Color Protocol. -pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S21); if !parser.alloc { @@ -18,8 +21,12 @@ pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option parser.state = State::Invalid; return None; } - let data = parser.buffered()?; + parser.buffered()?; + // The parse only reads the payload; failure marks the parser invalid + // after the borrow ends. + let data = parser.payload(); + let mut exceeded = false; let mut osc = OSC { list: Vec::new(), terminator: Terminator::init(terminator_ch), @@ -27,8 +34,8 @@ pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option for kv in data.split(|&c| c == b';') { if osc.list.len() >= Kind::MAX * 2 { log::warn!("exceeded limit for number of keys in kitty color protocol, ignoring"); - parser.state = State::Invalid; - return None; + exceeded = true; + break; } let (k, rest) = match kv.iter().position(|&c| c == b'=') { Some(p) => (&kv[..p], &kv[p + 1..]), @@ -59,6 +66,10 @@ pub fn parse(parser: &mut Parser, terminator_ch: Option) -> Option ); } } + if exceeded { + parser.state = State::Invalid; + return None; + } Some(Command::KittyColorProtocol(osc)) } diff --git a/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs b/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs index a87331e..aa5cf7c 100644 --- a/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs +++ b/crates/ghostty-vt/src/osc/parsers/kitty_text_sizing.rs @@ -2,7 +2,7 @@ //! Specification: https://sw.kovidgoyal.net/kitty/text-sizing-protocol/ //! (ZIG/src/terminal/osc/parsers/kitty_text_sizing.zig) -use crate::osc::{Command, Parser, State, encoding}; +use crate::osc::{ByteStorage, Command, Parser, State, encoding}; pub const MAX_PAYLOAD_LENGTH: usize = 4096; @@ -45,7 +45,7 @@ impl HAlign { } #[derive(Debug, Clone, PartialEq, Eq)] -pub struct OSC { +pub struct OSC<'a> { /// 1 - 7 (u3) pub scale: u8, /// 0 - 7 (u3), 0 means default @@ -56,7 +56,7 @@ pub struct OSC { pub denominator: u8, pub valign: VAlign, pub halign: HAlign, - pub text: Vec, + pub text: std::borrow::Cow<'a, [u8]>, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -91,7 +91,19 @@ fn parse_u4(value: &[u8]) -> Option { Some(acc as u8) } -impl OSC { +impl OSC<'_> { + pub fn into_owned(self) -> OSC<'static> { + OSC { + scale: self.scale, + width: self.width, + numerator: self.numerator, + denominator: self.denominator, + valign: self.valign, + halign: self.halign, + text: super::super::own(self.text), + } + } + fn update(&mut self, key: u8, value: &[u8]) -> Result<(), UpdateError> { // All values are numeric, so we can do a small hack here let v = parse_u4(value).ok_or(UpdateError::InvalidValue)?; @@ -119,31 +131,41 @@ impl OSC { } } -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S66); - let data = parser.data()?; + parser.data()?; - let Some(payload_start) = data.iter().position(|&c| c == b';') else { - log::warn!("missing semicolon before payload"); - parser.state = State::Invalid; - return None; + // Validate with a short-lived borrow (marking the parser invalid cannot + // overlap with the borrow returned in the command below). + let payload_start = { + let data = parser.payload(); + let Some(payload_start) = data.iter().position(|&c| c == b';') else { + log::warn!("missing semicolon before payload"); + parser.state = State::Invalid; + return None; + }; + let payload = &data[payload_start + 1..]; + + // Payload has to be a URL-safe UTF-8 string, + // and be under the size limit. + if payload.len() > MAX_PAYLOAD_LENGTH { + log::warn!("payload is too long"); + parser.state = State::Invalid; + return None; + } + if !encoding::is_safe_utf8(payload) { + log::warn!("payload is not escape code safe UTF-8"); + parser.state = State::Invalid; + return None; + } + payload_start }; - let payload = &data[payload_start + 1..]; - - // Payload has to be a URL-safe UTF-8 string, - // and be under the size limit. - if payload.len() > MAX_PAYLOAD_LENGTH { - log::warn!("payload is too long"); - parser.state = State::Invalid; - return None; - } - if !encoding::is_safe_utf8(payload) { - log::warn!("payload is not escape code safe UTF-8"); - parser.state = State::Invalid; - return None; - } + let data = parser.payload(); let mut cmd = OSC { scale: 1, width: 0, @@ -151,7 +173,7 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option denominator: 0, valign: VAlign::Top, halign: HAlign::Left, - text: payload.to_vec(), + text: data[payload_start + 1..].into(), }; // Parse any arguments if given @@ -191,15 +213,15 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option mod tests { use super::*; - fn run(input: &[u8]) -> Option { + fn run(input: &[u8]) -> Option> { let mut p = Parser::init(false); for &ch in input { p.next(ch); } - p.end(Some(0x1b)) + p.end(Some(0x1b)).map(Command::into_owned) } - fn sizing(input: &[u8]) -> OSC { + fn sizing(input: &[u8]) -> OSC<'static> { match run(input) { Some(Command::KittyTextSizing(o)) => o, other => panic!("unexpected {other:?}"), diff --git a/crates/ghostty-vt/src/osc/parsers/mouse_shape.rs b/crates/ghostty-vt/src/osc/parsers/mouse_shape.rs index 364f61c..b35ae1c 100644 --- a/crates/ghostty-vt/src/osc/parsers/mouse_shape.rs +++ b/crates/ghostty-vt/src/osc/parsers/mouse_shape.rs @@ -1,11 +1,16 @@ //! OSC 22 (ZIG/src/terminal/osc/parsers/mouse_shape.zig). -use crate::osc::{Command, Parser, State}; +use crate::osc::{ByteStorage, Command, Parser, State}; /// Parse OSC 22 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { debug_assert_eq!(parser.state, State::S22); - let data = parser.data()?; - Some(Command::MouseShape { value: data }) + parser.data()?; + Some(Command::MouseShape { + value: parser.payload().into(), + }) } #[cfg(test)] @@ -22,7 +27,7 @@ mod tests { assert_eq!( p.end(None).unwrap(), Command::MouseShape { - value: b"pointer".to_vec() + value: b"pointer".to_vec().into() } ); } diff --git a/crates/ghostty-vt/src/osc/parsers/osc9.rs b/crates/ghostty-vt/src/osc/parsers/osc9.rs index 898dd7d..b7f08ea 100644 --- a/crates/ghostty-vt/src/osc/parsers/osc9.rs +++ b/crates/ghostty-vt/src/osc/parsers/osc9.rs @@ -2,7 +2,14 @@ //! (ZIG/src/terminal/osc/parsers/osc9.zig). use super::semantic_prompt; -use crate::osc::{Command, ConemuChangeTabTitle, Parser, ProgressReport, ProgressState}; +use crate::osc::{ + ByteStorage, + Command, + ConemuChangeTabTitle, + Parser, + ProgressReport, + ProgressState, +}; /// Parse a base-10 unsigned integer from raw bytes (Zig `parseUnsigned`). fn parse_unsigned(bytes: &[u8]) -> Option { @@ -10,16 +17,20 @@ fn parse_unsigned(bytes: &[u8]) -> Option { } /// Parse OSC 9, which could be an iTerm2 notification or a ConEmu extension. -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { // Zig prologue: writer must exist. The NUL-terminator reservation is NOT // done here: Zig only appends the NUL (`writer.writeByte(0)`) in the // branches that return text, so full-buffer control commands (sleep, // xterm emulation, progress, ...) still parse. - let data = parser.buffered()?; + parser.buffered()?; + let data = parser.payload(); // Check first to see if this is a ConEmu OSC // https://conemu.github.io/en/AnsiEscapeCodes.html#ConEmu_specific_OSC - if let Some((cmd, needs_nul)) = conemu(&data) { + if let Some((cmd, needs_nul)) = conemu(data) { if needs_nul && !parser.reserve_nul() { return None; } @@ -31,14 +42,14 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option return None; } Some(Command::ShowDesktopNotification { - title: Vec::new(), - body: data, + title: std::borrow::Cow::Borrowed(&[]), + body: data.into(), }) } /// Returns None where the Zig code does `break :conemu`. The flag is true /// when the Zig branch appends a NUL terminator (needs buffer room). -fn conemu(data: &[u8]) -> Option<(Command, bool)> { +fn conemu(data: &[u8]) -> Option<(Command<'_>, bool)> { let r = conemu_inner(data)?; let needs_nul = matches!( r, @@ -53,11 +64,11 @@ fn conemu(data: &[u8]) -> Option<(Command, bool)> { Some((r, needs_nul)) } -fn conemu_inner(data: &[u8]) -> Option { +fn conemu_inner(data: &[u8]) -> Option> { if data.is_empty() { return None; } - let msg = |skip: usize| data[skip..].to_vec(); + let msg = |skip: usize| -> crate::osc::Bytes<'_> { data[skip..].into() }; match data[0] { // Check for OSC 9;1 9;10 9;11 9;12 b'1' => { @@ -210,18 +221,18 @@ fn conemu_inner(data: &[u8]) -> Option { mod tests { use super::*; - fn run(input: &str) -> Command { + fn run(input: &str) -> Command<'static> { let mut p = Parser::init(false); for ch in input.bytes() { p.next(ch); } - p.end(Some(0x1b)).unwrap() + p.end(Some(0x1b)).unwrap().into_owned() } fn notif(input: &str, body: &str) { match run(input) { Command::ShowDesktopNotification { title, body: b } => { - assert_eq!(title, b""); + assert_eq!(&title[..], b""); assert_eq!(b, body.as_bytes()); } c => panic!("unexpected {c:?}"), @@ -242,17 +253,17 @@ mod tests { ); } - fn run_opt(input: &str) -> Option { + fn run_opt(input: &str) -> Option> { let mut p = Parser::init(false); for ch in input.bytes() { p.next(ch); } - p.end(Some(0x1b)) + p.end(Some(0x1b)).map(Command::into_owned) } #[test] fn full_buffer_control_commands_do_not_need_nul() { - let fill = Parser::MAX_BUF; + let fill = crate::osc::MAX_BUF; // OSC 9;1 sleep with a buffer that has no room for a NUL: Zig never // appends one for this command, so it still parses (default 100ms). let input = format!("9;1;{}", "x".repeat(fill - 2)); @@ -267,7 +278,7 @@ mod tests { #[test] fn full_buffer_text_commands_are_invalid() { - let fill = Parser::MAX_BUF; + let fill = crate::osc::MAX_BUF; // These append a NUL in Zig, which fails on a full buffer. let input = format!("9;6;{}", "a".repeat(fill - 2)); assert_eq!(run_opt(&input), None); @@ -322,7 +333,7 @@ mod tests { fn osc_9_2_conemu_message_box() { assert_eq!( run("9;2;hello world"), - Command::ConemuShowMessageBox(b"hello world".to_vec()) + Command::ConemuShowMessageBox(b"hello world".to_vec().into()) ); } @@ -333,14 +344,17 @@ mod tests { #[test] fn osc_9_2_conemu_message_box_empty_message() { - assert_eq!(run("9;2;"), Command::ConemuShowMessageBox(Vec::new())); + assert_eq!( + run("9;2;"), + Command::ConemuShowMessageBox(Vec::new().into()) + ); } #[test] fn osc_9_2_conemu_message_box_spaces_only_message() { assert_eq!( run("9;2; "), - Command::ConemuShowMessageBox(b" ".to_vec()) + Command::ConemuShowMessageBox(b" ".to_vec().into()) ); } @@ -358,7 +372,7 @@ mod tests { fn osc_9_3_conemu_change_tab_title() { assert_eq!( run("9;3;foo bar"), - Command::ConemuChangeTabTitle(ConemuChangeTabTitle::Value(b"foo bar".to_vec())) + Command::ConemuChangeTabTitle(ConemuChangeTabTitle::Value(b"foo bar".to_vec().into())) ); } @@ -374,7 +388,7 @@ mod tests { fn osc_9_3_conemu_change_tab_title_spaces_only() { assert_eq!( run("9;3; "), - Command::ConemuChangeTabTitle(ConemuChangeTabTitle::Value(b" ".to_vec())) + Command::ConemuChangeTabTitle(ConemuChangeTabTitle::Value(b" ".to_vec().into())) ); } @@ -488,12 +502,15 @@ mod tests { #[test] fn osc_9_6_conemu_guimacro_1() { - assert_eq!(run("9;6;a"), Command::ConemuGuimacro(b"a".to_vec())); + assert_eq!(run("9;6;a"), Command::ConemuGuimacro(b"a".to_vec().into())); } #[test] fn osc_9_6_conemu_guimacro_2() { - assert_eq!(run("9;6;ab"), Command::ConemuGuimacro(b"ab".to_vec())); + assert_eq!( + run("9;6;ab"), + Command::ConemuGuimacro(b"ab".to_vec().into()) + ); } #[test] @@ -503,12 +520,15 @@ mod tests { #[test] fn osc_9_7_conemu_run_process_1() { - assert_eq!(run("9;7;ab"), Command::ConemuRunProcess(b"ab".to_vec())); + assert_eq!( + run("9;7;ab"), + Command::ConemuRunProcess(b"ab".to_vec().into()) + ); } #[test] fn osc_9_7_conemu_run_process_2() { - assert_eq!(run("9;7;"), Command::ConemuRunProcess(Vec::new())); + assert_eq!(run("9;7;"), Command::ConemuRunProcess(Vec::new().into())); } #[test] @@ -520,7 +540,7 @@ mod tests { fn osc_9_8_conemu_output_environment_variable_1() { assert_eq!( run("9;8;ab"), - Command::ConemuOutputEnvironmentVariable(b"ab".to_vec()) + Command::ConemuOutputEnvironmentVariable(b"ab".to_vec().into()) ); } @@ -528,7 +548,7 @@ mod tests { fn osc_9_8_conemu_output_environment_variable_2() { assert_eq!( run("9;8;"), - Command::ConemuOutputEnvironmentVariable(Vec::new()) + Command::ConemuOutputEnvironmentVariable(Vec::new().into()) ); } @@ -542,7 +562,7 @@ mod tests { assert_eq!( run("9;9;ab"), Command::ReportPwd { - value: b"ab".to_vec() + value: b"ab".to_vec().into() } ); } @@ -594,7 +614,10 @@ mod tests { #[test] fn osc_9_11_conemu_comment() { - assert_eq!(run("9;11;ab"), Command::ConemuComment(b"ab".to_vec())); + assert_eq!( + run("9;11;ab"), + Command::ConemuComment(b"ab".to_vec().into()) + ); } #[test] diff --git a/crates/ghostty-vt/src/osc/parsers/report_pwd.rs b/crates/ghostty-vt/src/osc/parsers/report_pwd.rs index f510687..6ed3cc3 100644 --- a/crates/ghostty-vt/src/osc/parsers/report_pwd.rs +++ b/crates/ghostty-vt/src/osc/parsers/report_pwd.rs @@ -1,10 +1,15 @@ //! OSC 7 (ZIG/src/terminal/osc/parsers/report_pwd.zig). -use crate::osc::{Command, Parser}; +use crate::osc::{ByteStorage, Command, Parser}; /// Parse OSC 7 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.data()?; - Some(Command::ReportPwd { value: data }) +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.data()?; + Some(Command::ReportPwd { + value: parser.payload().into(), + }) } #[cfg(test)] @@ -21,7 +26,7 @@ mod tests { assert_eq!( p.end(None).unwrap(), Command::ReportPwd { - value: b"file:///tmp/example".to_vec() + value: b"file:///tmp/example".to_vec().into() } ); } @@ -34,7 +39,9 @@ mod tests { } assert_eq!( p.end(None).unwrap(), - Command::ReportPwd { value: Vec::new() } + Command::ReportPwd { + value: Vec::new().into() + } ); } } diff --git a/crates/ghostty-vt/src/osc/parsers/rxvt_extension.rs b/crates/ghostty-vt/src/osc/parsers/rxvt_extension.rs index 8a08ab5..4896a61 100644 --- a/crates/ghostty-vt/src/osc/parsers/rxvt_extension.rs +++ b/crates/ghostty-vt/src/osc/parsers/rxvt_extension.rs @@ -1,31 +1,39 @@ //! OSC 777 (ZIG/src/terminal/osc/parsers/rxvt_extension.zig). -use crate::osc::{Command, Parser, State}; +use crate::osc::{ByteStorage, Command, Parser, State}; /// Parse OSC 777 -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.data()?; - let Some(k) = data.iter().position(|&b| b == b';') else { - parser.state = State::Invalid; - return None; - }; - let ext = &data[..k]; - if ext != b"notify" { - log::warn!("unknown rxvt extension: {}", String::from_utf8_lossy(ext)); - parser.state = State::Invalid; - return None; - } - let Some(t) = data[k + 1..] - .iter() - .position(|&b| b == b';') - .map(|i| i + k + 1) - else { - log::warn!("rxvt notify extension is missing the title"); - parser.state = State::Invalid; - return None; +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.data()?; + let (k, t) = { + let data = parser.payload(); + let Some(k) = data.iter().position(|&b| b == b';') else { + parser.state = State::Invalid; + return None; + }; + let ext = &data[..k]; + if ext != b"notify" { + log::warn!("unknown rxvt extension: {}", String::from_utf8_lossy(ext)); + parser.state = State::Invalid; + return None; + } + let Some(t) = data[k + 1..] + .iter() + .position(|&b| b == b';') + .map(|i| i + k + 1) + else { + log::warn!("rxvt notify extension is missing the title"); + parser.state = State::Invalid; + return None; + }; + (k, t) }; + let data = parser.payload(); Some(Command::ShowDesktopNotification { - title: data[k + 1..t].to_vec(), - body: data[t + 1..].to_vec(), + title: data[k + 1..t].into(), + body: data[t + 1..].into(), }) } @@ -43,8 +51,8 @@ mod tests { assert_eq!( p.end(Some(0x1b)).unwrap(), Command::ShowDesktopNotification { - title: b"Title".to_vec(), - body: b"Body".to_vec() + title: b"Title".to_vec().into(), + body: b"Body".to_vec().into() } ); } diff --git a/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs b/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs index 98c2c67..70334ce 100644 --- a/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs +++ b/crates/ghostty-vt/src/osc/parsers/semantic_prompt.rs @@ -4,7 +4,7 @@ //! use crate::{ - osc::{Command as OscCommand, Parser, State}, + osc::{ByteStorage, Command as OscCommand, Parser, State}, string_encoding::{printf_q_decode, url_percent_decode}, }; @@ -19,9 +19,9 @@ pub type DecodeError = crate::string_encoding::Error; /// all except one do and the spec does also say to ignore unknown /// options. So, I think this is a fair interpretation. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct Command { +pub struct Command<'a> { pub action: Action, - pub options_unvalidated: Vec, + pub options_unvalidated: crate::osc::Bytes<'a>, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -36,11 +36,18 @@ pub enum Action { EndCommand, // 'D' } -impl Command { - pub fn init(action: Action) -> Command { +impl<'a> Command<'a> { + pub fn into_owned(self) -> Command<'static> { + Command { + action: self.action, + options_unvalidated: super::super::own(self.options_unvalidated), + } + } + + pub fn init(action: Action) -> Command<'a> { Command { action, - options_unvalidated: Vec::new(), + options_unvalidated: std::borrow::Cow::Borrowed(&[]), } } @@ -364,43 +371,54 @@ pub enum Redraw { } /// Parse OSC 133, semantic prompts -pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option { - let data = parser.buffered()?; - if data.is_empty() { - parser.state = State::Invalid; - return None; - } +pub fn parse( + parser: &mut Parser, + _terminator_ch: Option, +) -> Option> { + parser.buffered()?; - // All valid cases terminate within this block. Any fallthroughs - // are invalid. - let action = match data[0] { - b'A' => Action::FreshLineNewPrompt, - b'B' => Action::EndPromptStartInput, - b'I' => Action::EndPromptStartInputTerminateEol, - b'C' => Action::EndInputStartOutput, - b'D' => Action::EndCommand, - b'N' => Action::NewCommand, - b'P' => Action::PromptStart, - b'L' => { - if data.len() > 1 { + // Validate with a short-lived borrow (marking the parser invalid cannot + // overlap with the borrow handed to the command below). + let (action, has_options) = { + let data = parser.payload(); + if data.is_empty() { + parser.state = State::Invalid; + return None; + } + + // All valid cases terminate within this block. Any fallthroughs + // are invalid. + let action = match data[0] { + b'A' => Action::FreshLineNewPrompt, + b'B' => Action::EndPromptStartInput, + b'I' => Action::EndPromptStartInputTerminateEol, + b'C' => Action::EndInputStartOutput, + b'D' => Action::EndCommand, + b'N' => Action::NewCommand, + b'P' => Action::PromptStart, + b'L' => { + if data.len() > 1 { + parser.state = State::Invalid; + return None; + } + return Some(OscCommand::SemanticPrompt(Command::init(Action::FreshLine))); + } + _ => { parser.state = State::Invalid; return None; } - return Some(OscCommand::SemanticPrompt(Command::init(Action::FreshLine))); - } - _ => { + }; + + if data.len() > 1 && data[1] != b';' { parser.state = State::Invalid; return None; } + (action, data.len() > 1) }; let mut cmd = Command::init(action); - if data.len() > 1 { - if data[1] != b';' { - parser.state = State::Invalid; - return None; - } - cmd.options_unvalidated = data[2..].to_vec(); + if has_options { + cmd.options_unvalidated = parser.payload()[2..].into(); } Some(OscCommand::SemanticPrompt(cmd)) } @@ -410,19 +428,19 @@ pub fn parse(parser: &mut Parser, _terminator_ch: Option) -> Option Option { + fn run(input: &str) -> Option> { let mut p = Parser::init(false); for ch in input.bytes() { p.next(ch); } match p.end(None) { - Some(OscCommand::SemanticPrompt(c)) => Some(c), + Some(OscCommand::SemanticPrompt(c)) => Some(c.into_owned()), Some(c) => panic!("unexpected {c:?}"), None => None, } } - fn cmdline(input: &str) -> (Command, Result, DecodeError>) { + fn cmdline(input: &str) -> (Command<'static>, Result, DecodeError>) { let cmd = run(input).unwrap(); assert_eq!(cmd.action, Action::EndInputStartOutput); let mut w = Vec::new(); diff --git a/crates/ghostty-vt/src/parser.rs b/crates/ghostty-vt/src/parser.rs index aa0fd06..2a29f4e 100644 --- a/crates/ghostty-vt/src/parser.rs +++ b/crates/ghostty-vt/src/parser.rs @@ -242,7 +242,7 @@ pub enum Action { EscDispatch(Esc), /// Execute the OSC command. - OscDispatch(osc::Command), + OscDispatch(osc::Command<'static>), /// DCS-related events. DcsHook(Dcs), @@ -358,7 +358,7 @@ impl Parser { match self.state { State::OscString => { if let Some(cmd) = self.osc_parser.end(Some(c)) { - f(0, Action::OscDispatch(cmd)); + f(0, Action::OscDispatch(cmd.into_owned())); } } State::DcsPassthrough => f(0, Action::DcsUnhook), @@ -840,7 +840,7 @@ mod tests { assert_eq!( a[0], Some(Action::OscDispatch(Command::ChangeWindowTitle( - b"abc".to_vec() + b"abc".to_vec().into() ))) ); } @@ -860,7 +860,7 @@ mod tests { assert_eq!( a[0], Some(Action::OscDispatch(Command::ChangeWindowTitle( - b"abc".to_vec() + b"abc".to_vec().into() ))) ); } diff --git a/crates/ghostty-vt/src/stream.rs b/crates/ghostty-vt/src/stream.rs index a9ec505..780da95 100644 --- a/crates/ghostty-vt/src/stream.rs +++ b/crates/ghostty-vt/src/stream.rs @@ -153,7 +153,7 @@ define_actions! { SetAttribute(sgr::Attribute<'a>), KittyColorReport(&'a kitty::color::OSC), ColorOperation(ColorOperation<'a>), - SemanticPrompt(&'a osc::SemanticPrompt), + SemanticPrompt(&'a osc::SemanticPrompt<'a>), } /// Field types of [`Action`] payloads. @@ -253,7 +253,7 @@ pub struct ColorOperation<'a> { } /// Alias matching Zig `Action.SemanticPrompt`. -pub type SemanticPrompt = osc::SemanticPrompt; +pub type SemanticPrompt<'a> = osc::SemanticPrompt<'a>; /// The receiver of [`Action`]s (Zig: a type with a /// `vt(comptime action, value) !void` method). diff --git a/docs/rewrite/notes/final-Z5.md b/docs/rewrite/notes/final-Z5.md new file mode 100644 index 0000000..ddc64d2 --- /dev/null +++ b/docs/rewrite/notes/final-Z5.md @@ -0,0 +1,22 @@ +# Z5 final pass: remaining RV4 "still partial" rows + +| RV4 row | Fix | Test | +|---|---|---| +| Surface: Flatpak cwd (os/flatpak.rs) | cwd and argv decoded with `config_str_to_bytes` before the D-Bus `ay`; fallback cwd uses `os_bytes(current_dir)` | `flatpak::tests::dbus_bytes_are_decoded` | +| Surface: screen-file filename (core/surface/binding.rs) | returns `path_to_config_string(canonical)`; paste writes decoded bytes, clipboard gets the decoded text | `screen_temp_file_returns_encoded_non_utf8_path` | +| Config: default template (config.rs) | `write_config_template` decodes the path for `create_dir_all`/`write`; template rendered as bytes (`{[path]s}` replaced by the raw path bytes) | `config_template_written_at_non_utf8_path` | +| resourcesdir consumers | `GHOSTTY_RESOURCES_DIR`, `TERMINFO` (`terminfo_dir`), shell-integration paths/env (`resource_path`), `darwin_bundle_env` (XDG_DATA_DIRS/MANPATH, new `append_env_always_os`) and `i18n::init` (bindtextdomain takes a `PathBuf`) all decode the encoded resources dir | existing termio/shell_integration tests + compile on all targets | +| config/path: custom shader load (shadertoy.rs) | `config_str_to_path` (ghostty-os is now a regular renderer dependency) | covered by load tests | +| cli/list_themes preview link | `to_uri` percent-encodes the decoded original bytes | `uri_escaping_percent_encodes_original_bytes` | +| cli/args + osbytes (non-Unix) | `os_to_config_string` on non-Unix goes through shared `valid_str_to_config_string` (escapes literal U+FDD0); cfg-independent | `valid_str_encoder_escapes_literal_escape_char` | +| gtk surface paste confirmation | `show_clipboard_confirmation(.., encoded)`; only `paste_bytes` (non-UTF-8) is flagged and decoded via `confirmed_paste_bytes`; ordinary text with U+FDD0 U+00FF is pasted verbatim | `confirmed_paste_decodes_only_encoded_text` | +| shadertoy whole-array uniform copies | naga rejects `float t[4] = iChannelTime;` because the std140 array has stride 16 (type mismatch), so the MSL text patch alone could not help. The prefix member is renamed `iChannelTimeUbo` and `iChannelTime` is a macro for `iChannelTime_plain()`, which rebuilds the plain `float[4]` from the padded elements; element reads and the MSL stride patch keep working (`iChannelTimeUbo_` offsets unchanged) | `whole_array_copy_of_padded_uniform_validates_and_keeps_layout` (naga validation, MSL + GLSL output, MSL offsets vs `Uniforms`) | +| lib/allocator + terminal/c/osc | `osc::Parser>`: the C parser stores `CByteStorage` inline in the caller-allocated `OscParser` (no `Box`). `Command<'a>` payloads are `Cow<'a,[u8]>` borrowed from parser storage (Zig slices), per-command parsers no longer copy; `Parser::end` returns `Command<'_>`, the VT stream uses `into_owned()` (same copy count as before). Only the small color request lists (OSC 4/5/10-19/21/104-119) still use the global allocator; they are not exposed through C and dropped before `ghostty_osc_end` returns | Rust: `end_makes_no_global_allocations` (counting `#[global_allocator]` + counting C allocator: 0 global allocs, exactly 1 caller alloc only for the title); C: new section 6 of `alloc_test.c` (allocator frozen after creation, 9 command kinds still parse, title = 1 alloc) | +| crash (aarch64 compile) | `fpregs` only on x86/x86_64; aarch64 walks the `_aarch64_ctx` extension chain in `__reserved` for the `fpsimd_context` | `minidump::tests::*` (pure `fpsimd_offset`), `cargo check --target aarch64-unknown-linux-gnu -p ghostty-crash --all-targets` | +| crash (Darwin) | Signal handler records siginfo (signo/code/fault address), full thread state from the `ucontext` (x86_64/arm64 registers) and a frame-pointer stack walk (`mach_vm_read_overwrite`, fault-safe) in the event (`exception.stacktrace.frames/registers`, `mechanism.meta.signal`). A real minidump needs a Mach exception-port handler thread (breakpad design) which cannot be validated without macOS, so it is **not** done | `native::tests::*` (platform independent frame walk + JSON schema); Darwin glue verified with `just check-macos` for both targets | +| tripwire (selectionString) | `Screen::try_selection_string_map` with `selection_string_tw::check(CopyMap)` and `SelectionStringError`; `selection_string_map` wraps it | `screen_selection_string_map_allocation_failure_cleanup` | +| libghostty-vt soname | `build.rs` emits `-soname,libghostty-vt.so.` (macOS install_name `@rpath/libghostty-vt..dylib`); `VT_VERSION` in build.rs is the single source, `install-lib.sh` reads it and only verifies the soname (no patchelf). The vt-capi C test harness installs `libghostty-vt.so.` next to the unversioned name (otherwise the loader would pick a stale system copy) | `tests/soname.rs`, `build.rs` unit test | + +Gate: `cargo test --workspace` (3342 passed, 0 failed), `cargo clippy --workspace --all-targets -- -D warnings`, `cargo +nightly fmt --all`, `just check-macos`, `just check-macos x86_64-apple-darwin`, `just check-windows`, `just check-wasm`, aarch64-linux `ghostty-crash` check: all clean. + +Remaining caveats: the "open" screen-file action passes the encoded path as the URL string; flatpak env (`a{ss}`) stays UTF-8 only by D-Bus type; the minidump writer occasionally reports `ThreadList` failures in the sandbox and falls back to the event-only envelope (pre-existing). diff --git a/scripts/install-lib.sh b/scripts/install-lib.sh index 22aba13..4e7be6a 100755 --- a/scripts/install-lib.sh +++ b/scripts/install-lib.sh @@ -10,7 +10,7 @@ # `ghostty`: libghostty only (Zig: `-Dapp-runtime=none`, non-Darwin); # `all` (default): both. # -# libghostty-vt (version 0.1.0): +# libghostty-vt (version from crates/ghostty-vt-capi/build.rs, 0.1.0): # lib/libghostty-vt.so.0.1.0 (+ libghostty-vt.so.0, libghostty-vt.so symlinks; # macOS: libghostty-vt.0.1.0.dylib, libghostty-vt.0.dylib, libghostty-vt.dylib) # include/ghostty/vt.h and include/ghostty/vt/**.h @@ -29,7 +29,9 @@ dir=$profile [ "$profile" = dev ] && dir=debug tdir=${CARGO_TARGET_DIR:-target}/$dir cargo_args=(--profile "$profile") -vt_version=0.1.0 +# Single source of truth: the build script that sets the soname. +vt_version=$(sed -n 's/^const VT_VERSION: &str = "\(.*\)";$/\1/p' crates/ghostty-vt-capi/build.rs) +[ -n "$vt_version" ] || { echo "cannot read VT_VERSION from build.rs" >&2; exit 1; } vt_major=${vt_version%%.*} case $(uname -s) in @@ -44,20 +46,18 @@ install_vt() { if [ "$dylib" = so ]; then local real="libghostty-vt.so.$vt_version" install -m755 "$src" "$dest/lib/$real" - # The cargo-built soname is the unversioned name; Zig's is `.so.0`. - if command -v patchelf >/dev/null 2>&1; then - patchelf --set-soname "libghostty-vt.so.$vt_major" "$dest/lib/$real" - else - echo "patchelf not found: soname stays libghostty-vt.so" >&2 + # The soname (`libghostty-vt.so.$vt_major`) is set by build.rs at link + # time; verify instead of patching it after the fact. + if command -v readelf >/dev/null 2>&1 && + ! readelf -d "$dest/lib/$real" | grep -q "Library soname: \[libghostty-vt.so.$vt_major\]"; then + echo "unexpected soname in $real" >&2 + exit 1 fi ln -sf "$real" "$dest/lib/libghostty-vt.so.$vt_major" ln -sf "libghostty-vt.so.$vt_major" "$dest/lib/libghostty-vt.so" else local real="libghostty-vt.$vt_version.dylib" install -m755 "$src" "$dest/lib/$real" - if command -v install_name_tool >/dev/null 2>&1; then - install_name_tool -id "@rpath/libghostty-vt.$vt_major.dylib" "$dest/lib/$real" - fi ln -sf "$real" "$dest/lib/libghostty-vt.$vt_major.dylib" ln -sf "libghostty-vt.$vt_major.dylib" "$dest/lib/libghostty-vt.dylib" fi