feat: improve deserialization errors by using serde-untagged on enums (…

…#9952) * Use serde-untagged instead of derive trait for capabilities structs * Update core/tauri-utils/Cargo.toml * improve errors for other untagged enums * clippy * add tests, fix deserialization * use schemars attribute instead --------- Co-authored-by: Amr Bashir <[email protected]> Co-authored-by: Lucas Nogueira <[email protected]>
tauri-apps · Jun 26, 2024 · 167b51a · 167b51a
1 parent 8781987
commit 167b51a
Show file tree

Hide file tree

Showing 5 changed files with 205 additions and 8 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/core/tauri-utils/Cargo.toml b/core/tauri-utils/Cargo.toml
@@ -42,6 +42,7 @@ infer = "0.15"
 dunce = "1"
 log = "0.4.21"
 cargo_metadata = { version = "0.18", optional = true }
+serde-untagged = "0.1"
 
 [target."cfg(target_os = \"macos\")".dependencies]
 swift-rs = { version = "1.0.6", optional = true, features = [ "build" ] }

diff --git a/core/tauri-utils/src/acl/capability.rs b/core/tauri-utils/src/acl/capability.rs
@@ -7,15 +7,19 @@
 use std::{path::Path, str::FromStr};
 
 use crate::{acl::Identifier, platform::Target};
-use serde::{Deserialize, Serialize};
+use serde::{
+  de::{Error, IntoDeserializer},
+  Deserialize, Deserializer, Serialize,
+};
+use serde_untagged::UntaggedEnumVisitor;
 
 use super::Scopes;
 
 /// An entry for a permission value in a [`Capability`] can be either a raw permission [`Identifier`]
 /// or an object that references a permission and extends its scope.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
-#[serde(untagged)]
+#[derive(Debug, Clone, PartialEq, Serialize)]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
+#[serde(untagged)]
 pub enum PermissionEntry {
   /// Reference a permission or permission set by identifier.
   PermissionRef(Identifier),
@@ -42,6 +46,34 @@ impl PermissionEntry {
   }
 }
 
+impl<'de> Deserialize<'de> for PermissionEntry {
+  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+  where
+    D: Deserializer<'de>,
+  {
+    #[derive(Deserialize)]
+    struct ExtendedPermissionStruct {
+      identifier: Identifier,
+      #[serde(default, flatten)]
+      scope: Scopes,
+    }
+
+    UntaggedEnumVisitor::new()
+      .string(|string| {
+        let de = string.into_deserializer();
+        Identifier::deserialize(de).map(Self::PermissionRef)
+      })
+      .map(|map| {
+        let ext_perm = map.deserialize::<ExtendedPermissionStruct>()?;
+        Ok(Self::ExtendedPermission {
+          identifier: ext_perm.identifier,
+          scope: ext_perm.scope,
+        })
+      })
+      .deserialize(deserializer)
+  }
+}
+
 /// A grouping and boundary mechanism developers can use to isolate access to the IPC layer.
 ///
 /// It controls application windows fine grained access to the Tauri core, application, or plugin commands.
@@ -204,9 +236,9 @@ pub struct CapabilityRemote {
 }
 
 /// Capability formats accepted in a capability file.
-#[derive(Deserialize)]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
-#[serde(untagged)]
+#[cfg_attr(feature = "schema", schemars(untagged))]
+#[cfg_attr(test, derive(Debug, PartialEq))]
 pub enum CapabilityFile {
   /// A single capability.
   Capability(Capability),
@@ -234,6 +266,36 @@ impl CapabilityFile {
   }
 }
 
+impl<'de> Deserialize<'de> for CapabilityFile {
+  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+  where
+    D: Deserializer<'de>,
+  {
+    UntaggedEnumVisitor::new()
+      .seq(|seq| seq.deserialize::<Vec<Capability>>().map(Self::List))
+      .map(|map| {
+        #[derive(Deserialize)]
+        struct CapabilityNamedList {
+          capabilities: Vec<Capability>,
+        }
+
+        let value: serde_json::Map<String, serde_json::Value> = map.deserialize()?;
+        if value.contains_key("capabilities") {
+          serde_json::from_value::<CapabilityNamedList>(value.into())
+            .map(|named| Self::NamedList {
+              capabilities: named.capabilities,
+            })
+            .map_err(|e| serde_untagged::de::Error::custom(e.to_string()))
+        } else {
+          serde_json::from_value::<Capability>(value.into())
+            .map(Self::Capability)
+            .map_err(|e| serde_untagged::de::Error::custom(e.to_string()))
+        }
+      })
+      .deserialize(deserializer)
+  }
+}
+
 impl FromStr for CapabilityFile {
   type Err = super::Error;
 
@@ -309,3 +371,71 @@ mod build {
     }
   }
 }
