Correctly strip : from <trailing> parameters in MODE commands

This is to fix irssi#601. The prior function used to extract the mode string
assumed that ":" could occur only in a particular spot. This lead to the
possibility that ":" could be treated as part of things like nicknames
or mode arguments, where it should have been stripped as part of
protocol escaping.

We now recognize ":" as beginning a <trailing> parameter in any
position. There are still cases where this parsing is arguably not RFC
correct, but Irssi's mode parsing expects the mode arguments as a single
string already, so collapsing parameters in this way is probably okay.

As part of this change, I add a program to test both the new function
and the old. In order to use it, you need to run autogen.sh again, and
then run "make check".

.gitignore

@@ -36,6 +36,8 @@ src/fe-fuzz/irssi-fuzz
 
 src/fe-common/irc/irc-modules.c
 src/irc/irc.c
+src/irc/core/test-modes
+src/irc/core/test-modes.trs
 
 src/perl/perl-signals-list.h
 src/perl/irssi-core.pl.h

src/irc/core/Makefile.am

@@ -58,3 +58,30 @@ pkginc_irc_core_HEADERS = \
         netsplit.h \
         servers-idle.h \
         servers-redirect.h
+
+TESTS = test-modes
+check_PROGRAMS = test-modes
+
+test_modes_DEPENDENCIES = \
+	../../core/libcore.a \
+	../../lib-config/libirssi_config.a
+
+test_modes_LDADD = \
+	@GLIB_LIBS@ \
+	@OPENSSL_LIBS@ \
+	../../core/libcore.a \
+	../../lib-config/libirssi_config.a
+
+test_modes_SOURCES = \
+	test-modes.c \
+	irc-cap.c \
+	irc-nicklist.c \
+	irc-queries.c \
+	irc-servers-reconnect.c \
+	irc-servers-setup.c \
+	irc-servers.c \
+	irc.c \
+	mode-lists.c \
+	modes.c \
+	servers-idle.c \
+	servers-redirect.c

src/irc/core/modes.c

@@ -480,13 +480,15 @@ static void event_user_mode(IRC_SERVER_REC *server, const char *data)
 static void event_mode(IRC_SERVER_REC *server, const char *data,
 		       const char *nick)
 {
-	IRC_CHANNEL_REC *chanrec;
-	char *params, *channel, *mode;
+	IRC_CHANNEL_REC *chanrec = NULL;
+	char *channel = NULL, *mode = NULL;
 
 	g_return_if_fail(data != NULL);
 
-	params = event_get_params(data, 2 | PARAM_FLAG_GETREST,
-				  &channel, &mode);
+	get_mode_params(data, &channel, &mode);
+	if (channel == NULL || mode == NULL) {
+		return;
+	}
 
 	if (!server_ischannel(SERVER(server), channel)) {
 		/* user mode change */
@@ -498,7 +500,82 @@ static void event_mode(IRC_SERVER_REC *server, const char *data,
 			parse_channel_modes(chanrec, nick, mode, TRUE);
 	}
 
-	g_free(params);
+	g_free(channel);
+	g_free(mode);
+}
+
+// Given <params> to a MODE command, parse them into the target of the modes (a
+// channel or nickname) and a string holding the modes and their parameters.
+//
+// ":" will be removed from any <trailing> parameter.
+//
+// Since the parsing combines several parameters into one string, the exact
+// position of <trailing> is not particularly important. This is not very RFC
+// correct, but it has historically worked.
+//
+// For example:
+//
+// "#channel +o test" gets parsed to: target = "#channel" and modes = "+o test"
+// "#channel :+o test" gets parsed to: target = "#channel" and modes = "+o test"
+// "#channel +o :test" gets parsed to: target = "#channel" and modes = "+o test"
+//
+// The caller must free the target and modes strings.
+void get_mode_params(char const * const params, char * * target, char * * modes)
+{
+	char * params_copy = NULL;
+	char * params_ptr = NULL;
+	char * s = NULL;
+	char * mode_buf = NULL;
+
+	g_return_if_fail(params != NULL);
+	g_return_if_fail(target != NULL);
+	g_return_if_fail(modes != NULL);
+
+	// Copy params as parsing modifies the buffer.
+	params_copy = g_strdup(params);
+	params_ptr = params_copy;
+
+	// Get the target.
+
+	while (*params_ptr == ' ') {
+		params_ptr++;
+	}
+
+	s = event_get_param(&params_ptr);
+	if (s == NULL || strlen(s) == 0) {
+		g_free(params_copy);
+		return;
+	}
+	*target = g_strdup(s);
+
+	// Get the mode string. This is all remaining parameters as a single string.
+
+	// This is potentially larger than we need.
+	mode_buf = g_malloc0(strlen(params_ptr)+1);
+
+	// Get a single parameter at a time so that we correctly handle ":".
+	while (strlen(params_ptr) > 0) {
+		// event_get_param() doesn't ignore leading whitespace. Since parameters
+		// can be separated by more than a single space, strip spaces before using
+		// it.
+		while (*params_ptr == ' ') {
+			params_ptr++;
+		}
+
+		s = event_get_param(&params_ptr);
+		if (s == NULL || strlen(s) == 0) {
+			break;
+		}
+
+		if (strlen(mode_buf) > 0) {
+			strcat(mode_buf, " ");
+		}
+		strcat(mode_buf, s);
+	}
+
+	*modes = mode_buf;
+
+	g_free(params_copy);
 }
 
 static void event_oper(IRC_SERVER_REC *server, const char *data)

src/irc/core/modes.h

@@ -50,6 +50,8 @@ int channel_mode_is_set(IRC_CHANNEL_REC *channel, char mode);
 void parse_channel_modes(IRC_CHANNEL_REC *channel, const char *setby,
 			 const char *modestr, int update_key);
 
+void get_mode_params(char const * const, char * *, char * *);
+
 void channel_set_singlemode(IRC_CHANNEL_REC *channel, const char *nicks,
 			    const char *mode);
 void channel_set_mode(IRC_SERVER_REC *server, const char *channel,