Skip to content

Commit 18033c7

Browse files
redboltzredboltz
committed
Fixed wildcard bugs.
It seems that #672 contains not only shared subscription support (incomplete) but also wildcard bug fix. This is the part of wildcard bug fix.
1 parent 224e5c0 commit 18033c7

File tree

1 file changed

+156
-31
lines changed

1 file changed

+156
-31
lines changed

‎test/test_broker.hpp

+156-31
Original file line numberDiff line numberDiff line change
@@ -32,41 +32,68 @@ using con_sp_t = std::shared_ptr<endpoint_t>;
3232
using con_wp_t = std::weak_ptr<endpoint_t>;
3333
using packet_id_t = endpoint_t::packet_id_t;
3434

35-
inline bool validate_topic_pattern(MQTT_NS::string_view topicPattern)
36-
{
35+
#if defined(MQTT_STD_STRING_VIEW)
36+
#define MQTT_STRING_VIEW_CONSTEXPR constexpr
37+
#else // defined(MQTT_STD_STRING_VIEW)
38+
#define MQTT_STRING_VIEW_CONSTEXPR
39+
#endif // defined(MQTT_STD_STRING_VIEW)
40+
41+
42+
// TODO: Technically this function is simply wrong, since it's treating the
43+
// topic pattern as if it were an ASCII sequence.
44+
// To make this function correct per the standard, it would be necessary
45+
// to conduct the search for the wildcard characters using a proper
46+
// UTF-8 API to avoid problems of interpreting parts of multi-byte characters
47+
// as if they were individual ASCII characters
48+
MQTT_STRING_VIEW_CONSTEXPR
49+
bool validate_topic_filter(MQTT_NS::string_view topic_filter) {
3750
/*
3851
* Confirm the topic pattern is valid before registering it.
3952
* Use rules from http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718106
4053
*/
41-
for(size_t idx = topicPattern.find_first_of("+#");
42-
MQTT_NS::string_view::npos != idx;
43-
idx = topicPattern.find_first_of("+#", idx+1)) {
44-
BOOST_ASSERT( ('+' == topicPattern[idx])
45-
|| ('#' == topicPattern[idx]));
46-
if('+' == topicPattern[idx]) {
54+
55+
// All Topic Names and Topic Filters MUST be at least one character long
56+
// Topic Names and Topic Filters are UTF-8 Encoded Strings; they MUST NOT encode to more than 65,535 bytes
57+
if (topic_filter.empty() || (topic_filter.size() > std::numeric_limits<std::uint16_t>::max())) {
58+
return false;
59+
}
60+
61+
for (MQTT_NS::string_view::size_type idx = topic_filter.find_first_of(MQTT_NS::string_view("\0+#", 3));
62+
MQTT_NS::string_view::npos != idx;
63+
idx = topic_filter.find_first_of(MQTT_NS::string_view("\0+#", 3), idx+1)) {
64+
BOOST_ASSERT(
65+
('\0' == topic_filter[idx])
66+
|| ('+' == topic_filter[idx])
67+
|| ('#' == topic_filter[idx])
68+
);
69+
if ('\0' == topic_filter[idx]) {
70+
// Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)
71+
return false;
72+
}
73+
else if ('+' == topic_filter[idx]) {
4774
/*
4875
* Either must be the first character,
4976
* or be preceeded by a topic seperator.
5077
*/
51-
if((0 != idx) && ('/' != topicPattern[idx-1])) {
78+
if ((0 != idx) && ('/' != topic_filter[idx-1])) {
5279
return false;
5380
}
5481

5582
/*
5683
* Either must be the last character,
5784
* or be followed by a topic seperator.
5885
*/
59-
if((topicPattern.size()-1 != idx) && ('/' != topicPattern[idx+1])) {
86+
if ((topic_filter.size()-1 != idx) && ('/' != topic_filter[idx+1])) {
6087
return false;
6188
}
6289
}
6390
// multilevel wildcard
64-
else {
91+
else if ('#' == topic_filter[idx]) {
6592
/*
6693
* Must be absolute last character.
6794
* Must only be one multi level wild card.
6895
*/
69-
if(idx != topicPattern.size()-1) {
96+
if (idx != topic_filter.size()-1) {
7097
return false;
7198
}
7299

@@ -75,27 +102,103 @@ inline bool validate_topic_pattern(MQTT_NS::string_view topicPattern)
75102
* immediately preceeding character must
76103
* be a topic level separator.
77104
*/
78-
if((0 != idx) && ('/' != topicPattern[idx-1])) {
105+
if ((0 != idx) && ('/' != topic_filter[idx-1])) {
79106
return false;
80107
}
81108
}
109+
else {
110+
return false;
111+
}
82112
}
83113
return true;
84114
}
85115

86-
inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::string_view topic)
87-
{
88-
BOOST_ASSERT(validate_topic_pattern(topicPattern));
89-
for(size_t idx = topicPattern.find_first_of("+#");
90-
MQTT_NS::string_view::npos != idx;
91-
idx = topicPattern.find_first_of("+#")) {
92-
BOOST_ASSERT( ('+' == topicPattern[idx])
93-
|| ('#' == topicPattern[idx]));
94-
if('+' == topicPattern[idx]) {
116+
#if defined(MQTT_STD_STRING_VIEW)
117+
// The following rules come from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
118+
static_assert( ! validate_topic_filter(""), "All Topic Names and Topic Filters MUST be at least one character long");
119+
static_assert(validate_topic_filter("/"), "A Topic Name or Topic Filter consisting only of the ‘/’ character is valid");
120+
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)");
121+
static_assert(validate_topic_filter(" "), "Topic Names and Topic Filters can include the space character");
122+
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");
123+
static_assert(validate_topic_filter("#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
124+
static_assert(validate_topic_filter("/#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
125+
static_assert(validate_topic_filter("+/#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
126+
static_assert( ! validate_topic_filter("+#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
127+
static_assert( ! validate_topic_filter("++"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
128+
static_assert( ! validate_topic_filter("f#"), "The multi-level wildcard character MUST be specified either on its own or following a topic level separator");
129+
static_assert( ! validate_topic_filter("#/"), "In either case the multi-level wildcard character MUST be the last character specified in the Topic Filter");
130+
131+
static_assert(validate_topic_filter("+"), "The single-level wildcard can be used at any level in the Topic Filter, including first and last levels");
132+
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");
133+
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");
134+
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");
135+
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");
136+
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");
137+
static_assert( ! validate_topic_filter("+a"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
138+
static_assert( ! validate_topic_filter("a+"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
139+
static_assert( ! validate_topic_filter("/a+"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
140+
static_assert( ! validate_topic_filter("a+/"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
141+
static_assert( ! validate_topic_filter("/a+/"), "Where it is used, the single-level wildcard MUST occupy an entire level of the filter.");
142+
#endif // defined(MQTT_STD_STRING_VIEW)
143+
144+
MQTT_STRING_VIEW_CONSTEXPR
145+
bool validate_topic_name(MQTT_NS::string_view topic_name) {
146+
/*
147+
* Confirm the topic name is valid
148+
* Use rules from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
149+
*/
150+
151+
// All Topic Names and Topic Filters MUST be at least one character long
152+
// Topic Names and Topic Filters are UTF-8 Encoded Strings; they MUST NOT encode to more than 65,535 bytes
153+
// The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name
154+
// Topic Names and Topic Filters MUST NOT include the null character (Unicode U+0000)
155+
return
156+
! topic_name.empty()
157+
&& (topic_name.size() <= std::numeric_limits<std::uint16_t>::max())
158+
&& (MQTT_NS::string_view::npos == topic_name.find_first_of(MQTT_NS::string_view("\0+#", 3)));
159+
}
160+
161+
#if defined(MQTT_STD_STRING_VIEW)
162+
// The following rules come from https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901247
163+
static_assert( ! validate_topic_name(""), "All Topic Names and Topic Filters MUST be at least one character long");
164+
static_assert(validate_topic_name("/"), "A Topic Name or Topic Filter consisting only of the ‘/’ character is valid");
165+
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)");
166+
static_assert(validate_topic_name(" "), "Topic Names and Topic Filters can include the space character");
167+
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");
168+
static_assert( ! validate_topic_name("#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
169+
static_assert( ! validate_topic_name("+"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
170+
static_assert( ! validate_topic_name("/#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
171+
static_assert( ! validate_topic_name("+/#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
172+
static_assert( ! validate_topic_name("f#"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
173+
static_assert( ! validate_topic_name("#/"), "The wildcard characters can be used in Topic Filters, but MUST NOT be used within a Topic Name");
174+
#endif // defined(MQTT_STD_STRING_VIEW)
175+
176+
MQTT_STRING_VIEW_CONSTEXPR
177+
bool compare_topic_filter(MQTT_NS::string_view topic_filter, MQTT_NS::string_view topic_name) {
178+
if ( ! validate_topic_filter(topic_filter)) {
179+
BOOST_ASSERT(validate_topic_filter(topic_filter));
180+
return false;
181+
}
182+
183+
if ( ! validate_topic_name(topic_name)) {
184+
BOOST_ASSERT(validate_topic_name(topic_name));
185+
return false;
186+
}
187+
188+
// TODO: The Server MUST NOT match Topic Filters starting with a wildcard character (# or +) with Topic Names beginning with a $ character
189+
for (MQTT_NS::string_view::size_type idx = topic_filter.find_first_of("+#");
190+
MQTT_NS::string_view::npos != idx;
191+
idx = topic_filter.find_first_of("+#")) {
192+
BOOST_ASSERT(
193+
('+' == topic_filter[idx])
194+
|| ('#' == topic_filter[idx])
195+
);
196+
197+
if ('+' == topic_filter[idx]) {
95198
// Compare everything up to the first +
96-
if(topicPattern.substr(0, idx) == topic.substr(0, idx)) {
199+
if(topic_filter.substr(0, idx) == topic_name.substr(0, idx)) {
97200
/*
98-
* We already know thanks to the topic pattern being validated
201+
* We already know thanks to the topic filter being validated
99202
* that the + symbol is directly touching '/'s on both sides
100203
* (if not the first or last character), so we don't need to
101204
* double check that.
@@ -104,12 +207,12 @@ inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::st
104207
* the loop continue, we get the proper comparison of the '/'s
105208
* automatically when the loop continues.
106209
*/
107-
topicPattern.remove_prefix(idx+1);
210+
topic_filter.remove_prefix(idx+1);
108211
/*
109212
* It's a bit more complicated for the incoming topic though
110213
* as we need to remove everything up to the next seperator.
111214
*/
112-
topic.remove_prefix(topic.find('/', idx));
215+
topic_name.remove_prefix(topic_name.find('/', idx));
113216
}
114217
else {
115218
return false;
@@ -121,14 +224,36 @@ inline bool compare_topic_pattern(MQTT_NS::string_view topicPattern, MQTT_NS::st
121224
* Compare up to where the multilevel wild card is found
122225
* and then anything after that matches the wildcard.
123226
*/
124-
return topicPattern.substr(0, idx) == topic.substr(0, idx);
227+
return topic_filter.substr(0, idx) == topic_name.substr(0, idx);
125228
}
126229
}
127230

128-
// No + or # found in the remaining topic pattern. Just do a string compare.
129-
return topicPattern == topic;
231+
// No + or # found in the remaining topic filter. Just do a string compare.
232+
return topic_filter == topic_name;
130233
}
131234

235+
#if defined(MQTT_STD_STRING_VIEW)
236+
static_assert(compare_topic_filter("bob", "bob"), "Topic Names and Topic Filters are case sensitive");
237+
static_assert( ! compare_topic_filter("Bob", "bob"), "Topic Names and Topic Filters are case sensitive");
238+
static_assert( ! compare_topic_filter("bob", "boB"), "Topic Names and Topic Filters are case sensitive");
239+
static_assert( ! compare_topic_filter("/bob", "bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
240+
static_assert( ! compare_topic_filter("bob/", "bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
241+
static_assert( ! compare_topic_filter("bob", "/bob"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
242+
static_assert( ! compare_topic_filter("bob", "bob/"), "A leading or trailing ‘/’ creates a distinct Topic Name or Topic Filter");
243+
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");
244+
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");
245+
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");
246+
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");
247+
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");
248+
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");
249+
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");
250+
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");
251+
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");
252+
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");
253+
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");
254+
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");
255+
#endif // defined(MQTT_STD_STRING_VIEW)
256+
132257
class test_broker {
133258
public:
134259
test_broker(as::io_context& ioc)
@@ -924,7 +1049,7 @@ class test_broker {
9241049
for( auto const& retain : retains_) {
9251050
MQTT_NS::buffer const& topic = std::get<0>(e);
9261051
MQTT_NS::subscribe_options options = std::get<1>(e);
927-
if (compare_topic_pattern(topic, retain.topic)) {
1052+
if (compare_topic_filter(topic, retain.topic)) {
9281053
auto rh = options.get_retain_handling();
9291054
if (rh == MQTT_NS::retain_handling::send ||
9301055
(rh == MQTT_NS::retain_handling::send_only_new_subscription && *new_sub_it)) {
@@ -1030,7 +1155,7 @@ class test_broker {
10301155
// For each active subscription registered for this topic
10311156
auto& idx = col.template get<tag_topic>();
10321157
for(auto& item : idx) {
1033-
if(compare_topic_pattern(item.topic, topic)) {
1158+
if(compare_topic_filter(item.topic, topic)) {
10341159
// publish the message to subscribers.
10351160
// retain is delivered as the original only if rap_value is rap::retain.
10361161
// On MQTT v3.1.1, rap_value is always rap::dont.

0 commit comments

Comments
 (0)