Refactroed self presence checks, handle null stanzas

Issue #341

2 files changed, 73 insertions, 108 deletions

diff --git a/src/xmpp/presence.c b/src/xmpp/presence.c
index 7969f0c0..5120e231 100644
--- a/src/xmpp/presence.c
+++ b/src/xmpp/presence.c
@@ -667,8 +667,8 @@ _muc_user_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza,
         return 1;
     }
 
-    char *room = from_jid->barejid;
-    char *nick = from_jid->resourcepart;
+    char *from_room = from_jid->barejid;
+    char *from_nick = from_jid->resourcepart;
 
     // handle self presence
     if (stanza_is_muc_self_presence(stanza, jabber_get_fulljid())) {
@@ -679,19 +679,19 @@ _muc_user_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza,
 
             // leave room if not self nick change
             if (new_nick != NULL) {
-                muc_set_room_pending_nick_change(room, new_nick);
+                muc_set_room_pending_nick_change(from_room, new_nick);
             } else {
-                handle_leave_room(room);
+                handle_leave_room(from_room);
             }
 
         // handle self nick change
-        } else if (muc_is_room_pending_nick_change(room)) {
-            muc_complete_room_nick_change(room, nick);
-            handle_room_nick_change(room, nick);
+        } else if (muc_is_room_pending_nick_change(from_room)) {
+            muc_complete_room_nick_change(from_room, from_nick);
+            handle_room_nick_change(from_room, from_nick);
 
         // handle roster complete
-        } else if (!muc_get_roster_received(room)) {
-            handle_room_roster_complete(room);
+        } else if (!muc_get_roster_received(from_room)) {
+            handle_room_roster_complete(from_room);
         }
 
     // handle presence from room members
@@ -714,28 +714,28 @@ _muc_user_handler(xmpp_conn_t * const conn, xmpp_stanza_t * const stanza,
             if (stanza_is_room_nick_change(stanza)) {
                 char *new_nick = stanza_get_new_nick(stanza);
                 if (new_nick != NULL) {
-                    muc_set_roster_pending_nick_change(room, new_nick, nick);
+                    muc_set_roster_pending_nick_change(from_room, new_nick, from_nick);
                     free(new_nick);
                 }
             } else {
-                handle_room_member_offline(room, nick, "offline", status_str);
+                handle_room_member_offline(from_room, from_nick, "offline", status_str);
             }
         } else {
             char *show_str = stanza_get_show(stanza, "online");
-            if (!muc_get_roster_received(room)) {
-                muc_add_to_roster(room, nick, show_str, status_str, caps_key);
+            if (!muc_get_roster_received(from_room)) {
+                muc_add_to_roster(from_room, from_nick, show_str, status_str, caps_key);
             } else {
-                char *old_nick = muc_complete_roster_nick_change(room, nick);
+                char *old_nick = muc_complete_roster_nick_change(from_room, from_nick);
 
                 if (old_nick != NULL) {
-                    muc_add_to_roster(room, nick, show_str, status_str, caps_key);
-                    handle_room_member_nick_change(room, old_nick, nick);
+                    muc_add_to_roster(from_room, from_nick, show_str, status_str, caps_key);
+                    handle_room_member_nick_change(from_room, old_nick, from_nick);
                     free(old_nick);
                 } else {
-                    if (!muc_nick_in_roster(room, nick)) {
-                        handle_room_member_online(room, nick, show_str, status_str, caps_key);
+                    if (!muc_nick_in_roster(from_room, from_nick)) {
+                        handle_room_member_online(from_room, from_nick, show_str, status_str, caps_key);
                     } else {
-                        handle_room_member_presence(room, nick, show_str, status_str, caps_key);
+                        handle_room_member_presence(from_room, from_nick, show_str, status_str, caps_key);
                     }
                 }
             }
diff --git a/src/xmpp/stanza.c b/src/xmpp/stanza.c
index 3c3f47fb..6006fdb4 100644
--- a/src/xmpp/stanza.c
+++ b/src/xmpp/stanza.c
@@ -634,127 +634,92 @@ gboolean
 stanza_is_muc_self_presence(xmpp_stanza_t * const stanza,
     const char * const self_jid)
 {
-    if (stanza == NULL) {
-        return FALSE;
-    }
-    if (strcmp(xmpp_stanza_get_name(stanza), STANZA_NAME_PRESENCE) != 0) {
-        return FALSE;
-    }
-
-    xmpp_stanza_t *x = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_X);
-
-    if (x == NULL) {
-        return FALSE;
-    }
-
-    char *ns = xmpp_stanza_get_ns(x);
-    if (ns == NULL) {
-        return FALSE;
-    }
-    if (strcmp(ns, STANZA_NS_MUC_USER) != 0) {
+    // no stanza, or not presence stanza
+    if ((stanza == NULL) || (g_strcmp0(xmpp_stanza_get_name(stanza), STANZA_NAME_PRESENCE) != 0)) {
         return FALSE;
     }
 
-    xmpp_stanza_t *x_children = xmpp_stanza_get_children(x);
-    if (x_children == NULL) {
-        return FALSE;
-    }
+    // muc user namespaced x element
+    xmpp_stanza_t *x = xmpp_stanza_get_child_by_ns(stanza, STANZA_NS_MUC_USER);
+    if (x != NULL) {
 
-    while (x_children != NULL) {
-        if (strcmp(xmpp_stanza_get_name(x_children), STANZA_NAME_STATUS) == 0) {
-            char *code = xmpp_stanza_get_attribute(x_children, STANZA_ATTR_CODE);
-            if (strcmp(code, "110") == 0) {
-                return TRUE;
+        // check for status child element with 110 code
+        xmpp_stanza_t *x_children = xmpp_stanza_get_children(x);
+        while (x_children != NULL) {
+            if (g_strcmp0(xmpp_stanza_get_name(x_children), STANZA_NAME_STATUS) == 0) {
+                char *code = xmpp_stanza_get_attribute(x_children, STANZA_ATTR_CODE);
+                if (g_strcmp0(code, "110") == 0) {
+                    return TRUE;
+                }
             }
+            x_children = xmpp_stanza_get_next(x_children);
         }
-        x_children = xmpp_stanza_get_next(x_children);
-    }
 
-    // for older server that don't send status 110
-    x_children = xmpp_stanza_get_children(x);
-    while (x_children != NULL) {
-        if (strcmp(xmpp_stanza_get_name(x_children), STANZA_NAME_ITEM) == 0) {
-            char *jid = xmpp_stanza_get_attribute(x_children, STANZA_ATTR_JID);
+        // check for item child element with jid property
+        xmpp_stanza_t *item = xmpp_stanza_get_child_by_name(x, STANZA_NAME_ITEM);
+        if (item != NULL) {
+            char *jid = xmpp_stanza_get_attribute(item, STANZA_ATTR_JID);
             if (jid != NULL) {
-                if (g_str_has_prefix(jid, self_jid)) {
+                if (g_str_has_prefix(self_jid, jid)) {
                     return TRUE;
                 }
             }
         }
-        x_children = xmpp_stanza_get_next(x_children);
-    }
-
-    // for servers that don't send status 110 or Jid property
 
-    // first check if 'from' attribute identifies this user
-    char *from = xmpp_stanza_get_attribute(stanza, STANZA_ATTR_FROM);
-    if (from != NULL) {
-        Jid *jidp = jid_create(from);
-        if (muc_room_is_active(jidp->barejid)) {
-            char *nick = muc_get_room_nick(jidp->barejid);
-            if (g_strcmp0(jidp->resourcepart, nick) == 0) {
-                return TRUE;
+        // check if 'from' attribute identifies this user
+        char *from = xmpp_stanza_get_attribute(stanza, STANZA_ATTR_FROM);
+        if (from != NULL) {
+            Jid *from_jid = jid_create(from);
+            if (muc_room_is_active(from_jid->barejid)) {
+                char *nick = muc_get_room_nick(from_jid->barejid);
+                if (g_strcmp0(from_jid->resourcepart, nick) == 0) {
+                    return TRUE;
+                }
             }
-        }
-        jid_destroy(jidp);
-    }
 
-    // secondly check if the new nickname maps to a pending nick change for this user
-    if (from != NULL) {
-        Jid *jidp = jid_create(from);
-        if (muc_is_room_pending_nick_change(jidp->barejid)) {
-            char *new_nick = jidp->resourcepart;
-            if (new_nick != NULL) {
-                char *nick = muc_get_room_nick(jidp->barejid);
-                char *old_nick = muc_get_old_nick(jidp->barejid, new_nick);
-                if (g_strcmp0(old_nick, nick) == 0) {
-                    return TRUE;
+            // check if a new nickname maps to a pending nick change for this user
+            if (muc_is_room_pending_nick_change(from_jid->barejid)) {
+                char *new_nick = from_jid->resourcepart;
+                if (new_nick != NULL) {
+                    char *nick = muc_get_room_nick(from_jid->barejid);
+                    char *old_nick = muc_get_old_nick(from_jid->barejid, new_nick);
+                    if (g_strcmp0(old_nick, nick) == 0) {
+                        return TRUE;
+                    }
                 }
             }
+
+            jid_destroy(from_jid);
         }
-        jid_destroy(jidp);
     }
 
+    // self presence not found
     return FALSE;
 }
 
 gboolean
 stanza_is_room_nick_change(xmpp_stanza_t * const stanza)
 {
-    if (stanza == NULL) {
-        return FALSE;
-    }
-    if (strcmp(xmpp_stanza_get_name(stanza), STANZA_NAME_PRESENCE) != 0) {
-        return FALSE;
-    }
-
-    xmpp_stanza_t *x = xmpp_stanza_get_child_by_name(stanza, STANZA_NAME_X);
-
-    if (x == NULL) {
-        return FALSE;
-    }
-
-    char *ns = xmpp_stanza_get_ns(x);
-    if (ns == NULL) {
-        return FALSE;
-    }
-    if (strcmp(ns, STANZA_NS_MUC_USER) != 0) {
+    // no stanza, or not presence stanza
+    if ((stanza == NULL) || (g_strcmp0(xmpp_stanza_get_name(stanza), STANZA_NAME_PRESENCE) != 0)) {
         return FALSE;
     }
 
-    xmpp_stanza_t *x_children = xmpp_stanza_get_children(x);
-    if (x_children == NULL) {
-        return FALSE;
-    }
+    // muc user namespaced x element
+    xmpp_stanza_t *x = xmpp_stanza_get_child_by_ns(stanza, STANZA_NS_MUC_USER);
+    if (x != NULL) {
 
-    while (x_children != NULL) {
-        if (strcmp(xmpp_stanza_get_name(x_children), STANZA_NAME_STATUS) == 0) {
-            char *code = xmpp_stanza_get_attribute(x_children, STANZA_ATTR_CODE);
-            if (strcmp(code, "303") == 0) {
-                return TRUE;
+        // check for status child element with 303 code
+        xmpp_stanza_t *x_children = xmpp_stanza_get_children(x);
+        while (x_children != NULL) {
+            if (g_strcmp0(xmpp_stanza_get_name(x_children), STANZA_NAME_STATUS) == 0) {
+                char *code = xmpp_stanza_get_attribute(x_children, STANZA_ATTR_CODE);
+                if (g_strcmp0(code, "303") == 0) {
+                    return TRUE;
+                }
             }
+            x_children = xmpp_stanza_get_next(x_children);
         }
-        x_children = xmpp_stanza_get_next(x_children);
     }
 
     return FALSE;