From 41abb3e644cfd127e81c963e0d940a81ff1f5f25 Mon Sep 17 00:00:00 2001
From: Stefan Knoblich <stkn@openisdn.net>
Date: Wed, 21 Sep 2011 15:50:57 +0200
Subject: [PATCH] mod_blacklist: Resource leak fixes, config checks and add
 help output for api interface

Don't leak the file handle after parsing the configuration
Don't leak memory if the file could not be opened

Check list name and filename attribute of configuration entry.

Add "blacklist help" and error messages to api interface.

Signed-off-by: Stefan Knoblich <stkn@openisdn.net>
---
 conf/autoload_configs/blacklist.conf.xml      |  6 ++-
 .../mod_blacklist/mod_blacklist.c             | 45 +++++++++++++------
 2 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/conf/autoload_configs/blacklist.conf.xml b/conf/autoload_configs/blacklist.conf.xml
index d6a810cc6f..a981ebbdb0 100644
--- a/conf/autoload_configs/blacklist.conf.xml
+++ b/conf/autoload_configs/blacklist.conf.xml
@@ -1,6 +1,10 @@
 <configuration name="mod_blacklist.conf" description="Blacklist module">
 <lists>
-	<!-- Example blacklist, the referenced file contains blacklisted items, one entry per line
+	<!--
+		Example blacklist, the referenced file contains blacklisted items, one entry per line
+
+		NOTE: make sure the file exists and is readable by FreeSWITCH.
+
 	<list name="example" filename="/usr/local/freeswitch/conf/blacklists/example.list"/>
 	-->
 </lists>
diff --git a/src/mod/applications/mod_blacklist/mod_blacklist.c b/src/mod/applications/mod_blacklist/mod_blacklist.c
index a8fe5e6da5..ab9f7f6c8c 100644
--- a/src/mod/applications/mod_blacklist/mod_blacklist.c
+++ b/src/mod/applications/mod_blacklist/mod_blacklist.c
@@ -86,24 +86,25 @@ void trim(char *string)
 static switch_status_t load_list(const char *name, const char *filename)
 {
 	FILE *f;
-	blacklist_t *bl = blacklist_create(name);
-	/* Create a hashtable + mutex for that list */
-	
+
 	if ((f = fopen(filename, "r"))) {
 		char buf[1024] = {0};
+		blacklist_t *bl = blacklist_create(name);	/* Create a hashtable + mutex for that list */
+
 		while (fgets(buf, 1024, f)) {
 			trim(buf);
-			switch_core_hash_insert(bl->list, buf, (void*)SWITCH_TRUE);
+			switch_core_hash_insert(bl->list, buf, (void *)SWITCH_TRUE);
 		}
+
+		switch_core_hash_insert(globals.lists, name, bl);
+		switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_INFO, "Loaded list [%s]\n", name);
+
+		fclose(f);
 	} else {
 		switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "Couldn't open [%s] to load list [%s]\n", filename, name);
 		return SWITCH_STATUS_FALSE;
 	}
-	
-	switch_core_hash_insert(globals.lists, name, bl);
-	switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_INFO, "Loaded list [%s]\n", name);
-	
-	
+
 	return SWITCH_STATUS_SUCCESS;
 }
 
@@ -133,10 +134,17 @@ static switch_status_t do_config(switch_bool_t reload)
 		for (list = switch_xml_child(lists, "list"); list; list = list->next) {
 			const char *name = switch_xml_attr_soft(list, "name");
 			const char *filename = switch_xml_attr_soft(list, "filename");
-			
-			if (name && filename) {
-				load_list(name, filename);
+
+			if (zstr(name)) {
+				switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "list has no name\n");
+				continue;
 			}
+			if (zstr(filename)) {
+				switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR, "list [%s] has no filename\n", name);
+				continue;
+			}
+
+			load_list(name, filename);
 		}
 	}
 	
@@ -150,6 +158,13 @@ static switch_status_t do_config(switch_bool_t reload)
 	return SWITCH_STATUS_SUCCESS;
 }
 
+#define BLACKLIST_API_SYNTAX	\
+	"blacklist check <listname> <item>\n"	\
+	"blacklist add <listname> <item>\n"	\
+	"blacklist del <listname> <item>\n"	\
+	"blacklist reload\n"			\
+	"blacklist help\n"
+
 SWITCH_STANDARD_API(blacklist_api_function)
 {
 	char *data;
@@ -209,7 +224,7 @@ SWITCH_STANDARD_API(blacklist_api_function)
 		switch_mutex_unlock(bl->list_mutex);
 		stream->write_function(stream, "+OK\n");
 	} else if (!strcasecmp(argv[0], "del")) {
-			blacklist_t *bl = NULL;
+		blacklist_t *bl = NULL;
 		if (argc < 2 || zstr(argv[1]) || zstr(argv[2])) {
 			switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_DEBUG, "Wrong syntax");
 			goto done;
@@ -233,6 +248,10 @@ SWITCH_STANDARD_API(blacklist_api_function)
 	} else if (!strcasecmp(argv[0], "reload"))  {
 		do_config(SWITCH_TRUE);
 		stream->write_function(stream, "+OK\n");
+	} else if (!strcasecmp(argv[0], "help")) {
+		stream->write_function(stream, BLACKLIST_API_SYNTAX "+OK\n");
+	} else if (!zstr(argv[0])) {
+		stream->write_function(stream, "-ERR: No such command: %s (see 'blacklist help')\n", argv[0]);
 	}
 	
 done: