diff --git a/src/config/io.rs b/src/config/io.rs index 898cf077..110d431e 100644 --- a/src/config/io.rs +++ b/src/config/io.rs @@ -93,9 +93,35 @@ fn platform_state_dir() -> PathBuf { } } -fn read_optional_config(path: &Path) -> std::io::Result> { +/// Normalize UTF-8 byte-order marks in config text. +/// +/// TOML tolerates a single BOM at the very start of the document, but a BOM at +/// the start of a later line makes the parser reject the whole file. A +/// line-oriented edit can displace a leading BOM into the middle of the file, +/// so drop line-start BOMs that the TOML parser actually rejects. A U+FEFF that +/// is valid string data is kept, because its parse error would not point at it. +fn normalize_utf8_bom(content: &str) -> String { + let content = content.strip_prefix('\u{feff}').unwrap_or(content); + if !content.contains('\u{feff}') { + return content.to_owned(); + } + + let mut normalized = content.to_owned(); + while let Err(error) = normalized.parse::() { + let Some(span) = error.span() else { + break; + }; + if normalized.get(span.clone()) != Some("\u{feff}") { + break; + } + normalized.replace_range(span, ""); + } + normalized +} + +pub(super) fn read_optional_config(path: &Path) -> std::io::Result> { match std::fs::read_to_string(path) { - Ok(content) => Ok(Some(content)), + Ok(content) => Ok(Some(normalize_utf8_bom(&content))), Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), Err(err) => Err(err), } @@ -1108,4 +1134,68 @@ mouse_capture = false assert!(!removed); assert_eq!(updated, content); } + + #[test] + fn normalize_utf8_bom_removes_a_leading_bom() { + let content = "\u{feff}onboarding = false\n[terminal]\n"; + assert_eq!( + normalize_utf8_bom(content), + "onboarding = false\n[terminal]\n" + ); + } + + #[test] + fn normalize_utf8_bom_recovers_from_a_displaced_mid_file_bom() { + let content = "onboarding = false\n\u{feff}[terminal]\ndefault_shell = \"pwsh.exe\"\n"; + let normalized = normalize_utf8_bom(content); + assert_eq!( + normalized, + "onboarding = false\n[terminal]\ndefault_shell = \"pwsh.exe\"\n" + ); + assert!(normalized.parse::().is_ok()); + } + + #[test] + fn normalize_utf8_bom_preserves_boms_in_multiline_basic_strings() { + let content = "[theme]\nname = \"\"\"\nfirst\n\u{feff}second\n\"\"\"\n"; + assert!(content.parse::().is_ok()); + assert_eq!(normalize_utf8_bom(content), content); + } + + #[test] + fn normalize_utf8_bom_preserves_boms_in_multiline_literal_strings() { + let content = "[theme]\nname = '''\nfirst\n\u{feff}second\n'''\n"; + assert!(content.parse::().is_ok()); + assert_eq!(normalize_utf8_bom(content), content); + } + + #[test] + fn normalize_utf8_bom_preserves_string_boms_despite_other_errors() { + let content = "[theme]\nname = \"\"\"\nfirst\n\u{feff}second\n\"\"\"\nbroken = \n"; + assert!(content.parse::().is_err()); + assert_eq!(normalize_utf8_bom(content), content); + } + + #[test] + fn config_load_recovers_from_a_mid_file_bom() { + let _guard = crate::config::test_config_env_lock().lock().unwrap(); + let path = std::env::temp_dir().join(format!( + "herdr-config-mid-file-bom-{}.toml", + std::process::id() + )); + std::fs::write( + &path, + b"onboarding = false\n\xEF\xBB\xBF[terminal]\ndefault_shell = \"pwsh.exe\"\n", + ) + .unwrap(); + std::env::set_var(CONFIG_PATH_ENV_VAR, &path); + + let loaded = Config::load(); + + std::env::remove_var(CONFIG_PATH_ENV_VAR); + let _ = std::fs::remove_file(path); + + assert!(loaded.diagnostics.is_empty(), "{:?}", loaded.diagnostics); + assert_eq!(loaded.config.terminal.default_shell, "pwsh.exe"); + } } diff --git a/src/config/write.rs b/src/config/write.rs index 75244583..f34ec2f6 100644 --- a/src/config/write.rs +++ b/src/config/write.rs @@ -55,9 +55,9 @@ pub(crate) fn update_file_at( std::fs::create_dir_all(parent) .map_err(|error| format!("failed to create config directory: {error}"))?; } - let content = match std::fs::read_to_string(path) { - Ok(content) => content, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), + let content = match super::io::read_optional_config(path) { + Ok(Some(content)) => content, + Ok(None) => String::new(), Err(error) => { return Err(format!( "failed to read config before saving {description}: {error}" @@ -73,3 +73,37 @@ pub(crate) fn write_edit(edit: ConfigEdit<'_>) -> Result<(), String> { edit.apply(content) }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn update_file_at_does_not_move_a_leading_bom_into_the_file() { + let dir = std::env::temp_dir().join(format!("herdr-config-bom-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config.toml"); + std::fs::write( + &path, + b"\xEF\xBB\xBF[terminal]\ndefault_shell = \"pwsh.exe\"\n", + ) + .unwrap(); + + update_file_at(&path, "onboarding setting", |content| { + crate::config::upsert_top_level_bool(content, "onboarding", false) + }) + .unwrap(); + + let written = std::fs::read_to_string(&path).unwrap(); + let _ = std::fs::remove_dir_all(dir); + + assert!( + !written.contains('\u{feff}'), + "unexpected BOM in {written:?}" + ); + assert!( + toml::from_str::(&written).is_ok(), + "written config is not valid TOML: {written:?}" + ); + } +}