Fix mosquitto_topic_matches_sub().

eclipse · Jun 2, 2016 · 30686f2 · 30686f2
1 parent ea2baa8
commit 30686f2
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 3 deletions.
diff --git a/ChangeLog.txt b/ChangeLog.txt
@@ -34,6 +34,8 @@ Client library:
   libmosquitto. Closes #166.
 - Clients can now cope with unknown incoming PUBACK, PUBREC, PUBREL, PUBCOMP
   without disconnecting. Closes #57.
+- Fix mosquitto_topic_matches_sub() reporting matches on some invalid
+   subscriptions.
 
 Clients:
 - Handle some unchecked malloc() calls. Closes #1.

diff --git a/lib/util_mosq.c b/lib/util_mosq.c
@@ -228,6 +228,11 @@ int mosquitto_topic_matches_sub(const char *sub, const char *topic, bool *result
 	slen = strlen(sub);
 	tlen = strlen(topic);
 
+	if(!slen || !tlen){
+		*result = false;
+		return MOSQ_ERR_INVAL;
+	}
+
 	if(slen && tlen){
 		if((sub[0] == '$' && topic[0] != '$')
 				|| (topic[0] == '$' && sub[0] != '$')){
@@ -240,7 +245,7 @@ int mosquitto_topic_matches_sub(const char *sub, const char *topic, bool *result
 	spos = 0;
 	tpos = 0;
 
-	while(spos < slen && tpos < tlen){
+	while(spos < slen && tpos <= tlen){
 		if(sub[spos] == topic[tpos]){
 			if(tpos == tlen-1){
 				/* Check for e.g. foo matching foo/# */
@@ -258,12 +263,26 @@ int mosquitto_topic_matches_sub(const char *sub, const char *topic, bool *result
 				*result = true;
 				return MOSQ_ERR_SUCCESS;
 			}else if(tpos == tlen && spos == slen-1 && sub[spos] == '+'){
+				if(spos > 0 && sub[spos-1] != '/'){
+					*result = false;
+					return MOSQ_ERR_INVAL;
+				}
 				spos++;
 				*result = true;
 				return MOSQ_ERR_SUCCESS;
 			}
 		}else{
 			if(sub[spos] == '+'){
+				/* Check for bad "+foo" or "a/+foo" subscription */
+				if(spos > 0 && sub[spos-1] != '/'){
+					*result = false;
+					return MOSQ_ERR_INVAL;
+				}
+				/* Check for bad "foo+" or "foo+/a" subscription */
+				if(spos < slen-1 && sub[spos+1] != '/'){
+					*result = false;
+					return MOSQ_ERR_INVAL;
+				}
 				spos++;
 				while(tpos < tlen && topic[tpos] != '/'){
 					tpos++;
@@ -273,10 +292,14 @@ int mosquitto_topic_matches_sub(const char *sub, const char *topic, bool *result
 					return MOSQ_ERR_SUCCESS;
 				}
 			}else if(sub[spos] == '#'){
+				if(spos > 0 && sub[spos-1] != '/'){
+					*result = false;
+					return MOSQ_ERR_INVAL;
+				}
 				multilevel_wildcard = true;
 				if(spos+1 != slen){
 					*result = false;
-					return MOSQ_ERR_SUCCESS;
+					return MOSQ_ERR_INVAL;
 				}else{
 					*result = true;
 					return MOSQ_ERR_SUCCESS;

diff --git a/test/lib/c/09-util-topic-matching.c b/test/lib/c/09-util-topic-matching.c
@@ -16,6 +16,13 @@ void do_check(const char *sub, const char *topic, bool bad_res)
 
 int main(int argc, char *argv[])
 {
+	do_check("foo/#", "foo/", false);
+	do_check("foo#", "foo", true);
+	do_check("fo#o/", "foo", true);
+	do_check("foo#", "fooa", true);
+	do_check("foo+", "foo", true);
+	do_check("foo+", "fooa", true);
+
 	do_check("test/6/#", "test/3", true);
 	do_check("foo/bar", "foo/bar", false);
 	do_check("foo/+", "foo/bar", false);

diff --git a/test/mosq_test.py b/test/mosq_test.py
@@ -10,7 +10,7 @@ def start_broker(filename, cmd=None, port=1888):
     if cmd is None:
         cmd = ['../../src/mosquitto', '-v', '-c', filename.replace('.py', '.conf')]
     if os.environ.get('MOSQ_USE_VALGRIND') is not None:
-        cmd = ['valgrind', '-q', '--log-file='+filename+'.vglog'] + cmd
+        cmd = ['valgrind', '--trace-children=yes', '-v', '--log-file='+filename+'.vglog'] + cmd
         delay = 1
 
     broker = subprocess.Popen(cmd, stderr=subprocess.PIPE)