eclipse-paho / paho.mqtt.java

Eclipse Paho Java MQTT client library. Paho is an Eclipse IoT project.
https://eclipse.org/paho
Other
2.14k stars 889 forks source link

Race condition on connect to wildcard topics #817

Closed PMacho closed 4 years ago

PMacho commented 4 years ago

In the class:

https://github.com/eclipse/paho.mqtt.java/blob/master/org.eclipse.paho.client.mqttv3/src/main/java/org/eclipse/paho/client/mqttv3/internal/CommsCallback.java

within run() function, the lines:

                MqttPublish message = null;
                synchronized (messageQueue) {
                    if (!messageQueue.isEmpty()) {
                        // Note, there is a window on connect where a publish
                        // could arrive before we've
                        // finished the connect logic.
                        message = (MqttPublish) messageQueue.elementAt(0);

                        messageQueue.removeElementAt(0);
                    }
                }
                if (null != message) {
                    handleMessage(message);
                }

seemingly try to handle situation when messages arrive and are added to messageQueue before the "connect logic" is finished (as stated in the comment).

The handling takes the message and just re-publishes it (by means of the handleMessage method). This is fine for subscriptions to fully qualified topics. However, for wildcard topics this mechanism must fail and an exception is thrown at org.eclipse.paho.client.mqttv3.MqttTopic.validate(MqttTopic.java:226), correctly saying, it is not allowed to publish to wildcard topics.

I think, the code should block until connect() is done and then continue with any other logic.

Comments also mention this in https://github.com/eclipse/paho.mqtt.java/issues/340 . However, this thread is super old and originally not related. To this one.

PMacho commented 4 years ago

Relaying messages instead of a clean connect process still looks a little hacky to me. However, the issue is related to our broker, that is sending messages with wildcards in the topic name, which is just plain wrong according to the specification.