Topic names are not compared correctly

[a1lu]

Hi, I subscribe a topic test and get a confirmation SUBACK from the gateway. After that I try to publish to topic test/test but all my messages are send to test. The problem is that the equality check in BasicClient.h is wrong.

A fix could be to check for equal length and for equal names.

 return elem.m_allocated && (elem.m_topic.size() == strlen(op->m_topic)) && (elem.m_topic == op->m_topic);

But at the end this seems to be a bug in COMMS. I did not check if this is fixed yet in a newer COMMS version and gets fixed here when we are compatible to new COMMS. I can create a pull request on the weekend if you like.

[arobenko]

At this moment I use ext_mqtt311 branch (instead of develop), which uses latest COMMS library as well as cc.mqtt311.generated (instead of mqtt) for MQTT protocol definition (used by gateway).

I think it's getting close to be officially released (don't have #7 to be merged yet). Could you please try it and say whether it works for you?

[arobenko]

I can't reproduce reported problem with "ext_mqtt311" branch.

[arobenko]

Just FYI "ext_mqtt311" was merged to develop

[a1lu]

I can still reproduce this with current develop branch,

[arobenko]

Can you please give me some more details on how to reproduce? What gw and what pub sub utilities you are using, exactly what topics? Are they predefined or regular, etc?

On Fri, Feb 21, 2020, 20:30 a1lu notifications@github.com wrote:

I can still reproduce this with current develop branch,

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/arobenko/mqtt-sn/issues/8?email_source=notifications&email_token=AASJKGQMS63VBYX5UT26A6LRD6UK5A5CNFSM4KYKE4YKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMSIEBY#issuecomment-589595143, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASJKGUQQJVXE4TXBHVEAFLRD6UK5ANCNFSM4KYKE4YA .

[a1lu]

I could not reproduce this on a blank pull from develop, but if I integrate it into my project. I noticed the difference in the implementation of operator==. The blank branch uses std::operator==. In my integration I use the config set(MQTTSN_CUSTOM_CLIENT_TOPIC_NAME_STATIC_STORAGE_SIZE 64), this leads to comms::util::StaticString<64ul, char>::operator==. Implementation:

bool operator==(const TChar* str) const
    {
        for (auto idx = 0U; idx < size(); ++idx) {
            if (*str == Ends) {
                return false;
            }

            auto ch = vec_[idx];
            if (*str != ch) {
                return false;
            }

            ++str;
        }

        return true;
    }

str = test/abc this->vec_ = test The loop ends after consuming test and returns true

[arobenko]

Thanks, good find and analysis. I'll introduce fix to StaticString in the comms library early next week.

Last 'return true' needs to be replaced with string termination check.

On Fri, Feb 21, 2020, 22:39 a1lu notifications@github.com wrote:

I could not reproduce this on a blank pull from develop, but if I integrate it into my project. I noticed the difference in the implementation of operator==. The blank branch uses std::operator==. In my integration I use the config set(MQTTSN_CUSTOM_CLIENT_TOPIC_NAME_STATIC_STORAGE_SIZE 64), this leads to comms::util::StaticString<64ul, char>::operator==. Implementation:

bool operator==(const TChar str) const { for (auto idx = 0U; idx < size(); ++idx) { if (str == Ends) { return false; }

        auto ch = vec_[idx];
        if (*str != ch) {
            return false;
        }

        ++str;
    }

    return true;
}

str = test/abc this->vec_ = test The loop ends after consuming test and returns true

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/arobenko/mqtt-sn/issues/8?email_source=notifications&email_token=AASJKGQ7JCCE3R2JCQPBMB3RD7DRFA5CNFSM4KYKE4YKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMSSKQA#issuecomment-589636928, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASJKGRQDNZOSNQCNDWQ5WDRD7DRFANCNFSM4KYKE4YA .

[arobenko]

Just FYI: At this moment "develop" branch of comms_champion project contains fixes to StaticString. I haven't released the fixes yet and as the result haven't integrated into this project yet.

[arobenko]

The "develop" branch of this project in conjunction with develop branch of comms_champion project should have the problem fixed. I'll prepare the official releases soon.

Also note, that I introduced new cmake variable CC_MQTTSN_DEFAULT_CLIENT_CONFIG_FILE (similar to CC_MQTTSN_CUSTOM_CLIENT_CONFIG_FILES). It can be used to provide custom configuration for the default library, used by the pub / sub utilities. It can be used to easily test your embedded configuration on the development PC.

[arobenko]

Should be fixed in v0.9 release, please reopen if problem persists.