mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(config): don't let a UTF-8 BOM silently reset every setting
Every config-dir file is read by a loader that treats any parse error as "there is no file" and falls back to defaults. serde_json rejects the U+FEFF a BOM puts before the opening brace, so a BOM never surfaced as a broken config — it surfaced as an absent one, and the app booted on defaults with nothing to explain it. Windows makes that 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. Strip a leading BOM in the three loaders whose files people hand-edit: config.json, session.json (which dropped every workspace the same way), and themes/*.yaml. read_to_string decodes the marker to one U+FEFF char, so this strips the char, not the three raw bytes — and only the first one, since a second is content the parser should still reject. window.json and update.json are left alone: they are machine-written state a relaunch rebuilds, never hand-edited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WCb8ZDmvdA5xbVtvs647tD
This commit is contained in:
@@ -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
|
||||
|
||||
+62
-1
@@ -679,7 +679,7 @@ impl Config {
|
||||
// Missing/unreadable config is the common case — start with defaults.
|
||||
return Config::default();
|
||||
};
|
||||
match serde_json::from_str::<Config>(&text) {
|
||||
match serde_json::from_str::<Config>(strip_bom(&text)) {
|
||||
Ok(mut cfg) => {
|
||||
cfg.sanitize();
|
||||
cfg
|
||||
@@ -824,6 +824,21 @@ pub fn config_path(file: &str) -> Option<PathBuf> {
|
||||
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).
|
||||
|
||||
+20
-1
@@ -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<Self, serde_json::Error> {
|
||||
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).
|
||||
|
||||
+2
-1
@@ -942,7 +942,8 @@ fn default_image_opacity() -> f32 {
|
||||
|
||||
fn load_yaml_theme(path: &std::path::Path) -> Result<Theme, String> {
|
||||
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()?;
|
||||
|
||||
Reference in New Issue
Block a user