diff --git a/CHANGELOG.md b/CHANGELOG.md index 3da0235d..3a7b3735 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A config file saved with a UTF-8 BOM no longer wipes your settings** — every + config-dir file is read by a loader that treats any parse error as "there is + no file", falling back to defaults. `serde_json` rejects the U+FEFF a BOM puts + before the opening brace, so a BOM didn't report a broken config — it reported + an absent one, and tty7 came up on defaults with nothing in the log to explain + it. Windows makes this easy to hit by accident: PowerShell's `>`, `Out-File` + and `Set-Content -Encoding utf8` all write a BOM, so editing `config.json` + from a shell was enough to lose every setting. `config.json`, `session.json` + (which lost every workspace the same way) and hand-authored `themes/*.yaml` + now skip a leading BOM. + - **New tabs and splits open in the right directory even when the shell can't be instrumented** ([#187](https://github.com/l0ng-ai/tty7/issues/187)) — a pane learned its directory from `OSC 7`, which only shells tty7 injects its diff --git a/src/core/config.rs b/src/core/config.rs index 01efaed6..32e2fe79 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -679,7 +679,7 @@ impl Config { // Missing/unreadable config is the common case — start with defaults. return Config::default(); }; - match serde_json::from_str::(&text) { + match serde_json::from_str::(strip_bom(&text)) { Ok(mut cfg) => { cfg.sanitize(); cfg @@ -824,6 +824,21 @@ pub fn config_path(file: &str) -> Option { Some(config_dir()?.join(file)) } +/// Drop a leading UTF-8 BOM so a hand-edited config still parses. +/// +/// `serde_json` rejects U+FEFF before the opening brace, and every config-dir +/// file is read by a loader that treats *any* parse error as "there is no +/// config" — so a BOM doesn't surface as an error, it silently resets the +/// user's settings. Windows makes that easy to hit by accident: PowerShell's +/// `>`, `Out-File` and `Set-Content -Encoding utf8` all write one, so a quick +/// `... | Set-Content config.json` is enough to lose every setting. +/// +/// `read_to_string` decodes the BOM to the single char U+FEFF, so this strips +/// the char rather than the three raw bytes. +pub fn strip_bom(text: &str) -> &str { + text.strip_prefix('\u{FEFF}').unwrap_or(text) +} + /// Write `bytes` to `path` atomically: write to a sibling temp file, fsync, then /// rename over the target. A crash/power-loss mid-write then leaves either the /// old file or the new one intact — never a truncated/half-written file that @@ -1417,8 +1432,19 @@ mod tests { set_config_dir(dir); } + /// Serialize the tests that write the shared `config.json`: they all resolve + /// the same pinned path, so without this they clobber each other's file. + static CONFIG_FILE: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn lock_config_file() -> std::sync::MutexGuard<'static, ()> { + // A poisoned lock only means another test failed mid-sequence; every + // holder rewrites the file from scratch, so the state is still sound. + CONFIG_FILE.lock().unwrap_or_else(|e| e.into_inner()) + } + #[test] fn save_load_and_shell_command_round_trip_through_disk() { + let _guard = lock_config_file(); pin_config_dir(); // Persist a config with a non-default shell + font + an SSH profile, then // read it back. @@ -1457,6 +1483,41 @@ mod tests { assert_eq!(args, vec!["-l".to_string()]); } + #[test] + fn a_utf8_bom_does_not_silently_reset_the_config() { + let _guard = lock_config_file(); + pin_config_dir(); + let path = Config::path().expect("pinned config dir"); + // Exactly what PowerShell's `>`, `Out-File` and `Set-Content -Encoding + // utf8` leave behind. + let text = "\u{FEFF}{\"font_size\": 21.0, \"restore_session\": false}"; + write_atomic(&path, text.as_bytes()).unwrap(); + + // The failure this guards is silent by construction: `load` turns *any* + // parse error into defaults, so a BOM didn't report a bad config — it + // reported no config, and the user's settings appeared to vanish. + let loaded = Config::load(); + assert_eq!(loaded.font_size, 21.0); + assert!(!loaded.restore_session); + + let _ = std::fs::remove_file(&path); + } + + #[test] + fn strip_bom_only_removes_a_leading_marker() { + assert_eq!(strip_bom("{}"), "{}"); + assert_eq!(strip_bom("\u{FEFF}{}"), "{}"); + // Only the first U+FEFF is a marker. A second one is content, and + // content that happens to be a BOM is still invalid JSON — stripping + // it too would be guessing at a file we can't rescue. + assert_eq!(strip_bom("\u{FEFF}\u{FEFF}{}"), "\u{FEFF}{}"); + // A BOM *inside* the document is data (U+FEFF is a legal string char), + // so it must survive untouched. + let inner = "{\"tab_title\":\"\u{FEFF}\"}"; + assert_eq!(strip_bom(inner), inner); + assert_eq!(strip_bom(""), ""); + } + #[test] fn ssh_profiles_default_empty_and_parse_from_json() { // Absent key → empty (a config predating profiles still loads). diff --git a/src/core/session.rs b/src/core/session.rs index 4e426489..ff971b35 100644 --- a/src/core/session.rs +++ b/src/core/session.rs @@ -300,7 +300,7 @@ impl Workspaces { /// `{active, tabs}` session, which migrates to a single open workspace so /// upgrading users keep their tabs (and their attached daemon panes). pub fn decode(text: &str) -> Result { - let value: serde_json::Value = serde_json::from_str(text)?; + let value: serde_json::Value = serde_json::from_str(crate::core::config::strip_bom(text))?; if value.get("workspaces").is_some() { return serde_json::from_value(value); } @@ -759,6 +759,25 @@ mod tests { )); } + #[test] + fn a_utf8_bom_does_not_discard_the_session() { + // `Session::load` treats a parse error as "no session", so a BOM on a + // hand-edited `session.json` doesn't warn — it drops every workspace + // and opens on the home page as if nothing had been saved. + // Legacy `{active, tabs}` shape, so this also covers the migration path. + let decoded = Workspaces::decode( + "\u{FEFF}{\"active\": 0, \"tabs\": [{\"pane\": {\"Leaf\": {\"cwd\": \"/work\"}}}]}", + ) + .expect("a BOM'd session still decodes"); + let tabs = &decoded + .workspaces + .first() + .expect("migrated workspace") + .session + .tabs; + assert_eq!(tabs.len(), 1); + } + #[test] fn session_defaults_fill_missing_fields() { // An empty object → default (active 0, no tabs). diff --git a/src/ui/presets.rs b/src/ui/presets.rs index 3f951cf2..80e94963 100644 --- a/src/ui/presets.rs +++ b/src/ui/presets.rs @@ -942,7 +942,8 @@ fn default_image_opacity() -> f32 { fn load_yaml_theme(path: &std::path::Path) -> Result { let text = std::fs::read_to_string(path).map_err(|e| e.to_string())?; - let file: ThemeFile = serde_yaml::from_str(&text).map_err(|e| e.to_string())?; + let file: ThemeFile = + serde_yaml::from_str(crate::core::config::strip_bom(&text)).map_err(|e| e.to_string())?; let (id, derived_name) = id_and_name(path); let background = file.background.into_fill()?;