This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 40a191379b ARTEMIS-3775 don't carry forward MQTT topic aliases to new
connection
40a191379b is described below
commit 40a191379b238fe3dcc44452defddcf878d30492
Author: Justin Bertram <[email protected]>
AuthorDate: Tue Apr 12 13:55:12 2022 -0500
ARTEMIS-3775 don't carry forward MQTT topic aliases to new connection
---
.../core/protocol/mqtt/MQTTPublishManager.java | 4 ++
.../artemis/core/protocol/mqtt/MQTTSession.java | 1 +
.../core/protocol/mqtt/MQTTSessionState.java | 20 +++++----
.../mqtt5/spec/controlpackets/PublishTests.java | 52 +++++++++++++++++++++-
4 files changed, 68 insertions(+), 9 deletions(-)
diff --git
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java
index bcf278e542..672545b427 100644
---
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java
+++
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTPublishManager.java
@@ -189,6 +189,10 @@ public class MQTTPublishManager {
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);
}
}
diff --git
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java
index 3f7b98ba28..e2063c22ae 100644
---
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java
+++
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSession.java
@@ -119,6 +119,7 @@ public class MQTTSession {
if (state != null) {
state.setAttached(false);
state.setDisconnectedTime(System.currentTimeMillis());
+ state.clearTopicAliases();
}
if (getVersion() == MQTTVersion.MQTT_5) {
diff --git
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java
index 4174931cb6..90b2e5233f 100644
---
a/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java
+++
b/artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java
@@ -119,14 +119,7 @@ public class MQTTSessionState {
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 @@ public class MQTTSessionState {
}
}
+ 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<>();
diff --git
a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java
index 839adf2fbb..8c4c2c9112 100644
---
a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java
+++
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt5/spec/controlpackets/PublishTests.java
@@ -46,6 +46,7 @@ import org.eclipse.paho.mqttv5.common.packet.MqttPublish;
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 @@ import org.junit.Test;
*
* [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 class PublishTests extends MQTT5TestSupport {
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.
