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-<epoch> 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.
This commit is contained in:
funman300
2026-08-23 02:06:38 +00:00
parent 9ba88c79fc
commit 561e666dc3
+95 -4
View File
@@ -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<Self, serde_json::Error> {
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::<LauncherConfig>(&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 {