[homematic] Fixed #16940: Spaces in URLs are not allowed (#17206)

* #16940: Added check in handler for invalid configuration values.

Signed-off-by: Sönke Küper <soenkekueper@gmx.de>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>

bundles/org.openhab.binding.homematic/README.md

@@ -141,7 +141,7 @@ Network address of the Homematic gateway
 Hint for the binding to identify the gateway type (auto|ccu|noccu) (default = "auto").
 
 - **callbackHost**
-Callback network address of the system runtime, default is auto-discovery
+Callback network address of the system runtime, default is auto-discovery. This value must not contain any white spaces.
 
 - **xmlCallbackPort**
 Callback port of the binding's XML-RPC server, default is 9125 and counts up for each additional bridge

bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandler.java

@@ -20,6 +20,7 @@ import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.annotation.NonNull;
@@ -58,11 +59,14 @@ import org.slf4j.LoggerFactory;
  * @author Gerhard Riegler - Initial contribution
  */
 public class HomematicBridgeHandler extends BaseBridgeHandler implements HomematicGatewayAdapter {
+
+    protected ScheduledExecutorService executorService = scheduler;
+
     private final Logger logger = LoggerFactory.getLogger(HomematicBridgeHandler.class);
     private static final long REINITIALIZE_DELAY_SECONDS = 10;
     private static final int DUTY_CYCLE_RATIO_LIMIT = 99;
     private static final int DUTY_CYCLE_DISCONNECTED = -1;
-    private static SimplePortPool portPool = new SimplePortPool();
+    private static final SimplePortPool portPool = new SimplePortPool();
 
     private final Object dutyCycleRatioUpdateLock = new Object();
     private final Object initDisposeLock = new Object();
@@ -93,7 +97,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat
     public void initialize() {
         synchronized (initDisposeLock) {
             isDisposed = false;
-            initializeFuture = scheduler.submit(this::initializeInternal);
+            initializeFuture = executorService.submit(this::initializeInternal);
         }
     }
 
@@ -106,6 +110,8 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat
             config = createHomematicConfig();
 
             try {
+                this.checkForConfigurationErrors();
+
                 String id = getThing().getUID().getId();
                 gateway = HomematicGatewayFactory.createGateway(id, config, this, httpClient);
                 configureThingProperties();
@@ -140,6 +146,15 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat
         }
     }
 
+    /**
+     * Validates, if the configuration contains errors.
+     */
+    private void checkForConfigurationErrors() {
+        if (this.config.getCallbackHost().contains(" ")) {
+            throw new ConfigurationException("The callback host mut not contain white spaces.");
+        }
+    }
+
     private void configureThingProperties() {
         final HmGatewayInfo info = config.getGatewayInfo();
         final Map<String, String> properties = getThing().getProperties();
@@ -160,7 +175,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat
      */
     private void scheduleReinitialize() {
         if (!isDisposed) {
-            initializeFuture = scheduler.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS,
+            initializeFuture = executorService.schedule(this::initializeInternal, REINITIALIZE_DELAY_SECONDS,
                     TimeUnit.SECONDS);
         }
     }
@@ -434,7 +449,7 @@ public class HomematicBridgeHandler extends BaseBridgeHandler implements Homemat
      * @param defer <i>true</i> will delete the device once it becomes available.
      */
     public void deleteFromGateway(String address, boolean reset, boolean force, boolean defer) {
-        scheduler.submit(() -> {
+        executorService.submit(() -> {
             logger.debug("Deleting the device '{}' from gateway '{}'", address, getBridge());
             getGateway().deleteDevice(address, reset, force, defer);
         });

bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerMock.java

@@ -0,0 +1,45 @@
+/**
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.homematic.internal.handler;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+
+import java.util.concurrent.ScheduledExecutorService;
+
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jetty.client.HttpClient;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.openhab.binding.homematic.internal.type.HomematicTypeGenerator;
+import org.openhab.core.thing.Bridge;
+
+/**
+ * The {@link HomematicBridgeHandlerMock} is responsible for mocking {@link HomematicBridgeHandler}
+ *
+ * @author Sönke Küper - Initial contribution
+ */
+@NonNullByDefault
+public class HomematicBridgeHandlerMock extends HomematicBridgeHandler {
+
+    public HomematicBridgeHandlerMock(@NonNull Bridge bridge, HomematicTypeGenerator typeGenerator, String ipv4Address,
+            HttpClient httpClient) {
+        super(bridge, typeGenerator, ipv4Address, httpClient);
+        executorService = Mockito.mock(ScheduledExecutorService.class);
+        doAnswer((InvocationOnMock invocation) -> {
+            ((Runnable) invocation.getArguments()[0]).run();
+            return null;
+        }).when(executorService).submit(any(Runnable.class));
+    }
+}

bundles/org.openhab.binding.homematic/src/test/java/org/openhab/binding/homematic/internal/handler/HomematicBridgeHandlerTest.java

@@ -0,0 +1,65 @@
+/**
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.homematic.internal.handler;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.util.Objects;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jetty.client.HttpClient;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+import org.openhab.binding.homematic.internal.HomematicBindingConstants;
+import org.openhab.binding.homematic.internal.type.HomematicTypeGenerator;
+import org.openhab.core.thing.Bridge;
+import org.openhab.core.thing.ThingStatus;
+import org.openhab.core.thing.ThingStatusDetail;
+import org.openhab.core.thing.binding.ThingHandlerCallback;
+import org.openhab.core.thing.internal.BridgeImpl;
+
+/**
+ * @author Sönke Küper - Initial contribution
+ */
+@NonNullByDefault
+public class HomematicBridgeHandlerTest {
+
+    @Test
+    public void testGetRpcCallbackUrlDoesNotContainsSpaces() {
+        HttpClient httpClient = Mockito.mock(HttpClient.class);
+
+        Bridge bridge = new BridgeImpl(HomematicBindingConstants.THING_TYPE_BRIDGE, "1234");
+        bridge.getConfiguration().put("callbackHost", " 192. 168.1.1");
+        assertThat(bridge.getStatus(), is(ThingStatus.UNINITIALIZED));
+        HomematicTypeGenerator typeGenerator = mock(HomematicTypeGenerator.class);
+
+        HomematicBridgeHandlerMock handler = new HomematicBridgeHandlerMock(bridge, typeGenerator, "1.2.3.4",
+                httpClient);
+        ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
+        handler.setCallback(callback);
+        handler.initialize();
+
+        try {
+            verify(callback).statusUpdated(eq(bridge), argThat(arg -> arg.getStatus().equals(ThingStatus.OFFLINE)
+                    && arg.getStatusDetail().equals(ThingStatusDetail.CONFIGURATION_ERROR)
+                    && Objects.equals(arg.getDescription(), "The callback host mut not contain white spaces.")));
+        } finally {
+            handler.dispose();
+        }
+    }
+}