[Bug 35239] Filter out illegal user names as per RFC 2812

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=35239

--- Comment #12 from Will Thompson <will.thompson at collabora.co.uk> 2011-03-25 09:09:25 PDT ---
Review of attachment 44854:
 --> (https://bugs.freedesktop.org/review?bug=35239&attachment=44854)

::: src/idle-connection.c
@@ +858,3 @@
+    IdleConnection *conn = IDLE_CONNECTION(user_data);
+    GHashTable *details = NULL;
+    TpConnectionStatus status = conn->parent.status;

If status is already TP_CONNECTION_STATUS_DISCONNECTED, then this function
should early-return. This will happen in the case where Disconnect() has
already been called before the ERROR is received from the server. Otherwise,
calling tp_base_connection_disconnect_with_dbus_error () on the
connection—which already knows it's disconnected/ing—will trigger a warning
from tp-glib.

@@ +875,3 @@
+        server_msg = g_new0(gchar, length + 1);
+        strncpy(server_msg, begin, length);
+    const gchar *end = strrchr(msg, ')');

I drew some boxes on some paper to check this code. :) I think this would be
clearer and equivalent:

  begin++;
  end--;
  length = end - begin;
  server_msg = g_strndup (server_msg + begin, length);

@@ +876,3 @@
+        strncpy(server_msg, begin, length);
+        server_msg[length] = '\0';
+    const gchar *error_name = (status == TP_CONNECTION_STATUS_CONNECTING) ?
TP_ERROR_STR_AUTHENTICATION_FAILED : TP_ERROR_STR_NETWORK_ERROR;

You should g_free (server_msg); here. ... ah, I see that you free it later. It
would be clearer, I think, to define gchar *server_msg only within this block,
and free it right after the call to tp_asv_new().

@@ +879,3 @@
+    }
+
+    gchar *server_msg = NULL;

As you noticed on IRC, the hash table needs to be unreffed if it's not NULL.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.