X-Ryl669 / eMQTT5

An embedded MQTTv5 client in C++ with minimal footprint, maximal performance
MIT License
65 stars 14 forks source link

read properties of received packet #16

Closed mf01 closed 7 months ago

mf01 commented 8 months ago

I try to parse mqtt packets which I dumped with Wireshark using the MQTTParsePacket.cpp example. Works so far. In my CONNECT packet, there is an UserProperty key value pair included. packet->dump() shows my value

I tried to read the Property but I was not sucesssful until now.

Using the example code for reading properties from [0], works not.

// packet is type of Protocol::MQTT::V5::ControlPacketSerializable*
Protocol::MQTT::V5::VisitorVariant visitor;
while(packet->props.getProperty(visitor)) {
     switch(visitor.propertyType()) {
     case Protocol::MQTT::V5::UserProperty:
     {
         // no code until now
         break;
     }
     }
 }

This does not compile because packet has no props member.

Next try: explicit cast packet to Protocol::MQTT::V5::ConnectPacket*, result is: no matching function for call to 'Protocol::MQTT::V5::Properties::getProperty(Protocol::MQTT::V5::VisitorVariant&)'

Next try:

Protocol::MQTT::V5::ConnectPacket* connect_packet = dynamic_cast<Protocol::MQTT::V5::ConnectPacket*>(packet);
auto myProp = connect_packet->props.getProperty(Protocol::MQTT::V5::UserProperty);
if(myProp->type == Protocol::MQTT::V5::UserProperty) {
   ????
}

Can I cast myProp to something like UserPropertyProp or do I need a completely different solution.

[0] https://github.com/X-Ryl669/eMQTT5/blob/master/doc/ClientAPI.md#receiving-packets-from-the-broker

X-Ryl669 commented 8 months ago

Ok, let's explain better here.

You have 2 ways to read properties of a packet. The easy way is to use a VisitorVariant that'll iterate on all properties and you'll react to the property you're interested in.

The more complex way is to use the Properties interface like you did.

In the latter solution, as you've written, auto is a PropertyBase* that's pointing to a Property<DynamicStringPair>*. This is because the standard says that User properties are string pairs. So you'd use them like this:

Protocol::MQTT::V5::ConnectPacket* connect_packet = dynamic_cast<Protocol::MQTT::V5::ConnectPacket*>(packet);
size_t index = 0; // You can have as many UserProperty as you want
auto myProp = connect_packet->props.getProperty(Protocol::MQTT::V5::UserProperty, index);
if (myProp) {
   Property<DynamicStringPair>* p = static_cast<Property<DynamicStringPair>*>(myProp);
   std::cout<<"User property: key = " << p->value.key << ", value = " << p->value.value << std::endl;
}

