From 8907f506c7040003ffc02c9ef56d82bcf3f86b5e Mon Sep 17 00:00:00 2001 From: ospab Date: Mon, 24 Aug 2026 15:40:07 +0300 Subject: [PATCH] feat(migrate): normalize any config to a clean, canonical, lossless form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ostp migrate` now finishes every kind (client/server/relay) with a uniform normalization pass so a messy config.json becomes a clean one: - Concise: null-valued keys are stripped at every nesting level. A JSON null means "unset", so it is noise; removing it never loses real data (a set value is never null). Empty [] / {} are kept — they carry intent. - Canonical order: free. serde_json serializes object keys sorted, so any rewrite comes out stably ordered regardless of how disordered the input was. - No data loss: normalization works on the JSON value and only removes nulls, so fields the schema has never heard of survive verbatim — proven by a test. This also fixes the "configs stay old even after ostp migrate" complaint: the normalize pass flips report.changed when it removes anything, so a config that was current-but-noisy actually gets rewritten clean instead of "nothing to migrate". Adds a forcing function: a test that the exact shapes `ostp init` / the wizard emit (client, server, relay — including the new outbound username/password) are already canonical, so migrate is a no-op on them. If a template or the schema gains a field without the migrator being taught, this fails instead of shipping a config that `ostp migrate` keeps trying to "fix". Plus tests for strip/keep, idempotency, and unknown-field preservation. --- ostp-client/src/migrate.rs | 109 +++++++++++++++++++++++++++++++++++++ ostp/src/main.rs | 12 +++- 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/ostp-client/src/migrate.rs b/ostp-client/src/migrate.rs index 2ea5e87..a856075 100644 --- a/ostp-client/src/migrate.rs +++ b/ostp-client/src/migrate.rs @@ -356,6 +356,41 @@ pub fn migrate_server_json(json: Value) -> (Value, MigrationReport) { (out, report) } +/// Final normalization pass applied to every migrated config: strip null-valued +/// keys at every nesting level. A JSON null means "unset", so it is pure noise — +/// removing it is the "concise" part of the migration, and it never loses real +/// data (a set value is never null). Key ORDER is already canonical for free: +/// serde_json serializes object keys in sorted order, so any write of a migrated +/// config comes out stably ordered no matter how disordered the input was. +/// +/// Returns whether it removed anything, so the caller folds it into the +/// "was this already up to date?" decision. +pub fn normalize(value: &mut Value) -> bool { + let before = value.clone(); + strip_nulls(value); + *value != before +} + +/// Recursively drop keys whose value is JSON null, descending into nested +/// objects and array elements. Empty objects and arrays are kept — an explicit +/// `rules: []` or `exclude: {}` carries intent; only nulls are noise. +fn strip_nulls(value: &mut Value) { + match value { + Value::Object(map) => { + map.retain(|_, v| !v.is_null()); + for v in map.values_mut() { + strip_nulls(v); + } + } + Value::Array(arr) => { + for v in arr.iter_mut() { + strip_nulls(v); + } + } + _ => {} + } +} + #[cfg(test)] mod tests { use super::*; @@ -556,4 +591,78 @@ mod tests { assert_eq!(detect_kind(&json!({"upstream_tcp": "x", "upstream_api_url": "y"})), Some(ConfigKind::Relay)); assert_eq!(detect_kind(&json!({"mode": "client", "server": "x"})), Some(ConfigKind::Client)); } + + // ── normalization (concise + no data loss + canonical) ────────────────── + + #[test] + fn normalize_strips_nulls_but_keeps_real_data_and_empty_collections() { + let mut v = json!({ + "mode": "client", + "server": "1.2.3.4:50000", + "access_key": "k", + "socks5_bind": null, // unset → removed + "tun": { "enable": true, "dns": null }, // nested null → removed + "exclude": { "domains": [], "ips": null }, // empty [] kept, null removed + "mux": { "enabled": false, "sessions": 1 }, + }); + let changed = normalize(&mut v); + assert!(changed, "stripping nulls is a change"); + assert!(v.get("socks5_bind").is_none(), "top-level null must be gone"); + assert!(v["tun"].get("dns").is_none(), "nested null must be gone"); + assert!(v["exclude"].get("ips").is_none(), "nested null must be gone"); + assert_eq!(v["exclude"]["domains"], json!([]), "an explicit empty array is intent, kept"); + assert_eq!(v["server"], json!("1.2.3.4:50000"), "real data untouched"); + assert_eq!(v["tun"]["enable"], json!(true)); + } + + #[test] + fn normalize_is_idempotent() { + let mut v = json!({ "mode": "server", "listen": "0.0.0.0:50000", "access_keys": ["k"], "debug": null }); + assert!(normalize(&mut v), "first pass removes the null"); + let once = v.clone(); + assert!(!normalize(&mut v), "second pass changes nothing"); + assert_eq!(v, once); + } + + #[test] + fn normalize_never_drops_unknown_fields() { + // A field the schema has never heard of must survive — no data loss ever. + let mut v = json!({ "mode": "client", "server": "s", "access_key": "k", "some_future_field": {"a": 1} }); + normalize(&mut v); + assert_eq!(v["some_future_field"], json!({"a": 1}), "unknown data must be preserved verbatim"); + } + + /// Forcing function: the configs the tool itself generates (init/setup + /// templates, current shape) must already be canonical — running the full + /// migrate pipeline over them must report NO change. If someone adds a field + /// to a template or the schema without teaching the migrator, this fails + /// instead of a user silently ending up with a config that `ostp migrate` + /// keeps trying to "fix". Covers all three kinds. + #[test] + fn generated_configs_are_already_canonical() { + // These mirror the exact shapes emitted by `ostp init` / the wizard. + let client = json!({ + "mode": "client", "server": "127.0.0.1:50000", "access_key": "k", + "socks5_bind": "127.0.0.1:1088", + "transport": { "mode": "udp", "tcp_fragmentation": false }, + "debug": false, + }); + let server = json!({ + "mode": "server", "listen": "0.0.0.0:50000", "access_keys": ["k"], + "outbound": { "enabled": false, "protocol": "socks5", "address": "127.0.0.1", + "port": 9050, "username": "", "password": "", "default_action": "proxy", "rules": [] }, + "debug": false, + }); + let relay = json!({ + "mode": "relay", "listen": "0.0.0.0:50000", + "upstream_tcp": "1.2.3.4:50000", "upstream_udp": "1.2.3.4:50000", "debug": false, + }); + + for (name, cfg) in [("client", client), ("server", server), ("relay", relay)] { + let mut v = cfg.clone(); + let changed = normalize(&mut v); + assert!(!changed, "the generated {name} template must be canonical (no nulls to strip)"); + assert_eq!(v, cfg, "normalizing the {name} template must not alter it"); + } + } } diff --git a/ostp/src/main.rs b/ostp/src/main.rs index f107854..5bb7c87 100644 --- a/ostp/src/main.rs +++ b/ostp/src/main.rs @@ -1556,7 +1556,7 @@ fn cmd_migrate(config_path: &std::path::Path) -> Result<()> { let kind = ostp_client::migrate::detect_kind(&parsed) .ok_or_else(|| anyhow!("Could not determine whether {:?} is a client, server, or relay config.", config_path))?; - let (migrated, report) = match kind { + let (mut migrated, mut report) = match kind { ostp_client::migrate::ConfigKind::Client => { let (mut v, r) = ostp_client::migrate::migrate_client_json(parsed); if v.get("mode").is_none() { v["mode"] = serde_json::json!("client"); } @@ -1573,6 +1573,16 @@ fn cmd_migrate(config_path: &std::path::Path) -> Result<()> { } }; + // Uniform final pass for every kind: strip null "unset" keys so the written + // config is concise. Key order is already canonical (serde_json sorts keys + // on write), so together with the kind-specific rules above this turns any + // disordered, noisy config.json into a clean canonical one — without losing + // any real data. + if ostp_client::migrate::normalize(&mut migrated) { + report.changed = true; + report.notes.push("Removed unset (null) keys and wrote the config in canonical order.".to_string()); + } + if !report.changed { println!("{} Config is already up to date, nothing to migrate.", "[ostp]".green().bold()); return Ok(());