Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hierarchical topics not handled correctly #41

Closed
mtappler opened this issue May 31, 2016 · 1 comment
Closed

Hierarchical topics not handled correctly #41

mtappler opened this issue May 31, 2016 · 1 comment

Comments

@mtappler
Copy link

mtappler commented May 31, 2016

Hierarchical topics are not handled correctly. More concretely, if I subscribe to a topic at a high level in the hierarchy I will also receive messages published at a lower level.

I tested the following simple example:

  1. Connect with Client C1 and receive ConnAck
  2. Subscribe with C1 to topic "my_topic" and receive SubAck
  3. Publish message "my_message" with C1 and QoS 1 to "my_topic/other_topic" and receive PubAck and additionally a Publish-message for "my_topic/other_topic" with message "my_message"

Thus, the subscription to "my_topic" works like a subscription to "my_topic/#". I would interpret this as a violation of the MQTT-standard and I also checked with emqtt and mosquitto, which would not send a Publish in response to the last step.

A quick fix would be to change Line 623 from
match_pattern = re.compile(a_filter.replace('#', '.*').replace('$', '\$').replace('+', '[/\$\s\w\d]+'))
to
match_pattern = re.compile("^"+a_filter.replace('#', '.*').replace('$', '\$').replace('+', '[/\$\s\w\d]+')+"$")

However, I am neither a Python-expert nor did I test the fix thoroughly.

@njouanin njouanin added this to the 0.7.2 milestone May 31, 2016
@mtappler
Copy link
Author

mtappler commented Jun 1, 2016

Thank you for fixing the issue so fast!

I do not know whether I should open another issue as this is kind of related, but I will do so if necessary. There is another problem, which existed before and persists with the fix committed yesterday. The single-level wildcards denoted by + should not match more than one level, but the replacement-regex allows matching strings as well as level-separators /, so I guess replace('+', '[/\$\s\w\d]+') should be changed to something like replace('+', '[^/\$\s\w\d]+'), i.e. it should only match characters unequal to a /.
I should note that I did not evaluate this fix thoroughly either. I just checked with some string and did not test special cases like zero length topic names.

However, by doing so, the issue raised yesterday would still persist I think. Since the regex would partially match, by subscribing to a/+/b, one would also receive messages published for a/something/b/anotherthing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants