Refactored command parsing.

Allows for automatically tested command parsing by splitting parsing
and invokation of the commands.

GNUmakefile

@@ -187,6 +187,7 @@ adcrush_SOURCES := src/tools/adcrush.c
 admin_SOURCES := src/tools/admin.c
 
 autotest_SOURCES := \
+		autotest/test_commands.tcc \
 		autotest/test_credentials.tcc \
 		autotest/test_eventqueue.tcc \
 		autotest/test_hub.tcc \

autotest/test_commands.tcc

@@ -0,0 +1,146 @@
+#include <uhub.h>
+
+static struct hub_info* hub = NULL;
+static struct hub_user user;
+static struct command_base* cbase = NULL;
+static struct command_handle* c_test1 = NULL;
+static struct command_handle* c_test2 = NULL;
+static struct command_handle* c_test3 = NULL;
+static struct command_handle* c_test4 = NULL;
+static struct command_handle* c_test5 = NULL;
+static struct command_handle* c_test6 = NULL;
+static struct command_handle* c_test7 = NULL;
+
+// for results:
+static int result = 0;
+
+EXO_TEST(setup, {
+	hub = hub_malloc_zero(sizeof(struct hub_info));
+	cbase = command_initialize(hub);
+	return cbase && hub && uman_init(hub) == 0;
+});
+
+static int test_handler(struct command_base* cbase, struct hub_user* user, struct hub_command* hcmd)
+{
+	result = 1;
+	return 0;
+}
+
+static struct command_handle* create_handler(const char* prefix, const char* args, enum auth_credentials cred)
+{
+	struct command_handle* c = hub_malloc_zero(sizeof(struct command_handle));
+	c->prefix = prefix;
+	c->length = strlen(prefix);
+	c->args = args;
+	c->cred = cred;
+	c->handler = test_handler;
+	c->description = "A handler added by autotest.";
+	c->description = "exotic";
+	c->ptr = &c->ptr;
+	return c;
+}
+
+EXO_TEST(command_setup_user, {
+	memset(&user, 0, sizeof(user));
+	user.id.sid = 1;
+	strcpy(user.id.nick, "tester");
+	strcpy(user.id.cid, "3AGHMAASJA2RFNM22AA6753V7B7DYEPNTIWHBAY");
+	user.credentials = auth_cred_guest;
+	return 1;
+});
+
+#define ADD_TEST(var, prefix, args, cred) \
+	var = create_handler(prefix, args, cred); \
+	if (!command_add(cbase, var, NULL)) \
+		return 0;
+
+#define DEL_TEST(var) \
+		if (var) \
+		{ \
+			if (!command_del(cbase, var)) \
+				return 0; \
+			hub_free(var); \
+			var = NULL; \
+		}
+
+EXO_TEST(command_create, {
+	ADD_TEST(c_test1, "test1", "", auth_cred_guest);
+	ADD_TEST(c_test2, "test2", "", auth_cred_operator);
+	ADD_TEST(c_test3, "test3", "N?N?N", auth_cred_guest);
+	ADD_TEST(c_test4, "test4", "n", auth_cred_guest);
+	ADD_TEST(c_test5, "test5", "i", auth_cred_guest);
+	ADD_TEST(c_test6, "test6", "?c", auth_cred_guest);
+	ADD_TEST(c_test6, "test7", "C", auth_cred_guest);
+	return 1;
+});
+
+extern void command_destroy(struct hub_command* cmd);
+
+static int verify(const char* str, enum command_parse_status expected)
+{
+	struct hub_command* cmd = command_parse(cbase, &user, str);
+	enum command_parse_status status = cmd->status;
+	command_free(cmd);
+	return status == expected;
+}
+
+EXO_TEST(command_access_1, { return verify("!test1", cmd_status_ok); });
+EXO_TEST(command_access_2, { return verify("!test2", cmd_status_access_error); });
+EXO_TEST(command_access_3, { user.credentials = auth_cred_operator; return verify("!test2", cmd_status_ok); });
+
+EXO_TEST(command_syntax_1, { return verify("", cmd_status_syntax_error); });
+EXO_TEST(command_syntax_2, { return verify("!", cmd_status_syntax_error); });
+
+EXO_TEST(command_missing_args_1, { return verify("!test3", cmd_status_missing_args); });
+EXO_TEST(command_missing_args_2, { return verify("!test3 12345", cmd_status_ok); });
+EXO_TEST(command_missing_args_3, { return verify("!test3 1 2 345", cmd_status_ok); });
+EXO_TEST(command_number_1, { return verify("!test3 abc", cmd_status_arg_number); });
+EXO_TEST(command_number_2, { return verify("!test3 -", cmd_status_arg_number); });
+EXO_TEST(command_number_3, { return verify("!test3 -12", cmd_status_ok); });
+
+EXO_TEST(command_user_1, { return verify("!test4 tester", cmd_status_arg_nick); });
+EXO_TEST(command_user_2, { return verify("!test5 3AGHMAASJA2RFNM22AA6753V7B7DYEPNTIWHBAY", cmd_status_arg_cid); });
+EXO_TEST(command_user_3, { return uman_add(hub, &user) == 0; });
+EXO_TEST(command_user_4, { return verify("!test4 tester", cmd_status_ok); });
+EXO_TEST(command_user_5, { return verify("!test5 3AGHMAASJA2RFNM22AA6753V7B7DYEPNTIWHBAY", cmd_status_ok); });
+
+EXO_TEST(command_command_1, { return verify("!test6 test1", cmd_status_ok); });
+EXO_TEST(command_command_2, { return verify("!test6 test2", cmd_status_ok); });
+EXO_TEST(command_command_3, { return verify("!test6 test3", cmd_status_ok); });
+EXO_TEST(command_command_4, { return verify("!test6 test4", cmd_status_ok); });
+EXO_TEST(command_command_5, { return verify("!test6 test5", cmd_status_ok); });
+EXO_TEST(command_command_6, { return verify("!test6 test6", cmd_status_ok); });
+EXO_TEST(command_command_7, { return verify("!test6 fail", cmd_status_arg_command); });
+EXO_TEST(command_command_8, { return verify("!test6", cmd_status_ok); });
+
+EXO_TEST(command_cred_1, { return verify("!test7 guest", cmd_status_ok); });
+EXO_TEST(command_cred_2, { return verify("!test7 user", cmd_status_ok); });
+EXO_TEST(command_cred_3, { return verify("!test7 operator", cmd_status_ok); });
+EXO_TEST(command_cred_4, { return verify("!test7 super", cmd_status_ok); });
+EXO_TEST(command_cred_5, { return verify("!test7 admin", cmd_status_ok); });
+EXO_TEST(command_cred_6, { return verify("!test7 nobody", cmd_status_arg_cred); });
+EXO_TEST(command_cred_7, { return verify("!test7 bot", cmd_status_ok); });
+EXO_TEST(command_cred_8, { return verify("!test7 link", cmd_status_ok); });
+
+
+#if 0
+	cmd_status_arg_cred,       /** <<< "A credentials argument is not valid ('C')" */
+};
+#endif
+
+// command not found
+EXO_TEST(command_parse_3, { return verify("!fail", cmd_status_not_found); });
+
+// built-in command
+EXO_TEST(command_parse_4, { return verify("!help", cmd_status_ok); });
+
+EXO_TEST(command_destroy, {
+	DEL_TEST(c_test1);
+	DEL_TEST(c_test2);
+	DEL_TEST(c_test3);
+	DEL_TEST(c_test4);
+	DEL_TEST(c_test5);
+	DEL_TEST(c_test6);
+	DEL_TEST(c_test7);
+	return 1;
+});

