libmm-glib,bearer-properties: allow loose comparisons

When comparing bearer properties provided by the user versus loaded
from the modem, we shouldn't be very strict, e.g.:
 * Password or other fields may not be readable from the device.
 * Some fields may not apply at all (e.g. RM protocol for EPS bearers)
 * NULL strings could be assumed equal to empty strings.
 * If no explicit IP type specified, an IPv4 default may be assumed.
 * If no explicit allowed auth specified, 'none' default may be
   assumed.

These loose comparisons are applied when managing the initial EPS
bearer settings and status, and we keep the strict comparison only
during the connection attempt lookup of a bearer with certain
settings, as those bearer objects are all created in the same place
with the same rules.

4 files changed, 123 insertions, 20 deletions

diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c
index 5ed4438e..904d1248 100644
--- a/libmm-glib/mm-bearer-properties.c
+++ b/libmm-glib/mm-bearer-properties.c
@@ -667,21 +667,102 @@ mm_bearer_properties_new_from_dictionary (GVariant *dictionary,
 
 /*****************************************************************************/
 
+static gboolean
+cmp_str (const gchar                *a,
+         const gchar                *b,
+         MMBearerPropertiesCmpFlags  flags)
+{
+    /* Strict match */
+    if ((!a && !b) || (a && b && g_strcmp0 (a, b) == 0))
+        return TRUE;
+    /* Additional loose match, consider NULL and EMPTY string equal */
+    if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) {
+        if ((!a && !b[0]) || (!b && !a[0]))
+            return TRUE;
+    }
+    return FALSE;
+}
+
+static gboolean
+cmp_ip_type (MMBearerIpFamily           a,
+             MMBearerIpFamily           b,
+             MMBearerPropertiesCmpFlags flags)
+{
+    /* Strict match */
+    if (a == b)
+        return TRUE;
+    /* Additional loose match NONE == IPV4 */
+    if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) {
+        if ((a == MM_BEARER_IP_FAMILY_NONE && b == MM_BEARER_IP_FAMILY_IPV4) ||
+            (b == MM_BEARER_IP_FAMILY_NONE && a == MM_BEARER_IP_FAMILY_IPV4))
+            return TRUE;
+    }
+    return FALSE;
+}
+
+static gboolean
+cmp_allowed_auth (MMBearerAllowedAuth        a,
+                  MMBearerAllowedAuth        b,
+                  MMBearerPropertiesCmpFlags flags)
+{
+    /* Strict match */
+    if (a == b)
+        return TRUE;
+    /* Additional loose match UNKNOWN == NONE */
+    if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) {
+        if ((a == MM_BEARER_ALLOWED_AUTH_UNKNOWN && b == MM_BEARER_ALLOWED_AUTH_NONE) ||
+            (b == MM_BEARER_ALLOWED_AUTH_UNKNOWN && a == MM_BEARER_ALLOWED_AUTH_NONE))
+            return TRUE;
+    }
+    return FALSE;
+}
+
+static gboolean
+cmp_allow_roaming (gboolean                   a,
+                   gboolean                   a_set,
+                   gboolean                   b,
+                   gboolean                   b_set,
+                   MMBearerPropertiesCmpFlags flags)
+{
+    /* Strict match */
+    if (a == b && a_set == b_set)
+        return TRUE;
+    /* Additional loose match UNSET ==  */
+    if (flags & MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE) {
+        if ((a == MM_BEARER_ALLOWED_AUTH_UNKNOWN && b == MM_BEARER_ALLOWED_AUTH_NONE) ||
+            (b == MM_BEARER_ALLOWED_AUTH_UNKNOWN && a == MM_BEARER_ALLOWED_AUTH_NONE))
+            return TRUE;
+    }
+    return FALSE;
+}
+
 /**
  * mm_bearer_properties_cmp: (skip)
  */
 gboolean
-mm_bearer_properties_cmp (MMBearerProperties *a,
-                          MMBearerProperties *b)
+mm_bearer_properties_cmp (MMBearerProperties         *a,
+                          MMBearerProperties         *b,
+                          MMBearerPropertiesCmpFlags  flags)
 {
-    return ((!g_strcmp0 (a->priv->apn, b->priv->apn)) &&
-            (a->priv->ip_type == b->priv->ip_type) &&
-            (a->priv->allowed_auth == b->priv->allowed_auth) &&
-            (!g_strcmp0 (a->priv->user, b->priv->user)) &&
-            (!g_strcmp0 (a->priv->password, b->priv->password)) &&
-            (a->priv->allow_roaming == b->priv->allow_roaming) &&
-            (a->priv->allow_roaming_set == b->priv->allow_roaming_set) &&
-            (a->priv->rm_protocol == b->priv->rm_protocol));
+    if (!cmp_str (a->priv->apn, b->priv->apn, flags))
+        return FALSE;
+    if (!cmp_ip_type (a->priv->ip_type, b->priv->ip_type, flags))
+        return FALSE;
+    if (!cmp_allowed_auth (a->priv->allowed_auth, a->priv->allowed_auth, flags))
+        return FALSE;
+    if (!cmp_str (a->priv->user, b->priv->user, flags))
+        return FALSE;
+    if (!(flags & MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD) && !cmp_str (a->priv->password, b->priv->password, flags))
+        return FALSE;
+    if (!(flags & MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING)) {
+        if (a->priv->allow_roaming != b->priv->allow_roaming)
+            return FALSE;
+        if (a->priv->allow_roaming_set != b->priv->allow_roaming)
+            return FALSE;
+    }
+    if (a->priv->rm_protocol != b->priv->rm_protocol)
+        return FALSE;
+    return TRUE;
 }
 
 /*****************************************************************************/
