Merge pull request #199 from ailin-nemui/config-parser

Make config parser more robust

4 files changed, 60 insertions, 14 deletions

diff --git a/src/lib-config/get.c b/src/lib-config/get.c
index af02b048..53d744da 100644
--- a/src/lib-config/get.c
+++ b/src/lib-config/get.c
@@ -38,12 +38,12 @@ CONFIG_NODE *config_node_find(CONFIG_NODE *node, const char *key)
 	return NULL;
 }
 
-CONFIG_NODE *config_node_section(CONFIG_NODE *parent, const char *key, int new_type)
+CONFIG_NODE *config_node_section(CONFIG_REC *rec, CONFIG_NODE *parent, const char *key, int new_type)
 {
-        return config_node_section_index(parent, key, -1, new_type);
+	return config_node_section_index(rec, parent, key, -1, new_type);
 }
 
-CONFIG_NODE *config_node_section_index(CONFIG_NODE *parent, const char *key,
+CONFIG_NODE *config_node_section_index(CONFIG_REC *rec, CONFIG_NODE *parent, const char *key,
 				       int index, int new_type)
 {
 	CONFIG_NODE *node;
@@ -54,7 +54,6 @@ CONFIG_NODE *config_node_section_index(CONFIG_NODE *parent, const char *key,
 
 	node = key == NULL ? NULL : config_node_find(parent, key);
 	if (node != NULL) {
-		g_return_val_if_fail(new_type == -1 || new_type == node->type, NULL);
 		nindex = g_slist_index(parent->value, node);
 		if (index >= 0 && nindex != index &&
 		    nindex <= g_slist_length(parent->value)) {
@@ -62,7 +61,25 @@ CONFIG_NODE *config_node_section_index(CONFIG_NODE *parent, const char *key,
 			parent->value = g_slist_remove(parent->value, node);
 			parent->value = g_slist_insert(parent->value, node, index);
 		}
-		return node;
+		if (!is_node_list(node)) {
+			int show_error = 0;
+
+			if (new_type != -1) {
+				config_node_remove(rec, parent, node);
+				node = NULL;
+				show_error = 1;
+			} else if (!g_hash_table_contains(rec->cache_nodes, node)) {
+				g_hash_table_insert(rec->cache_nodes, node, NULL);
+				show_error = 1;
+			}
+			if (show_error)
+				g_critical("Expected %s node at `..%s/%s' was of scalar type. Corrupt config?",
+						   new_type == NODE_TYPE_LIST ? "list" : new_type == NODE_TYPE_BLOCK ? "block" : "section",
+						   parent->key, key);
+		} else {
+			g_return_val_if_fail(new_type == -1 || new_type == node->type, NULL);
+			return node;
+		}
 	}
 
 	if (new_type == -1)
@@ -91,7 +108,21 @@ CONFIG_NODE *config_node_traverse(CONFIG_REC *rec, const char *section, int crea
 
 	/* check if it already exists in cache */
 	node = g_hash_table_lookup(rec->cache, section);
-	if (node != NULL) return node;
+	if (node != NULL) {
+		if (create) {
+			const char *path = strrchr(section, '/');
+			if (path == NULL) path = section;
+			else path++;
+			new_type = *path == '(' ? NODE_TYPE_LIST : NODE_TYPE_BLOCK;
+			if (node->type != new_type) {
+				g_critical("Expected %s node at `%s' was of %s type. Corrupt config?",
+						   new_type == NODE_TYPE_LIST ? "list" : "block", section,
+						   node->type == NODE_TYPE_LIST ? "list" : "block");
+				node->type = new_type;
+			}
+		}
+		return node;
+	}
 
         new_type = -1;
 
@@ -99,9 +130,18 @@ CONFIG_NODE *config_node_traverse(CONFIG_REC *rec, const char *section, int crea
 	list = g_strsplit(section, "/", -1);
 	for (tmp = list; *tmp != NULL; tmp++) {
 		is_list = **tmp == '(';
-		if (create) new_type = is_list ? NODE_TYPE_LIST : NODE_TYPE_BLOCK;
+		if (create) {
+			CONFIG_NODE *tmpnode;
+
+			new_type = is_list ? NODE_TYPE_LIST : NODE_TYPE_BLOCK;
+			tmpnode = config_node_find(node, *tmp + is_list);
+			if (tmpnode != NULL && tmpnode->type != new_type) {
+				g_critical("Expected %s node at `%s' was of scalar type. Corrupt config?", is_list ? "list" : "block", section);
+				config_node_remove(rec, node, tmpnode);
+			}
+		}
 
-		node = config_node_section(node, *tmp + is_list, new_type);
+		node = config_node_section(rec, node, *tmp + is_list, new_type);
 		if (node == NULL) {
 			g_strfreev(list);
 			return NULL;
@@ -109,9 +149,9 @@ CONFIG_NODE *config_node_traverse(CONFIG_REC *rec, const char *section, int crea
 	}
 	g_strfreev(list);
 
-        if (!is_node_list(node)) {
+	if (!is_node_list(node)) {
 		/* Will die. Better to not corrupt the config further in this case. */
-		g_error("Attempt to use non-list node as list. Corrupt config?");
+		g_critical("Attempt to use non-list node `%s' as list. Corrupt config?", section);
 		return NULL;
 	}
 
diff --git a/src/lib-config/iconfig.h b/src/lib-config/iconfig.h
index 3d9eb931..91583e40 100644
--- a/src/lib-config/iconfig.h
+++ b/src/lib-config/iconfig.h
@@ -116,8 +116,8 @@ int config_set_bool(CONFIG_REC *rec, const char *section, const char *key, int v
 CONFIG_NODE *config_node_find(CONFIG_NODE *node, const char *key);
 /* Find the section from node - if not found create it unless new_type is -1.
    You can also specify in new_type if it's NODE_TYPE_LIST or NODE_TYPE_BLOCK */
-CONFIG_NODE *config_node_section(CONFIG_NODE *parent, const char *key, int new_type);
-CONFIG_NODE *config_node_section_index(CONFIG_NODE *parent, const char *key,
+CONFIG_NODE *config_node_section(CONFIG_REC *rec, CONFIG_NODE *parent, const char *key, int new_type);
+CONFIG_NODE *config_node_section_index(CONFIG_REC *rec, CONFIG_NODE *parent, const char *key,
 				       int index, int new_type);
 /* Find the section with the whole path.
    Create the path if necessary if `create' is TRUE. */
diff --git a/src/lib-config/parse.c b/src/lib-config/parse.c
index 1b20195a..c106fc46 100644
--- a/src/lib-config/parse.c
+++ b/src/lib-config/parse.c
@@ -173,7 +173,7 @@ static GTokenType config_parse_symbol(CONFIG_REC *rec, CONFIG_NODE *node)
 		if (key == NULL && node->type != NODE_TYPE_LIST)
 			return G_TOKEN_ERROR;
 
-		newnode = config_node_section(node, key, NODE_TYPE_BLOCK);
+		newnode = config_node_section(rec, node, key, NODE_TYPE_BLOCK);
 		config_parse_loop(rec, newnode, (GTokenType) '}');
 		g_free_not_null(key);
 
@@ -188,7 +188,7 @@ static GTokenType config_parse_symbol(CONFIG_REC *rec, CONFIG_NODE *node)
 		/* list */
 		if (key == NULL)
 			return G_TOKEN_ERROR;
-		newnode = config_node_section(node, key, NODE_TYPE_LIST);
+		newnode = config_node_section(rec, node, key, NODE_TYPE_LIST);
 		config_parse_loop(rec, newnode, (GTokenType) ')');
 		g_free_not_null(key);
 
diff --git a/src/lib-config/set.c b/src/lib-config/set.c
index 3d6001e1..7ca55871 100644
--- a/src/lib-config/set.c
+++ b/src/lib-config/set.c
@@ -104,6 +104,12 @@ void config_node_set_str(CONFIG_REC *rec, CONFIG_NODE *parent, const char *key,
 		return;
 	}
 
+	if (node != NULL && !has_node_value(node)) {
+		g_critical("Expected scalar node at `..%s/%s' was of complex type. Corrupt config?",
+				   parent->key, key);
+		config_node_remove(rec, parent, node);
+		node = NULL;
+	}
 	if (node != NULL) {
 		if (g_strcmp0(node->value, value) == 0)
 			return;