Merge pull request #11565 from irynamatveieva/fix/mqtt-node-client-id-length

Mqtt node: added validation for client id

rule-engine/rule-engine-components/src/main/java/org/thingsboard/rule/engine/mqtt/TbMqttNode.java

@@ -77,6 +77,8 @@ public class TbMqttNode extends TbAbstractExternalNode {
         this.mqttNodeConfiguration = TbNodeUtils.convert(configuration, TbMqttNodeConfiguration.class);
         try {
             this.mqttClient = initClient(ctx);
+        } catch (TbNodeException e) {
+            throw e;
         } catch (Exception e) {
             throw new TbNodeException(e);
         }
@@ -119,8 +121,7 @@ public class TbMqttNode extends TbAbstractExternalNode {
         MqttClientConfig config = new MqttClientConfig(getSslContext());
         config.setOwnerId(getOwnerId(ctx));
         if (!StringUtils.isEmpty(this.mqttNodeConfiguration.getClientId())) {
-            config.setClientId(this.mqttNodeConfiguration.isAppendClientIdSuffix() ?
+            config.setClientId(getClientId(ctx));
-                    this.mqttNodeConfiguration.getClientId() + "_" + ctx.getServiceId() : this.mqttNodeConfiguration.getClientId());
         }
         config.setCleanSession(this.mqttNodeConfiguration.isCleanSession());
 
@@ -146,6 +147,17 @@ public class TbMqttNode extends TbAbstractExternalNode {
         return client;
     }
 
+    private String getClientId(TbContext ctx) throws TbNodeException {
+        String clientId = this.mqttNodeConfiguration.isAppendClientIdSuffix() ?
+                this.mqttNodeConfiguration.getClientId() + "_" + ctx.getServiceId() :
+                this.mqttNodeConfiguration.getClientId();
+        if (clientId.length() > 23) {
+            throw new TbNodeException("Client ID is too long '" + clientId + "'. " +
+                    "The length of Client ID cannot be longer than 23, but current length is " + clientId.length() + ".", true);
+        }
+        return clientId;
+    }
+
     MqttClient getMqttClient(TbContext ctx, MqttClientConfig config) {
         return MqttClient.create(config, null, ctx.getExternalCallExecutor());
     }

rule-engine/rule-engine-components/src/test/java/org/thingsboard/rule/engine/mqtt/TbMqttNodeTest.java

@@ -191,6 +191,42 @@ public class TbMqttNodeTest extends AbstractRuleNodeUpgradeTest {
         assertThatNoException().isThrownBy(() -> mqttNode.init(ctxMock, new TbNodeConfiguration(JacksonUtil.valueToTree(mqttNodeConfig))));
     }
 
+    @Test
+    public void givenClientIdIsTooLong_whenInit_thenThrowsException() {
+        String invalidClientId = "vhfrbeb38ygwfwrgfwefgterhytjytj";
+        mqttNodeConfig.setClientId(invalidClientId);
+
+        given(ctxMock.getTenantId()).willReturn(TENANT_ID);
+        given(ctxMock.getSelf()).willReturn(new RuleNode(RULE_NODE_ID));
+
+        assertThatThrownBy(() -> mqttNode.init(ctxMock, new TbNodeConfiguration(JacksonUtil.valueToTree(mqttNodeConfig))))
+                .isInstanceOf(TbNodeException.class)
+                .hasMessage("Client ID is too long '" + invalidClientId + "'. " +
+                        "The length of Client ID cannot be longer than 23, but current length is " + invalidClientId.length() + ".")
+                .extracting(e -> ((TbNodeException) e).isUnrecoverable())
+                .isEqualTo(true);
+    }
+
+    @Test
+    public void givenClientIdIsOkAndAppendClientIdSuffixIsTrue_whenInit_thenClientIdBecomesInvalidAndThrowsException() {
+        String validClientId = "fertjnhnjj4ge";
+        mqttNodeConfig.setClientId("fertjnhnjj4ge");
+        mqttNodeConfig.setAppendClientIdSuffix(true);
+
+        given(ctxMock.getTenantId()).willReturn(TENANT_ID);
+        given(ctxMock.getSelf()).willReturn(new RuleNode(RULE_NODE_ID));
+        String serviceId = "test-service";
+        given(ctxMock.getServiceId()).willReturn(serviceId);
+
+        String resultedClientId = validClientId + "_" + serviceId;
+        assertThatThrownBy(() -> mqttNode.init(ctxMock, new TbNodeConfiguration(JacksonUtil.valueToTree(mqttNodeConfig))))
+                .isInstanceOf(TbNodeException.class)
+                .hasMessage("Client ID is too long '" + resultedClientId + "'. " +
+                        "The length of Client ID cannot be longer than 23, but current length is " + resultedClientId.length() + ".")
+                .extracting(e -> ((TbNodeException) e).isUnrecoverable())
+                .isEqualTo(true);
+    }
+
     @Test
     public void givenFailedByTimeoutConnectResult_whenInit_thenThrowsException() throws ExecutionException, InterruptedException, TimeoutException {
         mqttNodeConfig.setHost("localhost");