diff --git a/ChangeLog.txt b/ChangeLog.txt index 0a237e56..9fa188b4 100644 --- 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 index 80793940..139bea34 100644 --- 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 index dac546b6..8b59a633 100644 --- 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 index 1770ac2f..b62b971a 100644 --- 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)