+
+#[cfg(test)]
+mod tests {
+  use crate::acl::{Identifier, Scopes};
+
+  use super::{Capability, CapabilityFile, PermissionEntry};
+
+  #[test]
+  fn permission_entry_de() {
+    let identifier = Identifier::try_from("plugin:perm".to_string()).unwrap();
+    let identifier_json = serde_json::to_string(&identifier).unwrap();
+    assert_eq!(
+      serde_json::from_str::<PermissionEntry>(&identifier_json).unwrap(),
+      PermissionEntry::PermissionRef(identifier.clone())
+    );
+
+    assert_eq!(
+      serde_json::from_value::<PermissionEntry>(serde_json::json!({
+        "identifier": identifier,
+        "allow": [],
+        "deny": null
+      }))
+      .unwrap(),
+      PermissionEntry::ExtendedPermission {
+        identifier,
+        scope: Scopes {
+          allow: Some(vec![]),
+          deny: None
+        }
+      }
+    );
+  }
+
+  #[test]
+  fn capability_file_de() {
+    let capability = Capability {
+      identifier: "test".into(),
+      description: "".into(),
+      remote: None,
+      local: true,
+      windows: vec![],
+      webviews: vec![],
+      permissions: vec![],
+      platforms: None,
+    };
+    let capability_json = serde_json::to_string(&capability).unwrap();
+
+    assert_eq!(
+      serde_json::from_str::<CapabilityFile>(&capability_json).unwrap(),
+      CapabilityFile::Capability(capability.clone())
+    );
+
+    assert_eq!(
+      serde_json::from_str::<CapabilityFile>(&format!("[{capability_json}]")).unwrap(),
+      CapabilityFile::List(vec![capability.clone()])
+    );
+
+    assert_eq!(
+      serde_json::from_str::<CapabilityFile>(&format!(
+        "{{ \"capabilities\": [{capability_json}] }}"
+      ))
+      .unwrap(),
+      CapabilityFile::NamedList {
+        capabilities: vec![capability.clone()]
+      }
+    );
+  }
+}
diff --git a/core/tauri-utils/src/config.rs b/core/tauri-utils/src/config.rs
@@ -18,6 +18,7 @@ use serde::{
   Deserialize, Serialize, Serializer,
 };
 use serde_json::Value as JsonValue;
+use serde_untagged::UntaggedEnumVisitor;
 use serde_with::skip_serializing_none;
 use url::Url;
 
@@ -270,7 +271,14 @@ impl<'de> Deserialize<'de> for BundleTarget {
 
     match BundleTargetInner::deserialize(deserializer)? {
       BundleTargetInner::All(s) if s.to_lowercase() == "all" => Ok(Self::All),
-      BundleTargetInner::All(t) => Err(DeError::custom(format!("invalid bundle type {t}"))),
+      BundleTargetInner::All(t) => Err(DeError::custom(format!(
+        "invalid bundle type {t}, expected one of `all`, {}",
+        BundleType::all()
+          .iter()
+          .map(|b| format!("`{b}`"))
+          .collect::<Vec<_>>()
+          .join(", ")
+      ))),
       BundleTargetInner::List(l) => Ok(Self::List(l)),
       BundleTargetInner::One(t) => Ok(Self::One(t)),
     }
@@ -1708,16 +1716,28 @@ pub struct SecurityConfig {
 }
 
 /// A capability entry which can be either an inlined capability or a reference to a capability defined on its own file.
-#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
+#[derive(Debug, Clone, PartialEq, Serialize)]
 #[cfg_attr(feature = "schema", derive(JsonSchema))]
-#[serde(rename_all = "camelCase", untagged)]
+#[serde(untagged)]
 pub enum CapabilityEntry {
   /// An inlined capability.
   Inlined(Capability),
   /// Reference to a capability identifier.
   Reference(String),
 }
 
+impl<'de> Deserialize<'de> for CapabilityEntry {
+  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+  where
+    D: Deserializer<'de>,
+  {
+    UntaggedEnumVisitor::new()
+      .string(|string| Ok(Self::Reference(string.to_owned())))
+      .map(|map| map.deserialize::<Capability>().map(Self::Inlined))
+      .deserialize(deserializer)
+  }
+}
+
 /// The application pattern.
 #[skip_serializing_none]
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]

diff --git a/tooling/cli/Cargo.lock b/tooling/cli/Cargo.lock