src/core/commands.h

@@ -17,20 +17,51 @@
  *
  */
 
-#include "uhub.h"
+#ifndef HAVE_UHUB_COMMANDS_H
+#define HAVE_UHUB_COMMANDS_H
 
 struct command_base;
 struct command_handle;
+struct hub_command;
 
-struct hub_command
+typedef int (*command_handler)(struct command_base* cbase, struct hub_user* user, struct hub_command* cmd);
+
+enum command_parse_status
 {
-	const char* message;
-	char* prefix;
-	size_t prefix_len;
-	struct linked_list* args;
+	cmd_status_ok,             /** <<< "Everything seems to OK" */
+	cmd_status_not_found,      /** <<< "Command was not found" */
+	cmd_status_access_error,   /** <<< "You don't have access to this command" */
+	cmd_status_syntax_error,   /** <<< "Not a valid command." */
+	cmd_status_missing_args,   /** <<< "Missing some or all required arguments." */
+	cmd_status_arg_nick,       /** <<< "A nick argument does not match an online user. ('n')" */
+	cmd_status_arg_cid,        /** <<< "A cid argument does not match an online user. ('i')." */
+	cmd_status_arg_address,    /** <<< "A address range argument is not valid ('a')." */
+	cmd_status_arg_number,     /** <<< "A number argument is not valid ('N')" */
+	cmd_status_arg_cred,       /** <<< "A credentials argument is not valid ('C')" */
+	cmd_status_arg_command,    /** <<< "A command argument is not valid ('c')" */
 };
 
