Skip to content
Permalink
Browse files
ARTEMIS-3775 don't carry forward MQTT topic aliases to new connection
  • Loading branch information
jbertram authored and clebertsuconic committed Apr 19, 2022
1 parent 7d11cf8 commit 40a191379b238fe3dcc44452defddcf878d30492
Showing 4 changed files with 68 additions and 9 deletions.
@@ -189,6 +189,10 @@ void sendToQueue(MqttPublishMessage message, boolean internal) throws Exception
topic = session.getState().getClientTopicAlias(alias);
if (topic == null) {
topic = message.variableHeader().topicName();
if (topic == null || topic.length() == 0) {
// using a topic alias with no matching topic in the state; potentially [MQTT-3.3.2-7]
throw new DisconnectException(MQTTReasonCodes.TOPIC_ALIAS_INVALID);
}
session.getState().addClientTopicAlias(alias, topic);
}
}
@@ -119,6 +119,7 @@ synchronized void stop(boolean failure) throws Exception {
if (state != null) {
state.setAttached(false);
state.setDisconnectedTime(System.currentTimeMillis());
state.clearTopicAliases();
}

if (getVersion() == MQTTVersion.MQTT_5) {
@@ -119,14 +119,7 @@ public synchronized void clear() {
willRetain = false;
willTopic = null;
clientMaxPacketSize = 0;
if (clientTopicAliases != null) {
clientTopicAliases.clear();
clientTopicAliases = null;
}
if (serverTopicAliases != null) {
serverTopicAliases.clear();
serverTopicAliases = null;
}
clearTopicAliases();
clientTopicAliasMaximum = 0;
}

@@ -367,6 +360,17 @@ void removeMessageRef(Integer mqttId) {
}
}

public void clearTopicAliases() {
if (clientTopicAliases != null) {
clientTopicAliases.clear();
clientTopicAliases = null;
}
if (serverTopicAliases != null) {
serverTopicAliases.clear();
serverTopicAliases = null;
}
}

public class OutboundStore {

private HashMap<Pair<Long, Long>, Integer> artemisToMqttMessageMap = new HashMap<>();
@@ -46,6 +46,7 @@
import org.eclipse.paho.mqttv5.common.packet.MqttWireMessage;
import org.eclipse.paho.mqttv5.common.packet.UserProperty;
import org.jboss.logging.Logger;
import org.junit.Ignore;
import org.junit.Test;

/**
@@ -66,7 +67,6 @@
*
* [MQTT-3.3.1-4] A PUBLISH Packet MUST NOT have both QoS bits set to 1.
* [MQTT-3.3.2-2] The Topic Name in the PUBLISH packet MUST NOT contain wildcard characters.
* [MQTT-3.3.2-7] A receiver MUST NOT carry forward any Topic Alias mappings from one Network Connection to another.
*
*
* I can't force the Paho client to set a Topic Alias of 0. It automatically adjusts it to 1 (confirmed via Wireshark), therefore this is not tested:
@@ -893,6 +893,56 @@ public void messageArrived(String topic, MqttMessage message) throws Exception {
consumer.disconnect();
}

/*
* [MQTT-3.3.2-7] A receiver MUST NOT carry forward any Topic Alias mappings from one Network Connection to another.
*
* Unfortunately the Paho MQTT 5 client performs automatic validation and therefore refuses to send a message with an
* empty topic even though the topic-alias is set appropriately. Therefore, this test won't actually run with the
* current client implementation. However, I built the client locally omitting the validation logic and the test
* passed as expected.
*
* I'm leaving this test here with @Ignore to demonstrate how to theoretically re-produce the problem. The problem
* was originally discovered using https://github.com/eclipse/paho.mqtt.testing/tree/master/interoperability to run
* the command:
*
* python3 client_test5.py Test.test_client_topic_alias
*/
@Ignore
@Test(timeout = DEFAULT_TIMEOUT)
public void testTopicAliasesNotCarriedForward() throws Exception {
final String TOPIC = "myTopicName";

MqttProperties properties = new MqttProperties();
properties.setTopicAlias(1);

MqttClient producer = createPahoClient("producer");
MqttConnectionOptions options = new MqttConnectionOptionsBuilder()
.topicAliasMaximum(2)
.sessionExpiryInterval(999L)
.cleanStart(false)
.build();
producer.connect(options);
MqttMessage m = new MqttMessage();
m.setProperties(properties);
producer.publish(TOPIC, m);
m = new MqttMessage();
m.setProperties(properties);
producer.publish("", m);
producer.disconnect();
// reconnect back to the old session, but the topic alias should now be gone so publishing should fail
producer.connect(options);
m = new MqttMessage();
m.setProperties(properties);
try {
producer.publish("", m);
fail("Publishing should fail here due to an invalid topic alias");
} catch (Exception e) {
// ignore
}
assertFalse(producer.isConnected());
producer.close();
}

/*
* [MQTT-3.3.2-12] A Server MUST accept all Topic Alias values greater than 0 and less than or equal to the Topic
* Alias Maximum value that it returned in the CONNACK packet.

0 comments on commit 40a1913

Please sign in to comment.