From 561e666dc35c876b56b036787cad0ad22075b2c9 Mon Sep 17 00:00:00 2001 From: funman300 Date: Sun, 23 Aug 2026 02:06:38 +0000 Subject: [PATCH] config: never let an unreadable config.json become production defaults load() did: read_to_string(&path).ok() .and_then(|s| serde_json::from_str(&s).ok()) .unwrap_or_default() so ANY parse failure silently produced compiled defaults -- blaze_main 42130 and account_sync 8099, both PRODUCTION -- with an empty game_profile, and the next save() wrote that over the operator's real settings. The launcher then could not start the game and was pointed at the live service. Observed 2026-08-23 from nothing worse than a UTF-8 BOM: PowerShell 5.1's Set-Content -Encoding UTF8 prepends EF BB BF and serde_json rejects it. A staging config (42327/42330/8299) was destroyed and replaced with production ports without a word. Two changes: - parse_json() strips a leading BOM, since Windows editors and PowerShell both emit one. Split out from load() so it is testable without touching the real config path. - A file that EXISTS but does not parse is no longer treated like a missing one. It is renamed to config.json.corrupt- and the error is reported naming the production risk, so defaults can never overwrite a recoverable config. A missing file still yields defaults: that is genuine first-run. Tests cover the exact incident (BOM-prefixed config keeps 42327/42330/8299 and does NOT fall back to 42130/8099) with a precondition asserting raw serde_json really does reject the BOM, so the guard cannot rot into a tautology. --- src/config.rs | 99 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index 87a3628..8ee10c8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -274,12 +274,57 @@ impl LauncherConfig { .join("config.json") } + /// Parse a config body, tolerating a leading UTF-8 BOM. + /// + /// Windows text editors and PowerShell's `Set-Content -Encoding UTF8` both + /// prepend `EF BB BF`, and `serde_json` rejects it. Kept separate from + /// [`Self::load`] so the BOM behaviour is testable without touching the + /// user's real config path. + pub fn parse_json(raw: &str) -> Result { + serde_json::from_str(raw.trim_start_matches('\u{feff}')) + } + + /// Load the saved config. + /// + /// A MISSING file is first-run and correctly yields defaults. A file that + /// exists but does not parse is NOT: silently returning defaults there means + /// the launcher comes up pointing at the **production** ports + /// (`blaze_main` 42130, `account_sync` 8099) with an empty `game_profile`, + /// and the next [`Self::save`] writes that over the user's real settings — + /// losing the configuration and silently retargeting the game. That happened + /// on 2026-08-23 from nothing worse than a BOM. + /// + /// So an unparseable config is quarantined rather than overwritten: it is + /// renamed next to itself and the error is reported, leaving the operator + /// something to recover from. pub fn load() -> Self { let path = Self::config_path(); - std::fs::read_to_string(&path) - .ok() - .and_then(|s| serde_json::from_str(&s).ok()) - .unwrap_or_default() + let Ok(raw) = std::fs::read_to_string(&path) else { + return Self::default(); + }; + match Self::parse_json(&raw) { + Ok(cfg) => cfg, + Err(e) => { + let stamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let quarantine = path.with_file_name(format!("config.json.corrupt-{stamp}")); + let moved = std::fs::rename(&path, &quarantine).is_ok(); + eprintln!( + "openfut-launcher: {} is not valid JSON ({e}). Falling back to defaults, \ + which point at the PRODUCTION ports — check the server settings before \ + launching.{}", + path.display(), + if moved { + format!(" Previous file kept at {}.", quarantine.display()) + } else { + String::new() + } + ); + Self::default() + } + } } pub fn save(&self) { @@ -517,6 +562,52 @@ mod tests { assert!(c.ea_hostnames.is_empty()); } + /// Regression, 2026-08-23: a config written by PowerShell's + /// `Set-Content -Encoding UTF8` carries a UTF-8 BOM. `serde_json` rejected + /// it, `load()` silently returned defaults, and the next `save()` wrote + /// those defaults over the operator's real settings — replacing the STAGING + /// ports with the PRODUCTION ones and emptying `game_profile`, so the + /// launcher could no longer start the game and would have pointed it at the + /// live service. Parsing must tolerate the BOM. + #[test] + fn a_bom_prefixed_config_still_parses_and_keeps_its_ports() { + let body = r#"{ + "core_binary":"","bridge_binary":"","core_database_url":"", + "core_data_dir":"","core_listen_addr":"","bridge_listen_addr":"", + "bridge_captures_dir":"","bridge_core_url":"","bridge_tls_enabled":true, + "hook_dll_path":"","fifa_game_dir":"C:\\FIFA 17", + "openfut_server_host":"10.10.0.120", + "openfut_blaze_redirector_port":42327, + "openfut_blaze_main_port":42330, + "openfut_account_sync_port":8299 + }"#; + let with_bom = format!("\u{feff}{body}"); + + assert!( + serde_json::from_str::(&with_bom).is_err(), + "precondition: raw serde_json must reject the BOM, else this guards nothing" + ); + + let c = LauncherConfig::parse_json(&with_bom).expect("BOM must be tolerated"); + assert_eq!(c.openfut_blaze_redirector_port, 42327); + assert_eq!( + c.openfut_blaze_main_port, 42330, + "must NOT fall back to 42130" + ); + assert_eq!( + c.openfut_account_sync_port, 8299, + "must NOT fall back to 8099" + ); + assert_eq!(c.fifa_game_dir, "C:\\FIFA 17"); + } + + /// Genuinely corrupt JSON must stay an error so `load()` quarantines the + /// file instead of overwriting it with defaults. + #[test] + fn a_corrupt_config_is_an_error_not_silent_defaults() { + assert!(LauncherConfig::parse_json("{not json").is_err()); + } + #[test] fn a_configured_profile_satisfies_launch_without_a_shell_command() { let mut c = LauncherConfig {