diff --git a/libmm-glib/mm-bearer-properties.h b/libmm-glib/mm-bearer-properties.h
index 751185d9..340d9bf1 100644
--- a/libmm-glib/mm-bearer-properties.h
+++ b/libmm-glib/mm-bearer-properties.h
@@ -113,8 +113,17 @@ gboolean mm_bearer_properties_consume_variant (MMBearerProperties *properties,
 
 GVariant *mm_bearer_properties_get_dictionary (MMBearerProperties *self);
 
-gboolean mm_bearer_properties_cmp (MMBearerProperties *a,
-                                   MMBearerProperties *b);
+typedef enum {
+    MM_BEARER_PROPERTIES_CMP_FLAGS_NONE             = 0,
+    MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE            = 1 << 0,
+    MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD      = 1 << 1,
+    MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING = 1 << 2,
+    MM_BEARER_PROPERTIES_CMP_FLAGS_NO_RM_PROTOCOL   = 1 << 3,
+} MMBearerPropertiesCmpFlags;
+
+gboolean mm_bearer_properties_cmp (MMBearerProperties         *a,
+                                   MMBearerProperties         *b,
+                                   MMBearerPropertiesCmpFlags  flags);
 
 #endif
 
diff --git a/src/mm-bearer-list.c b/src/mm-bearer-list.c
index 504b99ae..65717132 100644
--- a/src/mm-bearer-list.c
+++ b/src/mm-bearer-list.c
@@ -153,7 +153,11 @@ mm_bearer_list_find_by_properties (MMBearerList       *self,
     GList *l;
 
     for (l = self->priv->bearers; l; l = g_list_next (l)) {
-        if (mm_bearer_properties_cmp (mm_base_bearer_peek_config (MM_BASE_BEARER (l->data)), props))
+        /* always strict matching when comparing these bearer properties, as they're all
+         * built in the same place */
+        if (mm_bearer_properties_cmp (mm_base_bearer_peek_config (MM_BASE_BEARER (l->data)),
+                                      props,
+                                      MM_BEARER_PROPERTIES_CMP_FLAGS_NONE))
             return g_object_ref (l->data);
     }
 
diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c
index df8df272..4e153c78 100644
--- a/src/mm-iface-modem-3gpp.c
+++ b/src/mm-iface-modem-3gpp.c
@@ -30,6 +30,16 @@
 
 #define SUBSYSTEM_3GPP "3gpp"
 
+/* When comparing EPS bearer settings take into account that PASSWORD may not always
+ * be readable, and apply very loose matching for all fields. Also, some implementations
+ * may allow configuring roaming allowance in the initial EPS bearer, but that is also
+ * not common. */
+#define MM_BEARER_PROPERTIES_CMP_FLAGS_EPS              \
+    (MM_BEARER_PROPERTIES_CMP_FLAGS_LOOSE |             \
+     MM_BEARER_PROPERTIES_CMP_FLAGS_NO_PASSWORD |       \
+     MM_BEARER_PROPERTIES_CMP_FLAGS_NO_ALLOW_ROAMING |  \
+     MM_BEARER_PROPERTIES_CMP_FLAGS_NO_RM_PROTOCOL)
+
 /*****************************************************************************/
 /* Private data context */
 
@@ -1029,7 +1039,7 @@ after_set_load_initial_eps_bearer_settings_ready (MMIfaceModem3gpp
     mm_obj_dbg (self, "Updated initial EPS bearer settings:");
     log_initial_eps_bearer_settings (self, new_config);
 
-    if (!mm_bearer_properties_cmp (new_config, ctx->config)) {
+    if (!mm_bearer_properties_cmp (new_config, ctx->config, MM_BEARER_PROPERTIES_CMP_FLAGS_EPS)) {
         mm_obj_dbg (self, "Requested initial EPS bearer settings:");
         log_initial_eps_bearer_settings (self, ctx->config);
         g_dbus_method_invocation_return_error_literal (ctx->invocation, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
@@ -1108,15 +1118,11 @@ set_initial_eps_bearer_settings_auth_ready (MMBaseModem
         return;
     }
 
-    /* If the user doesn't specify explicit auth settings, assume NONE as default */
-    if (mm_bearer_properties_get_allowed_auth (ctx->config) == MM_BEARER_ALLOWED_AUTH_UNKNOWN)
-        mm_bearer_properties_set_allowed_auth (ctx->config, MM_BEARER_ALLOWED_AUTH_NONE);
-
     old_dictionary = mm_gdbus_modem3gpp_get_initial_eps_bearer_settings (ctx->skeleton);
     if (old_dictionary)
         old_config = mm_bearer_properties_new_from_dictionary (old_dictionary, NULL);
 
-    if (old_config && mm_bearer_properties_cmp (ctx->config, old_config)) {
+    if (old_config && mm_bearer_properties_cmp (ctx->config, old_config, MM_BEARER_PROPERTIES_CMP_FLAGS_EPS)) {
         mm_gdbus_modem3gpp_complete_set_initial_eps_bearer_settings (ctx->skeleton, ctx->invocation);
         handle_set_initial_eps_bearer_settings_context_free (ctx);
     } else {
@@ -1756,7 +1762,10 @@ mm_iface_modem_3gpp_update_initial_eps_bearer (MMIfaceModem3gpp   *self,
 
     /* skip update? */
     if ((!old_bearer && !properties) ||
-        (old_bearer && properties && mm_bearer_properties_cmp (properties, mm_base_bearer_peek_config (MM_BASE_BEARER (old_bearer)))))
+        (old_bearer && properties &&
+         mm_bearer_properties_cmp (properties,
+                                   mm_base_bearer_peek_config (MM_BASE_BEARER (old_bearer)),
+                                   MM_BEARER_PROPERTIES_CMP_FLAGS_EPS)))
         goto out;
 
     if (properties) {