-typedef int (*command_handler)(struct command_base*, struct hub_user* user, struct command_handle*, struct hub_command*);
+/**
+ * This struct contains all information needed to invoke
+ * a command, which includes the whole message, the prefix,
+ * the decoded arguments (according to parameter list), and
+ * the user pointer (ptr) which comes from the command it was matched to.
+ *
+ * The message and prefix is generally always available, but args only
+ * if status == cmd_status_ok.
+ * Handler and ptr are NULL if status == cmd_status_not_found, or status == cmd_status_access_error.
+ * Ptr might also be NULL if cmd_status_ok because the command that handles it was added with a NULL ptr.
+ */
+struct hub_command
+{
+	enum command_parse_status status;   /**<<< "Status of the hub_command." */
+	const char* message;                /**<<< "The complete message." */
+	char* prefix;                       /**<<< "The prefix extracted from the message." */
+	struct linked_list* args;           /**<<< "List of all parsed arguments from the message. Type depends on expectations." */
+	command_handler handler;            /**<<< "The function handler to call in order to invoke this command." */
+	const struct hub_user* user;        /**<<< "The user who invoked this command." */
+	void* ptr;                          /**<<< "A pointer of data which came from struct command_handler" */
+};
 
 /**
  * Argument codes are used to automatically parse arguments
@@ -55,18 +86,16 @@ typedef int (*command_handler)(struct command_base*, struct hub_user* user, stru
  */
 struct command_handle
 {
-	const char* prefix;				/**<<< "Command prefix, for instance 'help' would be the prefix for the !help command." */
-	size_t length;					/**<<< "Length of the prefix" */
-	const char* args;				/**<<< "Argument codes (see above)" */
-	enum auth_credentials cred;		/**<<< "Minimum access level for the command" */
-	command_handler handler;		/**<<< "Function pointer for the command" */
-	const char* description;		/**<<< "Description for the command" */
-	const char* origin;				/**<<< "Name of module where the command is implemented." */
-	void* ptr;
+	const char* prefix;             /**<<< "Command prefix, for instance 'help' would be the prefix for the !help command." */
+	size_t length;                  /**<<< "Length of the prefix" */
+	const char* args;               /**<<< "Argument codes (see above)" */
+	enum auth_credentials cred;     /**<<< "Minimum access level for the command" */
+	command_handler handler;        /**<<< "Function pointer for the command" */
+	const char* description;        /**<<< "Description for the command" */
+	const char* origin;             /**<<< "Name of module where the command is implemented." */
+	void* ptr;                      /**<<< "A pointer which will be passed along to the handler. @See hub_command::ptr" */
 };
 
-
-
 /**
  * Returns NULL on error, or handle
  */
@@ -85,11 +114,6 @@ extern int command_add(struct command_base*, struct command_handle*, void* ptr);
  */
 extern int command_del(struct command_base*, struct command_handle*);
 
-/**
- * Returns 1 if a command is available to a user (user has access to run it.)
- */
-extern int command_is_available(struct command_handle*, struct hub_user* user);
-
 /**
  * Dispatch a message and forward it as a command.
  * Returns 1 if the message should be forwarded as a chat message, or 0 if
@@ -99,3 +123,22 @@ extern int command_is_available(struct command_handle*, struct hub_user* user);
  * for that command if the sufficient access credentials are met.
  */
 extern int command_invoke(struct command_base*, struct hub_user* user, const char* message);
+
+/**
+ * Parse a message as a command and return a status indicating if the command
+ * is valid and that the arguments are sane.
+ *
+ * @param cbase Command base pointer.
+ * @param user User who invoked the command.
+ * @param message The message that is to be interpreted as a command (including the invokation prefix '!' or '+')
+ *
+ * @return a hub_command that must be freed with command_free(). @See struct hub_command.
+ */
+extern struct hub_command* command_parse(struct command_base* cbase, const struct hub_user* user, const char* message);
+
+/**
+ * Free a hub_command that was created in command_parse().
+ */
+extern void command_free(struct hub_command* command);
+
+#endif /* HAVE_UHUB_COMMANDS_H */

