Skip to content

Commit

Permalink
Merge pull request #681 from redboltz/fix_wildcard
Browse files Browse the repository at this point in the history 
Fixed wildcard bugs.
  • Loading branch information
@redboltz
redboltz authored Oct 14, 2020
2 parents 224e5c0 + 18033c7 commit c4d5ae5
Showing 1 changed file with 156 additions and 31 deletions.
187 changes: 156 additions & 31 deletions test/test_broker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,41 +32,68 @@ using con_sp_t = std::shared_ptr<endpoint_t>;
using con_wp_t = std::weak_ptr<endpoint_t>;
using packet_id_t = endpoint_t::packet_id_t;

inline bool validate_topic_pattern(MQTT_NS::string_view topicPattern)
{
#if defined(MQTT_STD_STRING_VIEW)
#define MQTT_STRING_VIEW_CONSTEXPR constexpr
#else // defined(MQTT_STD_STRING_VIEW)
#define MQTT_STRING_VIEW_CONSTEXPR
#endif // defined(MQTT_STD_STRING_VIEW)


// TODO: Technically this function is simply wrong, since it's treating the
// topic pattern as if it were an ASCII sequence.
// To make this function correct per the standard, it would be necessary
// to conduct the search for the wildcard characters using a proper
// UTF-8 API to avoid problems of interpreting parts of multi-byte characters
// as if they were individual ASCII characters
MQTT_STRING_VIEW_CONSTEXPR
bool validate_topic_filter(MQTT_NS::string_view topic_filter) {
/*
* Confirm the topic pattern is valid before registering it.
* Use rules from http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718106
*/
for(size_t idx = topicPattern.find_first_of("+#");
MQTT_NS::string_view::npos != idx;
idx = topicPattern.find_first_of("+#", idx+1)) {
BOOST_ASSERT( ('+' == topicPattern[idx])
|| ('#' == topicPattern[idx]));
if('+' == topicPattern[idx]) {

// All Topic Names and Topic Filters MUST be at least one character long
// Topic Names and Topic Filters are UTF-8 Encoded Strings; they MUST NOT encode to more than 65,535 bytes
if (topic_filter.empty() || (topic_filter.size() > std::numeric_limits<std::uint16_t>::max())) {
return false;
}

for (MQTT_NS::string_view::size_type idx = topic_filter.find_first_of(MQTT_NS::string_view("\0+#", 3));
MQTT_NS::string_view::npos != idx;
idx = topic_filter.find_first_of(MQTT_NS::string_view("\0+#", 3), idx+1)) {
BOOST_ASSERT(
('\0' == topic_filter[idx])
|| ('+' == topic_filter[idx])
|| ('#' == topic_filter[idx])
);
if ('\0' == topic_filter[idx]) {
// Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)
return false;
}
else if ('+' == topic_filter[idx]) {
/*
* Either must be the first character,
* or be preceeded by a topic seperator.
*/
if((0 != idx) && ('/' != topicPattern[idx-1])) {
if ((0 != idx) && ('/' != topic_filter[idx-1])) {
return false;
}

/*
* Either must be the last character,
* or be followed by a topic seperator.
*/
if((topicPattern.size()-1 != idx) && ('/' != topicPattern[idx+1])) {
if ((topic_filter.size()-1 != idx) && ('/' != topic_filter[idx+1])) {
return false;
}
}
// multilevel wildcard
else {
else if ('#' == topic_filter[idx]) {
/*
* Must be absolute last character.
* Must only be one multi level wild card.
*/
if(idx != topicPattern.size()-1) {
if (idx != topic_filter.size()-1) {
return false;
}

Expand All @@ -75,27 +102,103 @@ inline bool validate_topic_pattern(MQTT_NS::string_view topicPattern)
* immediately preceeding character must
* be a topic level separator.
*/
if((0 != idx) && ('/' != topicPattern[idx-1])) {
if ((0 != idx) && ('/' != topic_filter[idx-1])) {
return false;
}
}
else {
return false;
}
}
return true;
}

inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::string_view topic)
{
BOOST_ASSERT(validate_topic_pattern(topicPattern));
for(size_t idx = topicPattern.find_first_of("+#");
MQTT_NS::string_view::npos != idx;
idx = topicPattern.find_first_of("+#")) {
BOOST_ASSERT( ('+' == topicPattern[idx])
|| ('#' == topicPattern[idx]));
if('+' == topicPattern[idx]) {
#if defined(MQTT_STD_STRING_VIEW)
// The following rules come from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
static_assert( ! validate_topic_filter(""), "All Topic Names and Topic Filters MUST be at least one character long");
static_assert(validate_topic_filter("/"), "A Topic Name or Topic Filter consisting only of the ‘/’ character is valid");
static_assert( ! validate_topic_filter(MQTT_NS::string_view("\0", 1)), "Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)");
static_assert(validate_topic_filter(" "), "Topic Names and Topic Filters can include the space character");
static_assert(validate_topic_filter("/////"), "Topic level separators can appear anywhere in a Topic Filter or Topic Name. Adjacent Topic level separators indicate a zero-length topic level");
static_assert(validate_topic_filter("#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert(validate_topic_filter("/#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert(validate_topic_filter("+/#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert( ! validate_topic_filter("+#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert( ! validate_topic_filter("++"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert( ! validate_topic_filter("f#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
static_assert( ! validate_topic_filter("#/"), "In either case the multi-level wildcard character MUST be the last character specified in the Topic Filter");

static_assert(validate_topic_filter("+"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
static_assert(validate_topic_filter("+/bob/alice/sue"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
static_assert(validate_topic_filter("bob/alice/sue/+"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
static_assert(validate_topic_filter("+/bob/alice/sue/+"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
static_assert(validate_topic_filter("+/bob/+/sue/+"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
static_assert(validate_topic_filter("+/bob/+/sue/#"), "The single-level wildcard can be used at more than one level in the Topic Filter and can be used in conjunction with the multi-level wildcard");
static_assert( ! validate_topic_filter("+a"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
static_assert( ! validate_topic_filter("a+"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
static_assert( ! validate_topic_filter("/a+"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
static_assert( ! validate_topic_filter("a+/"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
static_assert( ! validate_topic_filter("/a+/"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
#endif // defined(MQTT_STD_STRING_VIEW)

MQTT_STRING_VIEW_CONSTEXPR
bool validate_topic_name(MQTT_NS::string_view topic_name) {
/*
* Confirm the topic name is valid
* Use rules from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
*/

// All Topic Names and Topic Filters MUST be at least one character long
// Topic Names and Topic Filters are UTF-8 Encoded Strings; they MUST NOT encode to more than 65,535 bytes
// The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name
// Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)
return
! topic_name.empty()
&& (topic_name.size() <= std::numeric_limits<std::uint16_t>::max())
&& (MQTT_NS::string_view::npos == topic_name.find_first_of(MQTT_NS::string_view("\0+#", 3)));
}

#if defined(MQTT_STD_STRING_VIEW)
// The following rules come from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
static_assert( ! validate_topic_name(""), "All Topic Names and Topic Filters MUST be at least one character long");
static_assert(validate_topic_name("/"), "A Topic Name or Topic Filter consisting only of the ‘/’ character is valid");
static_assert( ! validate_topic_name(MQTT_NS::string_view("\0", 1)), "Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)");
static_assert(validate_topic_name(" "), "Topic Names and Topic Filters can include the space character");
static_assert(validate_topic_name("/////"), "Topic level separators can appear anywhere in a Topic Filter or Topic Name. Adjacent Topic level separators indicate a zero-length topic level");
static_assert( ! validate_topic_name("#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
static_assert( ! validate_topic_name("+"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
static_assert( ! validate_topic_name("/#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
static_assert( ! validate_topic_name("+/#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
static_assert( ! validate_topic_name("f#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
static_assert( ! validate_topic_name("#/"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
#endif // defined(MQTT_STD_STRING_VIEW)

MQTT_STRING_VIEW_CONSTEXPR
bool compare_topic_filter(MQTT_NS::string_view topic_filter, MQTT_NS::string_view topic_name) {
if ( ! validate_topic_filter(topic_filter)) {
BOOST_ASSERT(validate_topic_filter(topic_filter));
return false;
}

if ( ! validate_topic_name(topic_name)) {
BOOST_ASSERT(validate_topic_name(topic_name));
return false;
}

// TODO: The Server MUST NOT match Topic Filters starting with a wildcard character (# or +) with Topic Names beginning with a $ character
for (MQTT_NS::string_view::size_type idx = topic_filter.find_first_of("+#");
MQTT_NS::string_view::npos != idx;
idx = topic_filter.find_first_of("+#")) {
BOOST_ASSERT(
('+' == topic_filter[idx])
|| ('#' == topic_filter[idx])
);

if ('+' == topic_filter[idx]) {
// Compare everything up to the first +
if(topicPattern.substr(0, idx) == topic.substr(0, idx)) {
if(topic_filter.substr(0, idx) == topic_name.substr(0, idx)) {
/*
* We already know thanks to the topic pattern being validated
* We already know thanks to the topic filter being validated
* that the + symbol is directly touching '/'s on both sides
* (if not the first or last character), so we don't need to
* double check that.
Expand All @@ -104,12 +207,12 @@ inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::st
* the loop continue, we get the proper comparison of the '/'s
* automatically when the loop continues.
*/
topicPattern.remove_prefix(idx+1);
topic_filter.remove_prefix(idx+1);
/*
* It's a bit more complicated for the incoming topic though
* as we need to remove everything up to the next seperator.
*/
topic.remove_prefix(topic.find('/', idx));
topic_name.remove_prefix(topic_name.find('/', idx));
}
else {
return false;
Expand All @@ -121,14 +224,36 @@ inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::st
* Compare up to where the multilevel wild card is found
* and then anything after that matches the wildcard.
*/
return topicPattern.substr(0, idx) == topic.substr(0, idx);
return topic_filter.substr(0, idx) == topic_name.substr(0, idx);
}
}

// No + or # found in the remaining topic pattern. Just do a string compare.
return topicPattern == topic;
// No + or # found in the remaining topic filter. Just do a string compare.
return topic_filter == topic_name;
}

#if defined(MQTT_STD_STRING_VIEW)
static_assert(compare_topic_filter("bob", "bob"), "Topic Names and Topic Filters are case sensitive");
static_assert( ! compare_topic_filter("Bob", "bob"), "Topic Names and Topic Filters are case sensitive");
static_assert( ! compare_topic_filter("bob", "boB"), "Topic Names and Topic Filters are case sensitive");
static_assert( ! compare_topic_filter("/bob", "bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
static_assert( ! compare_topic_filter("bob/", "bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
static_assert( ! compare_topic_filter("bob", "/bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
static_assert( ! compare_topic_filter("bob", "bob/"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
static_assert(compare_topic_filter("bob/alice", "bob/alice"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/alice/sue", "bob/alice/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob//////sue", "bob//////sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/#", "bob//////sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert( ! compare_topic_filter("bob///#", "bob/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/+/sue", "bob/alice/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert( ! compare_topic_filter("bob/+/sue", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("#", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/#", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/alice/#", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert(compare_topic_filter("bob/alice/mary/#", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
static_assert( ! compare_topic_filter("bob/alice/mary/sue/#", "bob/alice/mary/sue"), "Each non-wildcarded level in the Topic Filter has to match the corresponding level in the Topic Name character for character for the match to succeed");
#endif // defined(MQTT_STD_STRING_VIEW)

class test_broker {
public:
test_broker(as::io_context& ioc)
Expand Down Expand Up @@ -924,7 +1049,7 @@ class test_broker {
for( auto const& retain : retains_) {
MQTT_NS::buffer const& topic = std::get<0>(e);
MQTT_NS::subscribe_options options = std::get<1>(e);
if (compare_topic_pattern(topic, retain.topic)) {
if (compare_topic_filter(topic, retain.topic)) {
auto rh = options.get_retain_handling();
if (rh == MQTT_NS::retain_handling::send ||
(rh == MQTT_NS::retain_handling::send_only_new_subscription && *new_sub_it)) {
Expand Down Expand Up @@ -1030,7 +1155,7 @@ class test_broker {
// For each active subscription registered for this topic
auto& idx = col.template get<tag_topic>();
for(auto& item : idx) {
if(compare_topic_pattern(item.topic, topic)) {
if(compare_topic_filter(item.topic, topic)) {
// publish the message to subscribers.
// retain is delivered as the original only if rap_value is rap::retain.
// On MQTT v3.1.1, rap_value is always rap::dont.
Expand Down

0 comments on commit c4d5ae5

Please sign in to comment.