[folks] Bug 636714 — Assertion failure on invalid IM address

[Philip Withnall <pwithnall src gnome org>]

commit f3ac848861074425cb88753ddddf124f4f7d358c
Author: Philip Withnall <philip tecnocode co uk>
Date:   Sun Dec 12 17:52:52 2010 +0000

    Bug 636714 â?? Assertion failure on invalid IM address
    
    Handle invalid IM addresses in relationships.ini more gracefully by throwing
    an exception from IMable.normalise_im_address() if it detects an invalid
    address, rather than aborting due to an assertion failure. Closes: bgo#636714

 NEWS                                    |    3 +
 backends/key-file/kf-persona.vala       |   49 +++++++++++++++++++----
 backends/telepathy/lib/tpf-persona.vala |   12 +++++-
 folks/imable.vala                       |   65 ++++++++++++++++++++++++++----
 4 files changed, 109 insertions(+), 20 deletions(-)
---
diff --git a/NEWS b/NEWS
index f5c8d30..56c948d 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ API changes:
 * Rename the Avatar interface to HasAvatar
 * Rename the Presence interface to HasPresence
 * Rename the Favourite interface to Favouritable
+* Add IMableError
+* Throw IMableError from IMable.normalise_im_address()
 
 Bugs fixed:
 * Bug 635178 â?? Leak in
@@ -15,6 +17,7 @@ Bugs fixed:
 * Bug 636251 â?? Fails to add contact
 * Bug 629526 â?? Generate gtk-doc documentation
 * Bug 627397 â?? Use better interface names
+* Bug 636714 â?? Assertion failure on invalid IM address
 
 Overview of changes from libfolks 0.3.1 to libfolks 0.3.2
 ==========================================================
diff --git a/backends/key-file/kf-persona.vala b/backends/key-file/kf-persona.vala
index 2486519..5bd5a99 100644
--- a/backends/key-file/kf-persona.vala
+++ b/backends/key-file/kf-persona.vala
@@ -104,14 +104,37 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
           value.foreach ((k, v) =>
             {
               unowned string protocol = (string) k;
-              unowned GenericArray<string> addresses = (GenericArray<string>) v;
+              unowned GenericArray<string?> addresses =
+                  (GenericArray<string?>) v;
+              uint offset = 0;
 
               for (int i = 0; i < addresses.length; i++)
                 {
-                  addresses[i] =
-                      IMable.normalise_im_address (addresses[i], protocol);
+                  try
+                    {
+                      addresses[i - offset] =
+                          IMable.normalise_im_address (addresses[i],
+                              protocol);
+                    }
+                  catch (IMableError e)
+                    {
+                      /* Somehow an error has crept into the user's
+                       * relationships.ini. Warn of it and ignore the IM
+                       * address. We achieve this by decrementing the offset
+                       * between the index of the address being set and the
+                       * index of the address being read. */
+                      warning (e.message);
+                      addresses[i - offset] = null;
+                      offset++;
+                    }
                 }
 
+              /* Nullify the last few addresses if we have a non-zero offset,
+               * as they're the gaps left by invalid addresses, which we want
+               * set_string_list() to ignore. */
+              for (; offset > 0; offset--)
+                addresses[addresses.length - offset] = null;
+
               unowned string[] _addresses =
                   (string[]) ((PtrArray) addresses).pdata;
               _addresses.length = (int) addresses.length;
@@ -181,13 +204,21 @@ public class Folks.Backends.Kf.Persona : Folks.Persona,
 
               foreach (string _address in im_addresses)
                 {
-                  string address =
-                      IMable.normalise_im_address (_address, protocol);
+                  try
+                    {
+                      string address =
+                          IMable.normalise_im_address (_address, protocol);
 
-                  if (!address_set.contains (address))
+                      if (!address_set.contains (address))
+                        {
+                          im_address_array.add (address);
+                          address_set.add (address);
+                        }
+                    }
+                  catch (IMableError e)
                     {
-                      im_address_array.add (address);
-                      address_set.add (address);
+                      /* Warn of and ignore any invalid IM addresses */
+                      warning (e.message);
                     }
                 }
 
diff --git a/backends/telepathy/lib/tpf-persona.vala b/backends/telepathy/lib/tpf-persona.vala
index fe7aaee..ce4e727 100644
--- a/backends/telepathy/lib/tpf-persona.vala
+++ b/backends/telepathy/lib/tpf-persona.vala
@@ -244,8 +244,16 @@ public class Tpf.Persona : Folks.Persona,
 
       /* Set our single IM address */
       GenericArray<string> im_address_array = new GenericArray<string> ();
-      im_address_array.add (IMable.normalise_im_address (id,
-          account.get_protocol ()));
+      try
+        {
+          im_address_array.add (IMable.normalise_im_address (id,
+              account.get_protocol ()));
+        }
+      catch (IMableError e)
+        {
+          /* This should never happenâ?¦but if it does, warn of it and continue */
+          warning (e.message);
+        }
 
       this._im_addresses =
           new HashTable<string, GenericArray<string>> (str_hash, str_equal);
diff --git a/folks/imable.vala b/folks/imable.vala
index fc7042a..25be0df 100644
--- a/folks/imable.vala
+++ b/folks/imable.vala
@@ -21,6 +21,17 @@
 using GLib;
 
 /**
+ * Errors related to IM addresses and IM address handling.
+ */
+public errordomain Folks.IMableError
+{
+  /**
+   * The specified IM address could not be parsed.
+   */
+  INVALID_IM_ADDRESS
+}
+
+/**
  * IM addresses exposed by an object implementing { link HasPresence}.
  *
  * @since 0.1.13
@@ -60,12 +71,19 @@ public interface Folks.IMable : Object
    * only one of which is canonical. In order to allow simple string comparisons
    * of IM addresses to work, the IM addresses must be normalised beforehand.
    *
+   * If the provided IM address is invalid,
+   * { link Folks.IMableError.INVALID_IM_ADDRESS} will be thrown. Note that this
+   * isn't guaranteed to be thrown for all invalid addresses, but if it is
+   * thrown, the address is guaranteed to be invalid.
+   *
    * @param im_address the address to normalise
    * @param protocol the protocol of this im_address
    *
    * @since 0.2.0
+   * @throws Folks.IMableError if the provided IM address was invalid
    */
   public static string normalise_im_address (string im_address, string protocol)
+      throws Folks.IMableError
     {
       string normalised;
 
@@ -83,7 +101,13 @@ public interface Folks.IMable : Object
           /* Parse the JID */
           string[] parts = im_address.split ("/", 2);
 
-          return_val_if_fail (parts.length >= 1, null);
+          if (parts.length < 1)
+            {
+              throw new IMableError.INVALID_IM_ADDRESS (
+                  /* Translators: the parameter is an IM address. */
+                  _("The IM address '%s' could not be understood."),
+                  im_address);
+            }
 
           string resource = null;
           if (parts.length == 2)
@@ -91,7 +115,13 @@ public interface Folks.IMable : Object
 
           parts = parts[0].split ("@", 2);
 
-          return_val_if_fail (parts.length >= 1, null);
+          if (parts.length < 1)
+            {
+              throw new IMableError.INVALID_IM_ADDRESS (
+                  /* Translators: the parameter is an IM address. */
+                  _("The IM address '%s' could not be understood."),
+                  im_address);
+            }
 
           string node, domain;
           if (parts.length == 2)
@@ -105,9 +135,15 @@ public interface Folks.IMable : Object
               domain = parts[0];
             }
 
-          return_val_if_fail (node == null || node != "", null);
-          return_val_if_fail (domain != null && domain != "", null);
-          return_val_if_fail (resource == null || resource != "", null);
+          if ((node != null && node == "") ||
+              (domain == null || domain == "") ||
+              (resource != null && resource == ""))
+            {
+              throw new IMableError.INVALID_IM_ADDRESS (
+                  /* Translators: the parameter is an IM address. */
+                  _("The IM address '%s' could not be understood."),
+                  im_address);
+            }
 
           domain = domain.down ();
           if (node != null)
@@ -115,13 +151,24 @@ public interface Folks.IMable : Object
 
           /* Build a new JID */
           if (node != null && resource != null)
-            normalised = "%s %s/%s".printf (node, domain, resource);
+            {
+              normalised = "%s %s/%s".printf (node, domain, resource);
+            }
           else if (node != null)
-            normalised = "%s %s".printf (node, domain);
+            {
+              normalised = "%s %s".printf (node, domain);
+            }
           else if (resource != null)
-            normalised = "%s/%s".printf (domain, resource);
+            {
+              normalised = "%s/%s".printf (domain, resource);
+            }
           else
-            assert_not_reached ();
+            {
+              throw new IMableError.INVALID_IM_ADDRESS (
+                  /* Translators: the parameter is an IM address. */
+                  _("The IM address '%s' could not be understood."),
+                  im_address);
+            }
         }
       else
         {