[telepathy-idle/master] Be strict for what nicks we allow, and liberal for what nicks we accept from others
Jonathon Jongsma
jonathon.jongsma at collabora.co.uk
Wed May 27 16:06:34 PDT 2009
Bip has a nasty habit of sending backlog activity from a fictional '-bip' user.
Unfortunately, a nick with a leading '-' character is illegal according to the
IRC RFCs, but we have to accept it since people appear to be using it in the
wild.
This patch adds a flag to the nick validation script that specifies whether we
should use strict mode or lenient mode. We use strict mode for validating local
nicknames, and lenient mode for validating remote nicknames.
---
src/idle-connection-manager.c | 2 +-
src/idle-connection.c | 1 +
src/idle-handles.c | 52 ++++++++++++++++++------
src/idle-handles.h | 2 +-
src/idle-muc-channel.c | 1 +
src/idle-muc-factory.c | 2 +
src/idle-server-connection.c | 2 +-
tests/twisted/Makefile.am | 1 +
tests/twisted/connect/invalid-nick.py | 6 +++
tests/twisted/messages/accept-invalid-nicks.py | 31 ++++++++++++++
10 files changed, 84 insertions(+), 16 deletions(-)
create mode 100644 tests/twisted/messages/accept-invalid-nicks.py
diff --git a/src/idle-connection-manager.c b/src/idle-connection-manager.c
index fbd7ccd..a94550b 100644
--- a/src/idle-connection-manager.c
+++ b/src/idle-connection-manager.c
@@ -79,7 +79,7 @@ filter_nick(const TpCMParamSpec *paramspec, GValue *value, GError **error)
g_assert(G_VALUE_HOLDS_STRING(value));
const gchar* nick = g_value_get_string (value);
- if (!idle_nickname_is_valid(nick)) {
+ if (!idle_nickname_is_valid(nick, TRUE)) {
g_set_error(error, TP_ERRORS, TP_ERROR_INVALID_HANDLE, "Invalid account name '%s'", nick);
return FALSE;
}
diff --git a/src/idle-connection.c b/src/idle-connection.c
index 1e7929a..244ab69 100644
--- a/src/idle-connection.c
+++ b/src/idle-connection.c
@@ -766,6 +766,7 @@ static IdleParserHandlerResult _nick_handler(IdleParser *parser, IdleParserMessa
return IDLE_PARSER_HANDLER_RESULT_NOT_HANDLED;
if (old_handle == conn->parent.self_handle) {
+ IDLE_DEBUG("Self renamed: handle was %d, now %d", old_handle, new_handle);
tp_base_connection_set_self_handle(TP_BASE_CONNECTION(conn), new_handle);
}
diff --git a/src/idle-handles.c b/src/idle-handles.c
index 73396c1..f5d0e5d 100644
--- a/src/idle-handles.c
+++ b/src/idle-handles.c
@@ -27,14 +27,28 @@
#include <telepathy-glib/errors.h>
#include <telepathy-glib/handle-repo-dynamic.h>
-gboolean idle_nickname_is_valid(const gchar *nickname) {
+#define IDLE_DEBUG_FLAG IDLE_DEBUG_PARSER
+#include "idle-debug.h"
+
+/* When strict_mode is true, we validate the nick strictly against the IRC
+ * RFCs (e.g. only ascii characters, no leading '-'. When strict_mode is
+ * false, we releax the requirements slightly. This is because we don't want
+ * to allow contribute to invalid nicks on IRC, but we want to be able to
+ * handle them if another client allows people to use invalid nicks */
+gboolean idle_nickname_is_valid(const gchar *nickname, gboolean strict_mode) {
const gchar *char_pos = nickname;
+ IDLE_DEBUG("Validating nickname '%s' with strict mode %d", nickname, strict_mode);
+
/* FIXME: also check for max length? */
if (!nickname || *nickname == '\0')
return FALSE;
- while (TRUE) {
+ while (*char_pos) {
+
+ /* only used for non-strict checks */
+ gunichar ucs4char = g_utf8_get_char_validated(char_pos, -1);
+ IDLE_DEBUG("testing character %d", ucs4char);
switch (*char_pos) {
case '[':
@@ -48,23 +62,35 @@ gboolean idle_nickname_is_valid(const gchar *nickname) {
case '}':
break;
- /* - not allowed as first char in a nickname */
+ /* '-' is technically not allowed as first char in a nickname */
case '-':
- if (char_pos == nickname)
- return FALSE;
- break;
-
- case '\0':
- return TRUE;
+ if (strict_mode) {
+ if (char_pos == nickname)
+ return FALSE;
+ }
break;
default:
- if (!(g_ascii_isalpha(*char_pos) || ((char_pos != nickname) && g_ascii_isdigit(*char_pos))))
- return FALSE;
+ if (strict_mode) {
+ /* only accept ascii characters in strict mode */
+ if (!(g_ascii_isalpha(*char_pos) || ((char_pos != nickname) && g_ascii_isdigit(*char_pos)))) {
+ IDLE_DEBUG("invalid character '%c'", *char_pos);
+ return FALSE;
+ }
+ } else {
+ /* allow unicode and digits as first char in non-strict mode */
+ if (!(g_unichar_isalpha(ucs4char) || g_unichar_isdigit(ucs4char))) {
+ IDLE_DEBUG("invalid character %d", ucs4char);
+ return FALSE;
+ }
+ }
break;
}
- ++char_pos;
+ /* in strict mode, a multi-byte character would have already failed the
+ * test above, so the following line should be equivalent to ++char, but
+ * it doesn't seem worthwhile to special-case it */
+ char_pos = g_utf8_find_next_char(char_pos, NULL);
}
return TRUE;
@@ -109,7 +135,7 @@ static gboolean _channelname_is_valid(const gchar *channel) {
}
static gchar *_nick_normalize_func(TpHandleRepoIface *repo, const gchar *id, gpointer ctx, GError **error) {
- if (!idle_nickname_is_valid(id)) {
+ if (!idle_nickname_is_valid(id, FALSE)) {
g_set_error(error, TP_ERRORS, TP_ERROR_INVALID_HANDLE, "invalid nickname");
return NULL;
}
diff --git a/src/idle-handles.h b/src/idle-handles.h
index 24e277b..9e8f9a1 100644
--- a/src/idle-handles.h
+++ b/src/idle-handles.h
@@ -27,7 +27,7 @@
G_BEGIN_DECLS
void idle_handle_repos_init(TpHandleRepoIface **handles);
-gboolean idle_nickname_is_valid(const gchar *nickname);
+gboolean idle_nickname_is_valid(const gchar *nickname, gboolean strict_mode);
G_END_DECLS
diff --git a/src/idle-muc-channel.c b/src/idle-muc-channel.c
index b89b6cc..02cc1e2 100644
--- a/src/idle-muc-channel.c
+++ b/src/idle-muc-channel.c
@@ -675,6 +675,7 @@ static void change_state(IdleMUCChannel *obj, IdleMUCState state) {
priv = IDLE_MUC_CHANNEL_GET_PRIVATE(obj);
if ((state > MUC_STATE_JOINING) && (!priv->join_ready)) {
+ IDLE_DEBUG("emitting join-ready");
g_signal_emit(obj, signals[JOIN_READY], 0, MUC_CHANNEL_JOIN_ERROR_NONE);
priv->join_ready = TRUE;
}
diff --git a/src/idle-muc-factory.c b/src/idle-muc-factory.c
index ffbf5ef..1563f76 100644
--- a/src/idle-muc-factory.c
+++ b/src/idle-muc-factory.c
@@ -581,8 +581,10 @@ static void _channel_closed_cb(IdleMUCChannel *chan, gpointer user_data) {
static void _channel_join_ready_cb(IdleMUCChannel *chan, guint err, gpointer user_data) {
TpChannelFactoryIface *iface = TP_CHANNEL_FACTORY_IFACE(user_data);
IdleMUCFactoryPrivate *priv = IDLE_MUC_FACTORY_GET_PRIVATE(user_data);
+ IDLE_DEBUG("join-ready with error=%d", err);
if (err == MUC_CHANNEL_JOIN_ERROR_NONE) {
+ IDLE_DEBUG("emitting NewChannel signal");
tp_channel_factory_iface_emit_new_channel(iface, (TpChannelIface *) chan, NULL);
return;
}
diff --git a/src/idle-server-connection.c b/src/idle-server-connection.c
index 22382b4..46e9c5f 100644
--- a/src/idle-server-connection.c
+++ b/src/idle-server-connection.c
@@ -629,7 +629,7 @@ static gboolean iface_send_impl(IdleServerConnectionIface *iface, const gchar *c
return FALSE;
case G_IO_STATUS_NORMAL:
- IDLE_DEBUG("sent \"%s\"", cmd);
+ IDLE_DEBUG("sent \"%s\" to IOChannel %p", cmd, priv->io_chan);
return TRUE;
diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am
index b2c1498..9c4cf0b 100644
--- a/tests/twisted/Makefile.am
+++ b/tests/twisted/Makefile.am
@@ -7,6 +7,7 @@ TWISTED_TESTS = \
connect/server-quit-noclose.py \
connect/invalid-nick.py \
channels/join-muc-channel.py \
+ messages/accept-invalid-nicks.py \
messages/message-order.py \
messages/leading-space.py \
messages/long-message-split.py \
diff --git a/tests/twisted/connect/invalid-nick.py b/tests/twisted/connect/invalid-nick.py
index cb5e824..44c1f92 100644
--- a/tests/twisted/connect/invalid-nick.py
+++ b/tests/twisted/connect/invalid-nick.py
@@ -50,6 +50,12 @@ def test():
assert e.get_dbus_name() == INVALID_HANDLE
try:
+ connect('12foo') # numbers not allowed as first char
+ raise RuntimeError('Invalid nick not rejected')
+ except dbus.DBusException, e:
+ assert e.get_dbus_name() == INVALID_HANDLE
+
+ try:
connect('-foo') # '-' not allowed as first char
raise RuntimeError('Invalid nick not rejected')
except dbus.DBusException, e:
diff --git a/tests/twisted/messages/accept-invalid-nicks.py b/tests/twisted/messages/accept-invalid-nicks.py
new file mode 100644
index 0000000..a0e92b1
--- /dev/null
+++ b/tests/twisted/messages/accept-invalid-nicks.py
@@ -0,0 +1,31 @@
+# coding: utf-8
+"""
+Regression test for a bug where, if you were in a IRC channel that had the same
+name as your nickname (e.g. user 'foo' in room '#foo'), all private 1:1 messages
+to foo would appear to also be coming through room #foo as well (bug #19766)
+"""
+
+from idletest import exec_test
+from servicetest import EventPattern, call_async
+import dbus
+
+def test(q, bus, conn, stream):
+ conn.Connect()
+ q.expect('dbus-signal', signal='StatusChanged', args=[0, 1])
+
+ stream.sendMessage('PRIVMSG', stream.nick, ':testing testing', prefix='-bip')
+ q.expect('dbus-signal', signal='Received')
+ # FIXME: we should be lenient and accept unicode nicks that we recieve
+ # from remote servers, but twisted can't seem to send unicode text so I
+ # don't seem to be able to test this :(
+ #stream.sendMessage('PRIVMSG', stream.nick, ':testing testing', prefix=u'ê¹€ì •ì€')
+ #q.expect('dbus-signal', signal='Received')
+ stream.sendMessage('PRIVMSG', stream.nick, ':testing testing', prefix='12foo')
+ q.expect('dbus-signal', signal='Received')
+
+ call_async(q, conn, 'Disconnect')
+ return True
+
+if __name__ == '__main__':
+ exec_test(test)
+
--
1.5.6.5
More information about the telepathy-commits
mailing list