refactor: type escaped OS strings and fix the final audit findings
Escaped config strings (ghostty_os::osbytes) kept reaching OS boundaries undecoded, and decoding was occasionally applied to plain text. The root cause was that both were plain `String`s. Now: - `EncodedOs` is a newtype (no Display/AsRef<Path>) that decodes only via to_os/to_path/to_bytes/to_wide; plain text enters via from_plain_text. OS helpers (env, home, xdg, passwd, resources dir, path expansion), config::Path, WorkingDirectory, theme/file_load/edit paths, terminal pwd, OpenUrl/Pwd actions and termio cwd use it, so the compiler forces decoding at every boundary. os::open takes &OsStr. Config output is unchanged. Fixes all rows of docs/rewrite/notes/review-rv5-codex.md (final-Z7.md): screen-file copy; regex/OSC 8 URLs no longer decoded (regression); lossless Windows WTF-8/WTF-16 for args, env and CreateProcessW; Windows resources dir/TERMINFO; color OSC request lists and X11 color lookup no longer allocate globally (C zero-allocation test extended); macOS Mach handler resumes all threads before KERN_FAILURE (hang regression) and SIGABRT minidumps go through the exception thread; plus open-config, background images, crash dir, ssh cache, exe dir/PATH, fontconfig filenames, file links, kitty TMPDIR, .hushlogin and benchmark paths. The typing also surfaced and fixed gtk-custom-css, bell audio, bash HISTFILE, OSC 52 writes and the C config getters. Test Plan: - cargo test --workspace (3376 passed) - just check-macos (both), just check-windows, just check-wasm - diff <(ghostty-rs +show-config --default --docs) \ <(ghostty +show-config --default --docs)
This commit is contained in:
1 parent
0f898d3e41
commit
8f3e206d9f
92 files changed
+2967
-992
No files matched your search
@@ -2,48 +2,61 @@
|
||||
//! the benchmark and synthetic CLIs).
|
||||
use std::{
|
||||
collections::HashMap,
|
||||
ffi::{OsStr, OsString},
|
||||
fs::File,
|
||||
io::{self, Read},
|
||||
path::{Path, PathBuf},
|
||||
str::FromStr,
|
||||
};
|
||||
|
||||
use ghostty_os::osbytes::{os_bytes, os_from_bytes};
|
||||
|
||||
/// Parsed `--key[=value]` arguments.
|
||||
#[derive(Debug, Default)]
|
||||
pub struct Args {
|
||||
map: HashMap<String, Option<String>>,
|
||||
// Values are OS strings: file names (`--data=...`) need not be UTF-8.
|
||||
map: HashMap<String, Option<OsString>>,
|
||||
}
|
||||
|
||||
/// The action name of a `+action` argument, if `arg` is one.
|
||||
#[must_use]
|
||||
pub fn action_name(arg: &OsStr) -> Option<String> {
|
||||
let bytes = os_bytes(arg);
|
||||
let name = bytes.strip_prefix(b"+")?;
|
||||
Some(String::from_utf8_lossy(name).into_owned())
|
||||
}
|
||||
|
||||
impl Args {
|
||||
/// Parse arguments (skipping `+action` words and the program name).
|
||||
pub fn parse<I: IntoIterator<Item = String>>(iter: I) -> Result<Args, String> {
|
||||
pub fn parse<I, S>(iter: I) -> Result<Args, String>
|
||||
where
|
||||
I: IntoIterator<Item = S>,
|
||||
S: Into<OsString>,
|
||||
{
|
||||
let mut map = HashMap::new();
|
||||
for arg in iter {
|
||||
if arg.starts_with('+') {
|
||||
let bytes = os_bytes(&arg.into());
|
||||
if bytes.starts_with(b"+") {
|
||||
continue;
|
||||
}
|
||||
let Some(rest) = arg.strip_prefix("--") else {
|
||||
return Err(format!("unexpected argument: {arg}"));
|
||||
let Some(rest) = bytes.strip_prefix(b"--") else {
|
||||
return Err(format!(
|
||||
"unexpected argument: {}",
|
||||
String::from_utf8_lossy(&bytes)
|
||||
));
|
||||
};
|
||||
match rest.split_once('=') {
|
||||
Some((k, v)) => map.insert(k.to_owned(), Some(v.to_owned())),
|
||||
None => map.insert(rest.to_owned(), None),
|
||||
let (key, value) = match rest.iter().position(|b| *b == b'=') {
|
||||
Some(i) => (&rest[..i], Some(os_from_bytes(rest[i + 1..].to_vec()))),
|
||||
None => (rest, None),
|
||||
};
|
||||
let key = String::from_utf8(key.to_vec())
|
||||
.map_err(|_| "option names must be valid UTF-8".to_owned())?;
|
||||
map.insert(key, value);
|
||||
}
|
||||
Ok(Args { map })
|
||||
}
|
||||
|
||||
/// Take a typed value, falling back to `default`.
|
||||
pub fn take<T: FromStr>(&mut self, key: &str, default: T) -> Result<T, String> {
|
||||
match self.map.remove(key) {
|
||||
None => Ok(default),
|
||||
Some(None) => Err(format!("option --{key} requires a value")),
|
||||
Some(Some(v)) => v
|
||||
.parse()
|
||||
.map_err(|_| format!("invalid value for --{key}: {v}")),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn take_opt_string(&mut self, key: &str) -> Result<Option<String>, String> {
|
||||
fn take_os(&mut self, key: &str) -> Result<Option<OsString>, String> {
|
||||
match self.map.remove(key) {
|
||||
None => Ok(None),
|
||||
Some(None) => Err(format!("option --{key} requires a value")),
|
||||
@@ -51,15 +64,44 @@ impl Args {
|
||||
}
|
||||
}
|
||||
|
||||
/// Take a typed value, falling back to `default`.
|
||||
pub fn take<T: FromStr>(&mut self, key: &str, default: T) -> Result<T, String> {
|
||||
match self.take_os(key)? {
|
||||
None => Ok(default),
|
||||
Some(v) => v
|
||||
.to_str()
|
||||
.and_then(|s| s.parse().ok())
|
||||
.ok_or_else(|| format!("invalid value for --{key}: {}", v.to_string_lossy())),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn take_opt_string(&mut self, key: &str) -> Result<Option<String>, String> {
|
||||
match self.take_os(key)? {
|
||||
None => Ok(None),
|
||||
Some(v) => v
|
||||
.into_string()
|
||||
.map(Some)
|
||||
.map_err(|v| format!("invalid value for --{key}: {}", v.to_string_lossy())),
|
||||
}
|
||||
}
|
||||
|
||||
/// Take a file path value (any bytes are allowed).
|
||||
pub fn take_opt_path(&mut self, key: &str) -> Result<Option<PathBuf>, String> {
|
||||
Ok(self.take_os(key)?.map(PathBuf::from))
|
||||
}
|
||||
|
||||
/// Take a boolean flag (`--key`, `--key=true`, `--key=false`).
|
||||
pub fn take_flag(&mut self, key: &str) -> Result<bool, String> {
|
||||
match self.map.remove(key) {
|
||||
None => Ok(false),
|
||||
Some(None) => Ok(true),
|
||||
Some(Some(v)) => match v.as_str() {
|
||||
Some(Some(v)) => match v.to_str().unwrap_or_default() {
|
||||
"true" | "t" | "1" | "yes" => Ok(true),
|
||||
"false" | "f" | "0" | "no" => Ok(false),
|
||||
_ => Err(format!("invalid value for --{key}: {v}")),
|
||||
_ => Err(format!(
|
||||
"invalid value for --{key}: {}",
|
||||
v.to_string_lossy()
|
||||
)),
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -75,10 +117,10 @@ impl Args {
|
||||
|
||||
/// Returns the data source for the path: `None` if unset, stdin for `-`,
|
||||
/// otherwise the opened file (mirrors `benchmark/options.zig`).
|
||||
pub fn data_file(path: Option<&str>) -> io::Result<Option<Box<dyn Read>>> {
|
||||
pub fn data_file(path: Option<&Path>) -> io::Result<Option<Box<dyn Read>>> {
|
||||
match path {
|
||||
None => Ok(None),
|
||||
Some("-") => Ok(Some(Box::new(io::stdin()))),
|
||||
Some(p) if p == Path::new("-") => Ok(Some(Box::new(io::stdin()))),
|
||||
Some(p) => Ok(Some(Box::new(File::open(p)?))),
|
||||
}
|
||||
}
|
||||
@@ -96,3 +138,25 @@ pub fn read_short(r: &mut dyn Read, buf: &mut [u8]) -> io::Result<usize> {
|
||||
}
|
||||
Ok(n)
|
||||
}
|
||||
|
||||
#[cfg(all(test, unix))]
|
||||
mod tests {
|
||||
use std::os::unix::ffi::{OsStrExt, OsStringExt};
|
||||
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn data_path_keeps_non_utf8_bytes() {
|
||||
let arg = OsString::from_vec(b"--data=/tmp/a\xffb".to_vec());
|
||||
let mut args = Args::parse([arg]).unwrap();
|
||||
let p = args.take_opt_path("data").unwrap().unwrap();
|
||||
assert_eq!(p.as_os_str().as_bytes(), b"/tmp/a\xffb");
|
||||
args.finish().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn action_name_extracts_plus_words() {
|
||||
assert_eq!(action_name(OsStr::new("+ascii")).as_deref(), Some("ascii"));
|
||||
assert_eq!(action_name(OsStr::new("--x")), None);
|
||||
}
|
||||
}
|
||||
@@ -40,12 +40,12 @@ fn warn(what: &str, e: &dyn std::fmt::Display) -> BenchmarkFailed {
|
||||
|
||||
/// Shared data-file handling: opened in setup, closed in teardown.
|
||||
struct Data {
|
||||
path: Option<String>,
|
||||
path: Option<std::path::PathBuf>,
|
||||
f: Option<Box<dyn Read>>,
|
||||
}
|
||||
|
||||
impl Data {
|
||||
fn new(path: Option<String>) -> Data {
|
||||
fn new(path: Option<std::path::PathBuf>) -> Data {
|
||||
Data { path, f: None }
|
||||
}
|
||||
|
||||
@@ -103,7 +103,7 @@ impl TerminalStream {
|
||||
pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
|
||||
let rows = args.take("terminal-rows", 80u16)?;
|
||||
let cols = args.take("terminal-cols", 120u16)?;
|
||||
let data = args.take_opt_string("data")?;
|
||||
let data = args.take_opt_path("data")?;
|
||||
let full = match args.take_opt_string("handler")?.as_deref() {
|
||||
None | Some("print") => false,
|
||||
Some("full") => true,
|
||||
@@ -160,7 +160,7 @@ pub struct TerminalParser {
|
||||
impl TerminalParser {
|
||||
pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
|
||||
Ok(TerminalParser {
|
||||
data: Data::new(args.take_opt_string("data")?),
|
||||
data: Data::new(args.take_opt_path("data")?),
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -196,7 +196,7 @@ pub struct OscParser {
|
||||
impl OscParser {
|
||||
pub fn create(args: &mut Args) -> std::result::Result<Self, String> {
|
||||
Ok(OscParser {
|
||||
data: Data::new(args.take_opt_string("data")?),
|
||||
data: Data::new(args.take_opt_path("data")?),
|
||||
parser: osc::Parser::init(true),
|
||||
})
|
||||
}
|
||||
@@ -267,7 +267,7 @@ impl CodepointWidth {
|
||||
Some(m) => return Err(format!("invalid value for --mode: {m}")),
|
||||
};
|
||||
Ok(CodepointWidth {
|
||||
data: Data::new(args.take_opt_string("data")?),
|
||||
data: Data::new(args.take_opt_path("data")?),
|
||||
mode,
|
||||
})
|
||||
}
|
||||
@@ -390,7 +390,7 @@ impl GraphemeBreak {
|
||||
Some(m) => return Err(format!("invalid value for --mode: {m}")),
|
||||
};
|
||||
Ok(GraphemeBreak {
|
||||
data: Data::new(args.take_opt_string("data")?),
|
||||
data: Data::new(args.take_opt_path("data")?),
|
||||
noop,
|
||||
})
|
||||
}
|
||||
@@ -479,7 +479,7 @@ impl IsSymbol {
|
||||
Some(m) => return Err(format!("invalid value for --mode: {m}")),
|
||||
};
|
||||
Ok(IsSymbol {
|
||||
data: Data::new(args.take_opt_string("data")?),
|
||||
data: Data::new(args.take_opt_path("data")?),
|
||||
mode,
|
||||
})
|
||||
}
|
||||
@@ -544,7 +544,7 @@ impl ScreenClone {
|
||||
};
|
||||
let rows = args.take("terminal-rows", 80u16)?;
|
||||
let cols = args.take("terminal-cols", 120u16)?;
|
||||
let data = args.take_opt_string("data")?;
|
||||
let data = args.take_opt_path("data")?;
|
||||
let terminal = Terminal::new(TermOptions::new(cols, rows)).map_err(|e| format!("{e:?}"))?;
|
||||
Ok(ScreenClone {
|
||||
data: Data::new(data),
|
||||
@@ -674,9 +674,13 @@ mod tests {
|
||||
|
||||
fn run(action: &str, mode: &str, data: &std::path::Path) -> Result {
|
||||
let mut args = Args::parse([
|
||||
format!("+{action}"),
|
||||
format!("--mode={mode}"),
|
||||
format!("--data={}", data.display()),
|
||||
std::ffi::OsString::from(format!("+{action}")),
|
||||
std::ffi::OsString::from(format!("--mode={mode}")),
|
||||
{
|
||||
let mut a = std::ffi::OsString::from("--data=");
|
||||
a.push(data);
|
||||
a
|
||||
},
|
||||
])
|
||||
.unwrap();
|
||||
match action {
|
||||
|
||||
@@ -11,14 +11,14 @@ use ghostty_bench::{
|
||||
};
|
||||
|
||||
fn run() -> Result<(), String> {
|
||||
let argv: Vec<String> = std::env::args().skip(1).collect();
|
||||
let argv: Vec<std::ffi::OsString> = std::env::args_os().skip(1).collect();
|
||||
let mut action = None;
|
||||
for a in &argv {
|
||||
if let Some(name) = a.strip_prefix('+') {
|
||||
if let Some(name) = ghostty_bench::args::action_name(a) {
|
||||
if action.is_some() {
|
||||
return Err("multiple actions specified".into());
|
||||
}
|
||||
action = Some(name.to_owned());
|
||||
action = Some(name);
|
||||
}
|
||||
}
|
||||
let action = action.ok_or("no action specified; available: ascii, osc, sgr, utf8")?;
|
||||
|
||||
@@ -4,15 +4,15 @@ use std::process::ExitCode;
|
||||
use ghostty_bench::{args::Args, benches};
|
||||
|
||||
fn main() -> ExitCode {
|
||||
let argv: Vec<String> = std::env::args().skip(1).collect();
|
||||
let argv: Vec<std::ffi::OsString> = std::env::args_os().skip(1).collect();
|
||||
let mut action = None;
|
||||
for a in &argv {
|
||||
if let Some(name) = a.strip_prefix('+') {
|
||||
if let Some(name) = ghostty_bench::args::action_name(a) {
|
||||
if action.is_some() {
|
||||
eprintln!("error: multiple actions specified");
|
||||
return ExitCode::FAILURE;
|
||||
}
|
||||
action = Some(name.to_owned());
|
||||
action = Some(name);
|
||||
}
|
||||
}
|
||||
let Some(action) = action else {
|
||||
|
||||
Reference in new issue
Block a user