diff --git a/crates/bitwarden-json/src/client.rs b/crates/bitwarden-json/src/client.rs index 416067b66..590cf45e6 100644 --- a/crates/bitwarden-json/src/client.rs +++ b/crates/bitwarden-json/src/client.rs @@ -24,16 +24,10 @@ impl Client { } }; - if let Some(cmd_value_map) = cmd_value.as_object_mut() { - cmd_value_map.retain(|_, v| !v.is_null()); - - for &subcommand in SUBCOMMANDS_TO_CLEAN { - if let Some(cmd_value_secrets) = cmd_value_map - .get_mut(subcommand) - .and_then(|v| v.as_object_mut()) - { - cmd_value_secrets.retain(|_, v| !v.is_null()); - } + clean_null_fields(&mut cmd_value); + for &subcommand in SUBCOMMANDS_TO_CLEAN { + if let Some(v) = cmd_value.get_mut(subcommand) { + clean_null_fields(v) } } @@ -85,7 +79,17 @@ impl Client { fn parse_settings(settings_input: Option) -> Option { if let Some(input) = settings_input.as_ref() { - match serde_json::from_str(input) { + let mut value: serde_json::Value = match serde_json::from_str(input) { + Ok(value) => value, + Err(e) => { + log::error!("Failed to parse settings: {}", e); + return None; + } + }; + + clean_null_fields(&mut value); + + match serde_json::from_value(value) { Ok(settings) => return Some(settings), Err(e) => { log::error!("Failed to parse settings: {}", e); @@ -95,3 +99,22 @@ impl Client { None } } + +/// Removes null fields from a json object value, note that this isn't recursive. +/// This is needed because some of the language bindings will send null values when a value is not set, +/// and this causes problems with some deserializations, for example: +/// +/// ClientSettings is using #[serde(default)] and non-optional fields, which means any missing +/// field will be deserialized as the value from ClientSettings::default(). This does not work if the value +/// is explicitly defined as null, which will produce a deserialization error. +/// +/// The Command enum is serialized/deserialized as '{ "variant_name": {/* Variant contents */} }', serde +/// is expecting only a single variant field in the root object, but some of the bindings are sending the +/// rest of fields with null values, which will produce a deserialization error. +/// '{ "other_variant": null, "variant_name": {/* Variant contents */}, "another_variant": null }' +/// +fn clean_null_fields(value: &mut serde_json::Value) { + if let Some(object) = value.as_object_mut() { + object.retain(|_, v| !v.is_null()); + } +}