fix: systematic byte fidelity for config strings and RV7 gaps
Resolves the 9 gap groups in docs/rewrite/notes/review-rv7-codex.md and
audits every string-typed config field against its consumers
(field -> consumers -> decode status table in final-Z9.md):
- Windows EnvMap keys use a port of Zig's nls.zig uppercase table
(upcaseW); Windows locale init follows Zig's chain (setlocale +
_putenv_s); malformed WTF-8 now fails process launch conversions like
Zig instead of substituting U+FFFD; kitty file paths convert WTF-8.
- bash --rcfile/--init-file, configured TERM and inherited GTK LANG
keep their original bytes.
- C API clipboard completion and ghostty_surface_binding_action take
bytes; complete_clipboard_request is byte-typed through the core.
- text:/csi:/esc: bindings, enquiry-response, input = path:, embedded
initial_input, search needles, titles, link regexes, clipboard
codepoint map, command palette text and GTK font/namespace values
decode the escape encoding before use.
Remaining by design: D-Bus a{ss} values and +new-window arguments are
UTF-8 by D-Bus type. C API key/preedit text follows separately.
Test Plan:
- cargo test --workspace
- just check-macos (both), just check-windows, just check-wasm
This commit is contained in:
1 parent
1ff5db0527
commit
e6564cb838
40 files changed
+986
-167
No files matched your search
@@ -50,6 +50,8 @@ use crate::windows_args::{self, create_windows_env_block, windows_create_command
|
||||
/// Errors from [`Command::start`].
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum StartError {
|
||||
#[error("malformed wtf-8 in a path, argument or environment value")]
|
||||
InvalidWtf8(#[from] ghostty_os::osbytes::InvalidWtf8),
|
||||
#[error("string or environment is not valid unicode")]
|
||||
Encode(#[from] windows_args::EncodeError),
|
||||
#[error("windows error: {0}")]
|
||||
@@ -131,16 +133,22 @@ impl Command {
|
||||
pub fn start(&mut self) -> Result<(), StartError> {
|
||||
// `path` and `args` are the config command's argv strings (encoded
|
||||
// OS strings); decode them to WTF-16 for the Windows API.
|
||||
let application_w = EncodedOs::from_encoded(&self.path).to_wide_nul();
|
||||
let cwd_w = self.cwd.as_ref().map(EncodedOs::to_wide_nul);
|
||||
// Like Zig's fallible `utf8ToUtf16Le*`, malformed input fails the
|
||||
// launch instead of selecting a U+FFFD-substituted path or argument.
|
||||
let application_w = EncodedOs::from_encoded(&self.path).try_to_wide_nul()?;
|
||||
let cwd_w = self
|
||||
.cwd
|
||||
.as_ref()
|
||||
.map(EncodedOs::try_to_wide_nul)
|
||||
.transpose()?;
|
||||
let mut command_line_w = if self.args.is_empty() {
|
||||
None
|
||||
} else {
|
||||
let wide: Vec<Vec<u16>> = self
|
||||
.args
|
||||
.iter()
|
||||
.map(|a| EncodedOs::from_encoded(a).to_wide())
|
||||
.collect();
|
||||
.map(|a| EncodedOs::from_encoded(a).try_to_wide())
|
||||
.collect::<Result<_, _>>()?;
|
||||
let mut line = windows_create_command_line_wide(&wide);
|
||||
line.push(0);
|
||||
Some(line)
|
||||
|
||||
@@ -109,7 +109,7 @@ pub struct Config {
|
||||
pub cursor_blink: Option<bool>,
|
||||
pub working_directory: Option<EncodedOs>,
|
||||
pub resources_dir: Option<EncodedOs>,
|
||||
pub term: String,
|
||||
pub term: EncodedOs,
|
||||
|
||||
pub rt_pre_exec_info: RtPreExecInfo,
|
||||
pub rt_post_fork_info: RtPostForkInfo,
|
||||
@@ -124,7 +124,7 @@ pub struct Config {
|
||||
|
||||
impl Config {
|
||||
/// A config with Zig-struct-style defaults for everything optional.
|
||||
pub fn new(env: EnvMap, term: impl Into<String>) -> Config {
|
||||
pub fn new(env: EnvMap, term: EncodedOs) -> Config {
|
||||
Config {
|
||||
command: None,
|
||||
env,
|
||||
@@ -134,7 +134,7 @@ impl Config {
|
||||
cursor_blink: None,
|
||||
working_directory: None,
|
||||
resources_dir: None,
|
||||
term: term.into(),
|
||||
term,
|
||||
rt_pre_exec_info: RtPreExecInfo::default(),
|
||||
rt_post_fork_info: RtPostForkInfo::default(),
|
||||
create_scope: None,
|
||||
@@ -801,7 +801,8 @@ impl Subprocess {
|
||||
// For now, we just look up a bundled dir but in the future we should
|
||||
// also load the terminfo database and look for it.
|
||||
if let Some(base) = &cfg.resources_dir {
|
||||
env.insert("TERM", cfg.term.clone());
|
||||
// The configured TERM is an encoded OS string: export its bytes.
|
||||
env.insert("TERM", cfg.term.to_os());
|
||||
env.insert("COLORTERM", "truecolor");
|
||||
|
||||
// Assume that the resources directory is adjacent to the terminfo
|
||||
@@ -1586,6 +1587,18 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn configured_term_is_exported_decoded() {
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
let dir = ghostty_os::TempDir::init().unwrap();
|
||||
for raw in [&b"x\xfft"[..], "t\u{fdd0}".as_bytes()] {
|
||||
let mut cfg = Config::new(EnvMap::new(), EncodedOs::from_bytes(raw));
|
||||
cfg.resources_dir = Some(EncodedOs::from_path(&dir.path()));
|
||||
let exec = Exec::new(cfg).unwrap();
|
||||
assert_eq!(exec.env().unwrap().get("TERM").unwrap().as_bytes(), raw);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_command_darwin_hushlogin_with_non_utf8_home() {
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
|
||||
@@ -79,6 +79,10 @@ pub enum Error {
|
||||
Start(#[from] command::StartError),
|
||||
#[error(transparent)]
|
||||
ShellIntegration(#[from] shell_integration::Error),
|
||||
/// A configured path, argument or environment value is not well-formed
|
||||
/// WTF-8 (Zig's process launch conversions fail on such input).
|
||||
#[error(transparent)]
|
||||
InvalidWtf8(#[from] ghostty_os::osbytes::InvalidWtf8),
|
||||
#[error("io error: {0}")]
|
||||
Io(#[from] io::Error),
|
||||
#[error("failed to get WINDIR, cannot run shell command")]
|
||||
@@ -102,12 +106,12 @@ pub struct Config {
|
||||
pub cursor_blink: Option<bool>,
|
||||
pub working_directory: Option<EncodedOs>,
|
||||
pub resources_dir: Option<EncodedOs>,
|
||||
pub term: String,
|
||||
pub term: EncodedOs,
|
||||
}
|
||||
|
||||
impl Config {
|
||||
/// A config with Zig-struct-style defaults for everything optional.
|
||||
pub fn new(env: EnvMap, term: impl Into<String>) -> Config {
|
||||
pub fn new(env: EnvMap, term: EncodedOs) -> Config {
|
||||
Config {
|
||||
command: None,
|
||||
env,
|
||||
@@ -117,7 +121,7 @@ impl Config {
|
||||
cursor_blink: None,
|
||||
working_directory: None,
|
||||
resources_dir: None,
|
||||
term: term.into(),
|
||||
term,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -585,13 +589,20 @@ fn write_thread_main(handle: &OwnedHandle, rx: &Receiver<Vec<u8>>) {
|
||||
|
||||
/// Insert the config `env` overrides, decoding the encoded key/value text
|
||||
/// to OS strings (WTF-16 on Windows) like the Unix implementation.
|
||||
fn apply_env_overrides(env: &mut EnvMap, overrides: &RepeatableStringMap) {
|
||||
///
|
||||
/// Malformed WTF-8 fails (Zig's conversions for the environment block are
|
||||
/// fallible) instead of being substituted with U+FFFD.
|
||||
fn apply_env_overrides(
|
||||
env: &mut EnvMap,
|
||||
overrides: &RepeatableStringMap,
|
||||
) -> Result<(), ghostty_os::osbytes::InvalidWtf8> {
|
||||
for (k, v) in overrides.iter() {
|
||||
env.insert(
|
||||
ghostty_os::osbytes::config_str_to_os(k),
|
||||
ghostty_os::osbytes::config_str_to_os(v),
|
||||
EncodedOs::from_encoded(k).try_to_os()?,
|
||||
EncodedOs::from_encoded(v).try_to_os()?,
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct Subprocess {
|
||||
@@ -617,18 +628,20 @@ impl Subprocess {
|
||||
log::info!("found Ghostty resources dir: {}", dir.as_encoded());
|
||||
// The resources dir is an encoded OS string: decode it for the
|
||||
// child's environment.
|
||||
env.insert("GHOSTTY_RESOURCES_DIR", dir.to_os());
|
||||
env.insert("GHOSTTY_RESOURCES_DIR", dir.try_to_os()?);
|
||||
}
|
||||
|
||||
// Set our TERM var. This is a bit complicated because we want to use
|
||||
// the ghostty TERM value but we want to only do that if we have
|
||||
// ghostty in the TERMINFO database.
|
||||
if let Some(base) = &cfg.resources_dir {
|
||||
env.insert("TERM", cfg.term.clone());
|
||||
// The configured TERM is an encoded OS string: export its bytes.
|
||||
env.insert("TERM", cfg.term.try_to_os()?);
|
||||
env.insert("COLORTERM", "truecolor");
|
||||
|
||||
// Assume that the resources directory is adjacent to the terminfo
|
||||
// database
|
||||
base.try_to_os()?; // malformed WTF-8 fails like Zig
|
||||
env.insert("TERMINFO", terminfo_dir(base));
|
||||
} else {
|
||||
env.insert("TERM", "xterm-256color");
|
||||
@@ -698,7 +711,7 @@ impl Subprocess {
|
||||
};
|
||||
|
||||
// Add the environment variables that override any others.
|
||||
apply_env_overrides(&mut env, &cfg.env_override);
|
||||
apply_env_overrides(&mut env, &cfg.env_override)?;
|
||||
|
||||
// Build our args list. If we fail to find the shell, we'd still like
|
||||
// to try to run something so Ghostty can launch.
|
||||
@@ -720,7 +733,7 @@ impl Subprocess {
|
||||
|
||||
// Propagate the current working directory (CWD) to the shell.
|
||||
if let Some(pwd) = &cwd {
|
||||
env.insert("PWD", pwd.to_os());
|
||||
env.insert("PWD", pwd.try_to_os()?);
|
||||
}
|
||||
|
||||
Ok(Subprocess {
|
||||
@@ -948,7 +961,7 @@ mod tests {
|
||||
m.parse_cli(Some(&format!("A={lone}"))).unwrap();
|
||||
m.parse_cli(Some(&format!("{lit}=v"))).unwrap();
|
||||
let mut env = EnvMap::new();
|
||||
super::apply_env_overrides(&mut env, &m);
|
||||
super::apply_env_overrides(&mut env, &m).unwrap();
|
||||
assert_eq!(
|
||||
env.get("A").unwrap(),
|
||||
std::ffi::OsString::from_wide(&[0x61, 0xD800])
|
||||
@@ -956,6 +969,17 @@ mod tests {
|
||||
assert_eq!(env.get("K\u{fdd0}").unwrap(), "v");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_wtf8_env_overrides_fail() {
|
||||
use ghostty_config::cli_args::ConfigField;
|
||||
|
||||
let bad = EncodedOs::from_bytes(b"a\xff").into_encoded();
|
||||
let mut m = RepeatableStringMap::default();
|
||||
m.parse_cli(Some(&format!("A={bad}"))).unwrap();
|
||||
let mut env = EnvMap::new();
|
||||
assert!(super::apply_env_overrides(&mut env, &m).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn terminfo_dir_is_sibling_of_resources_and_lossless() {
|
||||
use std::os::windows::ffi::{OsStrExt, OsStringExt};
|
||||
|
||||
@@ -263,7 +263,9 @@ fn setup_bash(
|
||||
|
||||
env.insert("GHOSTTY_BASH_INJECT", inject);
|
||||
if let Some(v) = rcfile {
|
||||
env.insert("GHOSTTY_BASH_RCFILE", v);
|
||||
// The intercepted argument is an encoded OS string; the env var gets
|
||||
// its original bytes (Zig inserts the argument slice unchanged).
|
||||
env.insert("GHOSTTY_BASH_RCFILE", EncodedOs::from_encoded(v).to_os());
|
||||
}
|
||||
|
||||
// In POSIX mode, HISTFILE defaults to ~/.sh_history, so unless we're
|
||||
@@ -671,6 +673,24 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn bash_rcfile_keeps_original_bytes() {
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
|
||||
let res = TmpResourcesDir::new(Shell::Bash);
|
||||
for raw in [&b"/home/x/\xffrc"[..], "/home/x/\u{fdd0}rc".as_bytes()] {
|
||||
let mut env = EnvMap::new();
|
||||
let command = Command::Direct(vec![
|
||||
"bash".to_owned(),
|
||||
"--rcfile".to_owned(),
|
||||
EncodedOs::from_bytes(raw).into_encoded(),
|
||||
]);
|
||||
setup_bash(&command, &res.enc(), &mut env).unwrap();
|
||||
assert_eq!(env.get("GHOSTTY_BASH_RCFILE").unwrap().as_bytes(), raw);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bash_histfile() {
|
||||
let res = TmpResourcesDir::new(Shell::Bash);
|
||||
|
||||
@@ -348,7 +348,7 @@ impl TerminalStream {
|
||||
let st = &mut self.state;
|
||||
st.osc_color_report_format = config.osc_color_report_format;
|
||||
st.clipboard_write = config.clipboard_write;
|
||||
st.enquiry_response = config.enquiry_response.clone().into_bytes();
|
||||
st.enquiry_response.clone_from(&config.enquiry_response);
|
||||
st.default_cursor_style = config.cursor_style;
|
||||
st.default_cursor_blink = config.cursor_blink;
|
||||
|
||||
|
||||
@@ -108,7 +108,8 @@ pub struct DerivedConfig {
|
||||
pub background: Color,
|
||||
pub osc_color_report_format: OSCColorReportFormat,
|
||||
pub clipboard_write: ClipboardAccess,
|
||||
pub enquiry_response: String,
|
||||
/// The decoded bytes of `enquiry-response` (written to the pty as is).
|
||||
pub enquiry_response: Vec<u8>,
|
||||
pub conditional_state: conditional::State,
|
||||
}
|
||||
|
||||
@@ -142,7 +143,7 @@ impl DerivedConfig {
|
||||
background: config.background,
|
||||
osc_color_report_format: config.osc_color_report_format,
|
||||
clipboard_write: config.clipboard_write,
|
||||
enquiry_response: config.enquiry_response.clone(),
|
||||
enquiry_response: ghostty_os::osbytes::config_str_to_bytes(&config.enquiry_response),
|
||||
conditional_state: config.conditional_state,
|
||||
}
|
||||
}
|
||||
@@ -175,8 +176,9 @@ impl ThreadEnterState {
|
||||
match item {
|
||||
ReadableIO::Raw(v) => out.push(ghostty_os::osbytes::config_str_to_bytes(v)),
|
||||
ReadableIO::Path(path) => {
|
||||
let f = std::fs::File::open(path).map_err(|err| {
|
||||
log::warn!("failed to open input file={path} err={err}");
|
||||
// The path is an encoded OS string: decode it to open.
|
||||
let f = std::fs::File::open(path.to_path()).map_err(|err| {
|
||||
log::warn!("failed to open input file={} err={err}", path.as_encoded());
|
||||
Error::InputNotFound
|
||||
})?;
|
||||
let mut buf = Vec::new();
|
||||
@@ -419,7 +421,7 @@ impl Termio {
|
||||
renderer_wakeup: renderer_wakeup.clone(),
|
||||
default_cursor_style: config.cursor_style,
|
||||
default_cursor_blink: config.cursor_blink,
|
||||
enquiry_response: config.enquiry_response.clone().into_bytes(),
|
||||
enquiry_response: config.enquiry_response.clone(),
|
||||
osc_color_report_format: config.osc_color_report_format,
|
||||
clipboard_write: config.clipboard_write,
|
||||
});
|
||||
@@ -791,3 +793,26 @@ fn set_image_limits(term: &mut Terminal, limit: usize) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod derived_config_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn enquiry_response_is_decoded_to_bytes() {
|
||||
let mut cfg = Config {
|
||||
enquiry_response: ghostty_os::osbytes::EncodedOs::from_bytes(
|
||||
"a\u{fdd0}\u{ff}\u{e9}".as_bytes(),
|
||||
)
|
||||
.into_encoded(),
|
||||
..Config::default()
|
||||
};
|
||||
assert_eq!(
|
||||
DerivedConfig::new(&cfg).enquiry_response,
|
||||
"a\u{fdd0}\u{ff}\u{e9}".as_bytes()
|
||||
);
|
||||
let raw = b"x\xffy";
|
||||
cfg.enquiry_response = ghostty_os::osbytes::EncodedOs::from_bytes(raw).into_encoded();
|
||||
assert_eq!(DerivedConfig::new(&cfg).enquiry_response, raw);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user