search

eclipse / paho.mqtt.java

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

Incorrect implementation of the Topic Alias property #510

Closed borgendale closed 6 years ago

borgendale commented 6 years ago

Unlike the other properties of an application message, the topic alias does not have the life of the message, but is only valid in a single connection. Some of the problems:

  1. The code writes the topic alias property into the message image which is persisted. However since the topic alias is only valid within a connection, this fails if the message is sent or resent in another connection.

  2. If the topic alias is set in the MqttProperties object associated with a message, that topic alias is used even if it is invalid in the current connection. This can cause the wrong topic to be used or a protocol error to be sent. The client should not ever send invalid MQTT to the server.

  3. The topic alias is not being set for messages at QoS=0. A major reason for making topic alias bound only for a single connection is to make it work for QoS=0 messages.

It appears there is no way of disabling the use of topic alias in the client to server direction by configuring the client. Currently to circumvent this bug you much change the server configuration to have it return a MaximumTopicAlias of 0.

If this is a bug regarding the Android Service, please raise the bug here instead: https://github.com/eclipse/paho.mqtt.android/issues/new

jpwsutton commented 6 years ago

I've implemented a change in MqttPersistableWireMessage.getHeaderBytes() that will check for MqttPublish Messages that have a topic alias. When one is found, the message will temporarily have it's topic alias set to null, it's header will be extracted and then the topic alias will be set back again. Realistically, nothing from this point should need to access the topic alias as it has effectively been sent, however I think it would be best that the property still be visible during runtime in case it is required. My tests have shown that in the case when a PUBLISH message with a topic alias that is set is persisted, then the getHeaderBytes() function takes roughly 0.007ms longer to execute but I think that this is probably acceptable.