From 7a3a4a151c86263fe41ddd789d910e3d8b256104 Mon Sep 17 00:00:00 2001 From: Akrm Al-Hakimi Date: Tue, 16 Jun 2026 10:21:32 -0400 Subject: [PATCH] fix(#442): merge saved connection patches before update Fetch the existing saved profile settings before applying SettingsPatch, then write the full merged settings map back through NetworkManager Update or UpdateUnsaved. NetworkManager replaces profile settings on Update, so sending only the patch delta could drop required fields like connection.type and fail validation. Closes #442 --- nmrs/CHANGELOG.md | 3 ++ nmrs/src/api/network_manager.rs | 5 ++- nmrs/src/core/saved_connection.rs | 48 ++++++++++++++++++++++++++-- nmrs/src/dbus/settings_connection.rs | 2 +- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index 1335155f..849e8f13 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to the `nmrs` crate will be documented in this file. - `NetworkManager::get_saved_connection_uuid()` — resolve a profile UUID from `connection.id` (usually the Wi-Fi SSID) for use with `update_saved_connection` ([#442](https://github.com/networkmanager-rs/nmrs/issues/442)) - `DeviceState::is_enabled()`, `Device.frequency`, and `WifiDevice.active_frequency_mhz` expose device usability and active Wi-Fi AP frequency without requiring separate AP lookups ([#445](https://github.com/networkmanager-rs/nmrs/pull/445)) +### Fixed +- `NetworkManager::update_saved_connection()` now merges `SettingsPatch` into the full existing settings map before calling NetworkManager `Update`, avoiding missing required fields such as `connection.type` ([#446](https://github.com/networkmanager-rs/nmrs/pull/446)) + ## [3.2.0] - 2026-05-31 ### Added - Add EAP-TLS support for WPA-Enterprise Wi-Fi, including TLS certificate/key path or blob configuration on `EapOptions` and `EapMethod::Tls` ([#434](https://github.com/networkmanager-rs/nmrs/pull/434)) diff --git a/nmrs/src/api/network_manager.rs b/nmrs/src/api/network_manager.rs index 408a59d3..ea0b0bd0 100644 --- a/nmrs/src/api/network_manager.rs +++ b/nmrs/src/api/network_manager.rs @@ -1171,7 +1171,10 @@ impl NetworkManager { saved_profiles::delete_saved_connection(&self.conn, uuid).await } - /// Merges a [`SettingsPatch`] into an existing profile (`Update` / `UpdateUnsaved`). + /// Merges a [`SettingsPatch`] into an existing profile. + /// + /// This loads the current settings, applies the patch, then writes the full + /// settings map back with NetworkManager's `Update` / `UpdateUnsaved`. /// /// `uuid` is the profile's `connection.uuid` (see [`SavedConnection::uuid`]), **not** /// the Wi-Fi SSID from a scan [`Network`]. Use [`Self::get_saved_connection_uuid`] or diff --git a/nmrs/src/core/saved_connection.rs b/nmrs/src/core/saved_connection.rs index 472b2c4e..d89a6da8 100644 --- a/nmrs/src/core/saved_connection.rs +++ b/nmrs/src/core/saved_connection.rs @@ -61,6 +61,18 @@ pub(crate) fn build_settings_patch_delta( delta } +fn merge_settings_patch_delta( + settings: &mut HashMap>, + delta: HashMap>, +) { + for (section, entries) in delta { + let target = settings.entry(section).or_default(); + for (key, value) in entries { + target.insert(key, value); + } + } +} + fn owned_to_str(v: &OwnedValue) -> Option { Str::try_from(v.clone()) .ok() @@ -684,6 +696,15 @@ pub(crate) async fn update_saved_connection( return Ok(()); } + let mut settings = proxy + .get_settings() + .await + .map_err(|e| ConnectionError::DbusOperation { + context: "GetSettings failed before update".into(), + source: e, + })?; + merge_settings_patch_delta(&mut settings, delta); + let unsaved = proxy .unsaved() .await @@ -694,7 +715,7 @@ pub(crate) async fn update_saved_connection( if unsaved { proxy - .update_unsaved(delta) + .update_unsaved(settings) .await .map_err(|e| ConnectionError::DbusOperation { context: "UpdateUnsaved failed".into(), @@ -702,7 +723,7 @@ pub(crate) async fn update_saved_connection( })?; } else { proxy - .update(delta) + .update(settings) .await .map_err(|e| ConnectionError::DbusOperation { context: "Update failed".into(), @@ -929,6 +950,29 @@ mod tests { ); } + #[test] + fn patch_delta_merges_into_full_settings() { + let mut settings = HashMap::new(); + settings.insert( + "connection".into(), + conn_section("u5", "Home", "802-11-wireless"), + ); + + let patch = SettingsPatch { + autoconnect: Some(false), + ..Default::default() + }; + let delta = build_settings_patch_delta(&patch); + merge_settings_patch_delta(&mut settings, delta); + + let conn = settings.get("connection").unwrap(); + assert_eq!( + owned_to_str(conn.get("type").unwrap()).as_deref(), + Some("802-11-wireless") + ); + assert_eq!(conn.get("autoconnect"), Some(&OwnedValue::from(false))); + } + #[test] fn patch_delta_overlay_merges_section() { let mut overlay = HashMap::new(); diff --git a/nmrs/src/dbus/settings_connection.rs b/nmrs/src/dbus/settings_connection.rs index 2a77409f..00977994 100644 --- a/nmrs/src/dbus/settings_connection.rs +++ b/nmrs/src/dbus/settings_connection.rs @@ -13,7 +13,7 @@ pub trait NMSettingsConnection { /// Full connection settings (`a{sa{sv}}`), excluding secrets. fn get_settings(&self) -> zbus::Result>>; - /// Merges partial settings into this profile. + /// Replaces this profile with the provided complete settings map. fn update(&self, settings: HashMap>) -> zbus::Result<()>; /// Like [`update`](Self::update) for in-memory (unsaved) profiles.