The former case is, IMHO, easier to use and you'll use like this:

  typedef ControlPacket<CONNECT, true>          ROConnectPacket;
  ROConnectPacket * connect = static_cast<ROConnectPacket*>(packet);
  VisitorVariant visitor;
  while (connect->props.getProperty(visitor))
  {
        switch (visitor.propertyType())
        {
            case UserProperty:
            {
                auto t = visitor.as< DynamicStringPairView >();
                const DynamicStringView & key = t->key;
                const DynamicStringView & value = t->value;
                // Do something with key and value
                break;
            }

This is the generic version of a code with a UserProperty parsing. I've tried the above and it doesn't work on straight ConnectPacket because they can't be mapped to a View. Maybe I should introduce a ROControlPacket in the library like I did for the other packets so it would be easier. That's because a ConnectPacket is the only bidirectional packet in the protocol (used for both the Client=>Server and Server=>Client, the idea was to reuse the packet to answer the server so it can't be read only).

Let me think a bit more about this.

mf01 commented 8 months ago

This code works. Currently this is ok for my. Maybe you can add this to the documentation. I converted the value in to a string_view for usage on cout.

using namespace Protocol::MQTT::V5;
auto myProp = connect_packet->props.getProperty(UserProperty);
if(myProp) {
   const Property<DynamicStringPair>* userProp = dynamic_cast<const Property<DynamicStringPair>* >(myProp2);
   const std::string_view myKey { userProp->value.key.data, userProp->value.key.length };
   const std::string_view myValue { userProp->value.value.data, userProp->value.value.length };
   std::cout << "User property: key = " << myKey << ", value = " << myValue << std::endl;
}

The ROConnectPacket example works not with interesting behaviour:

My Connect packet dump:

CONNECT control packet (rlength: 42)
  Header: (type CONNECT, no flags)
  CONNECT packet (clean 1, will 0, willQoS 0, willRetain 0, password 0, username 0, keepAlive: 60)
  Properties with length VBInt: 20
    Type ReceiveMax
      
    Type UserProperty
      KV:
        Str (5 bytes): myKey
        Str (7 bytes): myValue
  CONNECT payload
    ClientID: Str (9 bytes): my-client
    Username: Str (0 bytes): 
    Password: Bin (0 bytes):
X-Ryl669 commented 8 months ago

Ok, I've thought a bit more about this. Your usage case is unusual for the protocol. You're building a Connect packet with properties (that's correct). Then you are trying to parse it.

This is illogical.

Your Connect packet goes to the broker but the broker will answer with a ConnACK packet (which does provide a PropertyView interface via the ROConnACKPacket).

In fact the line ROConnectPacket * connect = static_cast<ROConnectPacket*>(packet); in my last answer is only valid if the packet was a ROConnectPacket (with a long typedef of: Protocol::MQTT::V5::ControlPacket<Protocol::MQTT::V5::CONNECT, true>) at first and not a ConnectPacket (or its long typedef version: Protocol::MQTT::V5::ControlPacket<Protocol::MQTT::V5::CONNECT>)

May I ask you why you are dealing with a CONNECT packet by yourself and not using the library's MQTTClient::connectTo method ?

If you need to append properties to the connect packet, you can call it like this:

MQTTClient client;
// The user property to embed
Properties properties(new Property<DynamicStringPair>(UserProperty, DynamicStringPair("myKey", "myValue")));

if (ErrorType ret = client.connectTo(server, port, useTLS, keepAliveTimeoutInSec, true, username, password, lastWillMsg, lastWillQoS, true, &properties) != Success) {
   std::cerr<< "Can't connect to server with error: "<<ret<<std::endl;
}

or if you want to skip using the heap and use the stack only:

MQTTClient client;
// The user property to embed
Property<DynamicStringPair> userProp(UserProperty, DynamicStringPair("myKey", "myValue"));
Properties properties;
properties.append(&userProp);

if (ErrorType ret = client.connectTo(server, port, useTLS, keepAliveTimeoutInSec, true, username, password, lastWillMsg, lastWillQoS, true, &properties) != Success) {
   std::cerr<< "Can't connect to server with error: "<<ret<<std::endl;
}
X-Ryl669 commented 8 months ago

I've added a SerializeTest in the test code folder that makes an example of using the VisitorVariant on a CONNECT packet.

I've also added what's required to use a visitor in the Properties (not PropertiesView) so the same code will work regardless of the packet type.

I think it solves the issue for you, please confirm.

mf01 commented 8 months ago

I got packet->props.getProperty(visitor) == false on my connect packet. Calling auto myProp = connect_packet->props.getProperty(UserProperty) still works.

For clarification: I intercept mqtt packets on a tcp stream and evaluate it. I do not implement a broker.

X-Ryl669 commented 8 months ago

Can you share the packet dump of the Connect packet that fails parsing or better, the code that's generating it? Use the latest MQTTParsePacket, it's outputting a hexdump of the packet.

BTW, I've modified the code so you don't need to cast to ROConnectPacket anymore. Just use ConnectPacket and it should work too.

Thanks!

mf01 commented 8 months ago

I use mosquitto_(sub|pub)

mosquitto_sub -h localhost  --protocol-version mqttv5 --id client2 --property CONNECT user-property "mykey" "myVal"  -t "my/topic"

As broker, I use mosquitto with anonymous access. The packet can be dumped with wireshark.

the attached file are the packet bytes of the connect packet.

connect_sample.zip

X-Ryl669 commented 8 months ago

Ok, it seems it works for me:

Protocol::MQTT::V5::registerAllProperties();
Protocol::MQTT::V5::ControlPacket<Protocol::MQTT::V5::CONNECT> toPacket;
// Test exterior serialization
uint8 buf[] = {
          0x10, 0x29, 0x00, 0x04, 0x4d, 0x51, 0x54, 0x54, 0x05, 0x02, 0x00, 0x3c,
          0x15, 0x26, 0x00, 0x06, 0x20, 0x6d, 0x79, 0x6b, 0x65, 0x79, 0x00, 0x07,
          0x6d, 0x79, 0x56, 0x61, 0x6c, 0x75, 0x65, 0x21, 0x00, 0x14, 0x00, 0x07,
          0x63, 0x6c, 0x69, 0x65, 0x6e, 0x74, 0x32
};
if (toPacket.readFrom(buf, ArrSz(buf)) != ArrSz(buf)) return err("Can't parse alien packet");
Protocol::MQTT::V5::VisitorVariant visitor;
while (toPacket.props.getProperty(visitor))
{
      if (visitor.propertyType() == Protocol::MQTT::V5::ReceiveMax) {
                auto v = visitor.as< Protocol::MQTT::V5::LittleEndianPODVisitor<uint32> >();
                if (!v) return err("Can't find property PacketSizeMax");
                if (v->getValue() != 20) return err("Invalid PacketSizeMax property");
      }
      else if (visitor.propertyType() == Protocol::MQTT::V5::UserProperty) {
                auto v = visitor.as< Protocol::MQTT::V5::DynamicStringPairView >();
                if (!v) return err("Can't find property UserProperty");
                if (v->key != " mykey") return err("Invalid UserProperty key");
                if (v->value != "myValue") return err("Invalid UserProperty value");
      }
}
X-Ryl669 commented 7 months ago

Feel free to reopen if it doesn't work for you.