Removed unnecessary QuotedString from SExpr model

Author: ancwrd1

snx-rs-gui/src/settings.rs

@@ -348,7 +348,7 @@ impl SettingsDialog {
                                 .map(|factor| factor.factor_type.clone())
                                 .collect::<Vec<_>>();
                             unsafe { auth_type.set_data(&option.id, factors); }
-                            auth_type.append(Some(&option.id), &option.display_name.0);
+                            auth_type.append(Some(&option.id), &option.display_name);
                             if params2.login_type == option.id {
                                 auth_type.set_active(Some(i as _));
                             }

snxcore/src/model/proto.rs

@@ -93,7 +93,7 @@ pub struct OfficeMode {
     pub ipaddr: String,
     pub keep_address: Option<bool>,
     pub dns_servers: Option<Vec<Ipv4Addr>>,
-    pub dns_suffix: Option<QuotedStringList>,
+    pub dns_suffix: Option<StringList>,
 }
 
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -220,16 +220,16 @@ pub struct MultiChallengeRequest {
 
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct ClientLoggingData {
-    pub client_name: Option<QuotedString>,
+    pub client_name: Option<String>,
     pub client_ver: Option<String>,
     pub client_build_number: Option<String>,
     pub os_name: Option<String>,
     pub os_version: Option<String>,
     pub device_type: Option<String>,
     pub hardware_model: Option<String>,
     pub machine_name: Option<String>,
-    pub device_id: Option<QuotedString>,
-    pub mac_address: Option<QuotedStringList>,
+    pub device_id: Option<String>,
+    pub mac_address: Option<StringList>,
     pub physical_ip: Option<Ipv4Addr>,
     pub is_compliant: Option<String>,
 }
@@ -324,7 +324,7 @@ pub struct KeyManagementResponse {
     pub om_dns0: Option<u32>,
     pub om_dns1: Option<u32>,
     pub om_dns2: Option<u32>,
-    pub om_domain_name: Option<QuotedString>,
+    pub om_domain_name: Option<String>,
     pub lifetime: Option<u64>,
     pub encalg: EncryptionAlgorithm,
     pub authalg: AuthenticationAlgorithm,
@@ -380,7 +380,7 @@ pub struct DisconnectRequest {
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct DisconnectRequestData {
     pub code: String,
-    pub message: Option<QuotedString>,
+    pub message: Option<String>,
 }
 
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -406,7 +406,7 @@ pub struct ProtocolVersion {
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct UpgradeConfiguration {
     pub available_client_version: u32,
-    pub client_upgrade_url: QuotedString,
+    pub client_upgrade_url: String,
     pub upgrade_mode: String,
 }
 
@@ -420,7 +420,7 @@ pub struct ConnectivityInfo {
     pub ipsec_transport: String,
     pub tcpt_port: u16,
     pub natt_port: u16,
-    pub connect_with_certificate_url: QuotedString,
+    pub connect_with_certificate_url: String,
     pub cookie_name: String,
     pub internal_ca_fingerprint: BTreeMap<String, String>,
 }
@@ -435,7 +435,7 @@ pub struct LoginOptionsData {
 pub struct LoginOption {
     pub id: String,
     pub secondary_realm_hash: String,
-    pub display_name: QuotedString,
+    pub display_name: String,
     pub show_realm: u32,
     pub factors: BTreeMap<String, LoginFactor>,
 }
@@ -463,15 +463,15 @@ pub struct LoginFactor {
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(untagged)]
 pub enum LoginDisplayLabelSelect {
-    LoginDisplayLabel(BTreeMap<String, QuotedString>),
+    LoginDisplayLabel(BTreeMap<String, String>),
     Empty(String),
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct LoginDisplayLabel {
-    pub header: QuotedString,
-    pub username: Option<QuotedString>,
-    pub password: Option<QuotedString>,
+    pub header: String,
+    pub username: Option<String>,
+    pub password: Option<String>,
 }
 
 #[derive(Default, Debug, Clone, PartialEq, Serialize, Deserialize)]

snxcore/src/model/wrappers.rs

@@ -5,63 +5,11 @@ use serde::{
     Deserialize, Deserializer, Serialize, Serializer,
 };
 
-/// String encoded with double quotes
+/// String separated with commas or semicolons
 #[derive(Default, Clone, PartialEq)]
-pub struct QuotedString(pub String);
+pub struct StringList(pub Vec<String>);
 
-impl Serialize for QuotedString {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: Serializer,
-    {
-        format!("\"{}\"", self.0).serialize(serializer)
-    }
-}
-
-impl<'de> Deserialize<'de> for QuotedString {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        Ok(Self(String::deserialize(deserializer)?.trim_matches('"').to_owned()))
-    }
-}
-
-impl From<String> for QuotedString {
-    fn from(value: String) -> Self {
-        Self(value)
-    }
-}
-
-impl From<QuotedString> for String {
-    fn from(value: QuotedString) -> Self {
-        value.0
-    }
-}
-
-impl<'a> From<&'a str> for QuotedString {
-    fn from(value: &'a str) -> Self {
-        Self(value.to_owned())
-    }
-}
-
-impl fmt::Display for QuotedString {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        std::fmt::Display::fmt(&self.0, f)
-    }
-}
-
-impl fmt::Debug for QuotedString {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        std::fmt::Debug::fmt(&self.0, f)
-    }
-}
-
-/// String encoded with double quotes and separated with commas
-#[derive(Default, Clone, PartialEq)]
-pub struct QuotedStringList(pub Vec<String>);
-
-impl Serialize for QuotedStringList {
+impl Serialize for StringList {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
@@ -70,7 +18,7 @@ impl Serialize for QuotedStringList {
     }
 }
 
-impl<'de> Deserialize<'de> for QuotedStringList {
+impl<'de> Deserialize<'de> for StringList {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
@@ -85,19 +33,19 @@ impl<'de> Deserialize<'de> for QuotedStringList {
     }
 }
 
-impl From<Vec<String>> for QuotedStringList {
+impl From<Vec<String>> for StringList {
     fn from(value: Vec<String>) -> Self {
         Self(value)
     }
 }
 
-impl From<QuotedStringList> for Vec<String> {
-    fn from(value: QuotedStringList) -> Self {
+impl From<StringList> for Vec<String> {
+    fn from(value: StringList) -> Self {
         value.0
     }
 }
 
-impl fmt::Debug for QuotedStringList {
+impl fmt::Debug for StringList {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         std::fmt::Debug::fmt(&self.0, f)
     }
@@ -122,7 +70,7 @@ impl<'de> Deserialize<'de> for EncryptedString {
         D: Deserializer<'de>,
     {
         let s = String::deserialize(deserializer)?;
-        let decrypted = crate::util::snx_decrypt(s.as_bytes()).map_err(serde::de::Error::custom)?;
+        let decrypted = crate::util::snx_decrypt(s.as_bytes()).map_err(Error::custom)?;
         Ok(Self(String::from_utf8_lossy(&decrypted).into_owned()))
     }
 }

snxcore/src/sexpr.rs

@@ -17,7 +17,6 @@ struct SExpressionParser;
 pub enum SExpression {
     Null,
     Value(String),
-    QuotedValue(String),
     Object(Option<String>, BTreeMap<String, SExpression>),
     Array(Vec<SExpression>),
 }
@@ -40,9 +39,7 @@ impl SExpression {
     }
 
     pub fn get_value<T: FromStr>(&self, path: &str) -> Option<T> {
-        self.get(path)
-            .and_then(|v| v.as_value().or_else(|| v.as_quoted_value()))
-            .and_then(|v| v.parse().ok())
+        self.get(path).and_then(|v| v.as_value()).and_then(|v| v.parse().ok())
     }
 
     pub fn get_num_value<T: Num>(&self, path: &str) -> Option<T> {
@@ -89,8 +86,7 @@ impl SExpression {
     fn encode_with_level(&self, level: u32) -> Option<String> {
         match self {
             SExpression::Null => None,
-            SExpression::Value(value) => Some(format!("({value})")),
-            SExpression::QuotedValue(value) => Some(format!("(\"{value}\")")),
+            SExpression::Value(value) => Some(format_value(value)),
             SExpression::Object(name, object) => Some(self.encode_object(level, name.as_deref(), object)),
             SExpression::Array(items) => Some(self.encode_array(level, items)),
         }
@@ -130,7 +126,6 @@ impl SExpression {
         match self {
             Self::Null => Value::Null,
             Self::Value(v) => to_json_value(v),
-            Self::QuotedValue(v) => Value::String(format!("\"{v}\"")),
             Self::Object(name, fields) => to_json_object(name.as_deref(), fields),
             SExpression::Array(elements) => Value::Array(elements.iter().map(|v| v.to_json()).collect()),
         }
@@ -141,13 +136,7 @@ impl SExpression {
             Value::Null => Self::Null,
             Value::Bool(v) => Self::Value(v.to_string()),
             Value::Number(v) => Self::Value(v.to_string()),
-            Value::String(v) => {
-                if v.starts_with('"') && v.ends_with('"') {
-                    Self::QuotedValue(v.trim_matches('"').to_string())
-                } else {
-                    Self::Value(v.to_string())
-                }
-            }
+            Value::String(v) => Self::Value(v.to_string()),
             Value::Array(v) => Self::Array(v.into_iter().map(Self::from_json).collect()),
             Value::Object(v) => match v.iter().next() {
                 Some((key, value)) if key.starts_with('(') => Self::Object(
@@ -211,6 +200,14 @@ fn to_json_value(v: &str) -> Value {
     }
 }
 
+fn format_value(value: &str) -> String {
+    if value.contains(|c: char| !c.is_alphanumeric()) {
+        format!("(\"{}\")", value)
+    } else {
+        format!("({})", value)
+    }
+}
+
 fn to_json_object<N: AsRef<str>, K: AsRef<str>>(name: Option<N>, fields: &BTreeMap<K, SExpression>) -> Value {
     let inner = Value::Object(
         fields
@@ -277,7 +274,7 @@ fn parse_value(mut pairs: RulePairs) -> anyhow::Result<SExpression> {
     match pairs.next() {
         Some(pair) if pair.as_rule() == Rule::quoted_str => {
             let value = pair.into_inner().as_str().to_owned();
-            Ok(SExpression::QuotedValue(value))
+            Ok(SExpression::Value(value))
         }
         Some(pair) if pair.as_rule() == Rule::simple_val => {
             let value = pair.as_str().to_owned();
@@ -356,8 +353,13 @@ mod tests {
     fn test_parse_array() {
         let data = "(Response :data (: (hello) : (world)))";
         let expr = data.parse::<SExpression>().unwrap();
-        println!("{expr:#?}");
-        println!("{expr}");
+        assert_eq!(
+            expr.get("Response:data").unwrap().as_array().unwrap(),
+            &vec![
+                SExpression::Value("hello".to_string()),
+                SExpression::Value("world".to_string())
+            ]
+        );
     }
 
     #[test]
@@ -368,7 +370,36 @@ mod tests {
         }
         let data = Data { key: None };
         let expr = SExpression::from(&data);
-        println!("{expr:#?}");
-        println!("{expr}");
+        assert_eq!(expr.get("key"), Some(&SExpression::Null));
+    }
+
+    #[test]
+    fn test_quoted_value_from_str() {
+        let data = "(Response\n\t:data (\"hello world\"))";
+        let expr = data.parse::<SExpression>().unwrap();
+
+        let inner = expr.get("Response:data").unwrap().as_value().unwrap();
+        assert_eq!(inner, "hello world");
+
+        let encoded = format!("{}", expr);
+        assert_eq!(encoded, data);
+    }
+
+    #[test]
+    fn test_quoted_value_from_pod() {
+        #[derive(Serialize)]
+        struct Data {
+            key: String,
+        }
+        let data = Data {
+            key: "Helloworld!".to_owned(),
+        };
+        let expr = SExpression::from(&data);
+
+        let inner = expr.get("key").unwrap().as_value().unwrap();
+        assert_eq!(inner, "Helloworld!");
+
+        let encoded = format!("{}", expr);
+        assert_eq!(encoded, "(\n\t:key (\"Helloworld!\"))");
     }
 }