src/core/hub.c

@@ -254,6 +254,7 @@ int hub_handle_password(struct hub_info* hub, struct hub_user* u, struct adc_mes
 int hub_handle_chat_message(struct hub_info* hub, struct hub_user* u, struct adc_message* cmd)
 {
 	char* message = adc_msg_get_argument(cmd, 0);
+	char* message_decoded = NULL;
 	int ret = 0;
 	int relay = 1;
 	int broadcast;
@@ -289,7 +290,9 @@ int hub_handle_chat_message(struct hub_info* hub, struct hub_user* u, struct adc
 		}
 		else
 		{
-			relay = command_invoke(hub->commands, u, message);
+			message_decoded = adc_msg_unescape(message);
+			relay = command_invoke(hub->commands, u, message_decoded);
+			hub_free(message_decoded);
 		}
 	}
 

src/core/inf.c

@@ -546,6 +546,7 @@ static int set_credentials(struct hub_info* hub, struct hub_user* user, struct a
 	{
 		user->credentials = auth_cred_guest;
 	}
+	hub_free(info);
 
 	switch (user->credentials)
 	{

src/core/plugincallback.c

@@ -33,9 +33,9 @@ static struct plugin_callback_data* get_callback_data(struct plugin_handle* plug
 	return data;
 }
 
-static int plugin_command_dispatch(struct command_base* cbase, struct hub_user* user, struct command_handle* handle, struct hub_command* cmd)
+static int plugin_command_dispatch(struct command_base* cbase, struct hub_user* user, struct hub_command* cmd)
 {
-	struct plugin_handle* plugin = (struct plugin_handle*) handle->ptr;
+	struct plugin_handle* plugin = (struct plugin_handle*) cmd->ptr;
 	struct plugin_callback_data* data = get_callback_data(plugin);
 	struct plugin_command_handle* cmdh;
 	struct plugin_user* puser = (struct plugin_user*) user; // FIXME: Use a proper conversion function instead.
@@ -46,9 +46,6 @@ static int plugin_command_dispatch(struct command_base* cbase, struct hub_user*
 	cmdh = (struct plugin_command_handle*) list_get_first(data->commands);
 	while (cmdh)
 	{
-		if (cmdh->length != cmd->prefix_len)
-			continue;
-
 		if (strcmp(cmdh->prefix, cmd->prefix) == 0)
 			return cmdh->handler(plugin, puser, pcommand);
 

src/plugin_api/command_api.h

@@ -31,8 +31,7 @@
 struct plugin_command
 {
 	const char* message;
-	char* prefix;
-	size_t prefix_len;
+	const char* prefix;
 	struct linked_list* args;
 };
 

src/util/memory.h

@@ -35,9 +35,11 @@ extern char* debug_mem_strndup(const char* s, size_t n);
 
 #define hub_malloc     malloc
 #define hub_free       free
+#define hub_realloc    realloc
 #define hub_strdup     strdup
 #define hub_strndup    strndup
 
+
 #endif
 
 extern void* hub_malloc_zero(size_t size);

src/util/misc.c

@@ -281,6 +281,27 @@ int uhub_atoi(const char* value) {
 	return value[0] == '-' ? -val : val;
 }
 
+int is_number(const char* value, int* num)
+{
+	int len = strlen(value);
+	int offset = (value[0] == '-') ? 1 : 0;
+	int val = 0;
+	int i = offset;
+
+	if (!*(value + offset))
+		return 0;
+
+	for (; i < len; i++)
+		if (value[i] > '9' || value[i] < '0')
+			return 0;
+
+	for (i = offset; i< len; i++)
+		val = val*10 + (value[i] - '0');
+	*num = value[0] == '-' ? -val : val;
+
+	return 1;
+}
+
 
 /*
  * FIXME: -INTMIN is wrong!

src/util/misc.h

@@ -36,6 +36,12 @@ extern char* strip_white_space(char* string);
 extern void strip_off_ini_line_comments(char* line, int line_count);
 extern char* strip_off_quotes(char* line);
 
+/**
+ * Convert number in str to integer and store it in num.
+ * @return 1 on success, or 0 on error.
+ */
+extern int is_number(const char* str, int* num);
+
 extern int file_read_lines(const char* file, void* data, file_line_handler_t handler);
 
 /**