HttpUtil: fix invalid URI (#4546)

Signed-off-by: Mark Herwege <mark.herwege@telenet.be>
openhab · Jan 10, 2025 · 8dd8ddd · 8dd8ddd
1 parent f8203c9
commit 8dd8ddd
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 28 deletions.
diff --git a/bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/HttpUtil.java b/bundles/org.openhab.core.io.net/src/main/java/org/openhab/core/io/net/http/HttpUtil.java
@@ -21,7 +21,6 @@
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.net.URL;
 import java.net.URLConnection;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
@@ -193,9 +192,17 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String
         // Get shared http client from factory "on-demand"
         final HttpClient httpClient = httpClientFactory.getCommonHttpClient();
 
+        URI uri = null;
+        try {
+            uri = new URI(url);
+        } catch (NullPointerException | URISyntaxException e) {
+            LOGGER.debug("String {} can not be parsed as URI reference", url);
+            throw new IOException(e);
+        }
+
         HttpProxy proxy = null;
         // Only configure a proxy if a host is provided
-        if (proxyHost != null && !proxyHost.isBlank() && proxyPort != null && shouldUseProxy(url, nonProxyHosts)) {
+        if (proxyHost != null && !proxyHost.isBlank() && proxyPort != null && shouldUseProxy(uri, nonProxyHosts)) {
             AuthenticationStore authStore = httpClient.getAuthenticationStore();
             ProxyConfiguration proxyConfig = httpClient.getProxyConfiguration();
             List<Proxy> proxies = proxyConfig.getProxies();
@@ -209,7 +216,7 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String
 
         final HttpMethod method = HttpUtil.createHttpMethod(httpMethod);
 
-        final Request request = httpClient.newRequest(url).method(method).timeout(timeout, TimeUnit.MILLISECONDS);
+        final Request request = httpClient.newRequest(uri).method(method).timeout(timeout, TimeUnit.MILLISECONDS);
 
         if (httpHeaders != null) {
             for (String httpHeaderKey : httpHeaders.stringPropertyNames()) {
@@ -222,20 +229,15 @@ private static ContentResponse executeUrlAndGetReponse(String httpMethod, String
         }
 
         // add basic auth header, if url contains user info
-        try {
-            URI uri = new URI(url);
-            if (uri.getUserInfo() != null) {
-                String[] userInfo = uri.getUserInfo().split(":");
+        if (uri.getUserInfo() != null) {
+            String[] userInfo = uri.getUserInfo().split(":");
 
-                String user = userInfo[0];
-                String password = userInfo[1];
+            String user = userInfo[0];
+            String password = userInfo[1];
 
-                String basicAuthentication = "Basic "
-                        + Base64.getEncoder().encodeToString((user + ":" + password).getBytes());
-                request.header(HttpHeader.AUTHORIZATION, basicAuthentication);
-            }
-        } catch (URISyntaxException e) {
-            LOGGER.debug("String {} can not be parsed as URI reference", url);
+            String basicAuthentication = "Basic "
+                    + Base64.getEncoder().encodeToString((user + ":" + password).getBytes());
+            request.header(HttpHeader.AUTHORIZATION, basicAuthentication);
         }
 
         // add content if a valid method is given ...
@@ -296,22 +298,21 @@ private static ProxyParams prepareProxyParams() {
 
     /**
      * Determines whether the list of <code>nonProxyHosts</code> contains the
-     * host (which is part of the given <code>urlString</code> or not.
+     * url host (which is part of the given <code>uri</code> or not.
      *
-     * @param urlString
+     * @param uri
      * @param nonProxyHosts
-     * @return <code>false</code> if the host of the given <code>urlString</code> is contained in
+     * @return <code>false</code> if the host of the given <code>uri</code> is contained in
      *         <code>nonProxyHosts</code>-list and <code>true</code> otherwise
      */
-    private static boolean shouldUseProxy(String urlString, String nonProxyHosts) {
+    private static boolean shouldUseProxy(URI uri, String nonProxyHosts) {
         if (nonProxyHosts != null && !nonProxyHosts.isBlank()) {
-            String givenHost = urlString;
+            String givenHost = uri.toString();
 
             try {
-                URL url = new URL(urlString);
-                givenHost = url.getHost();
-            } catch (MalformedURLException e) {
-                LOGGER.error("the given url {} is malformed", urlString);
+                givenHost = uri.toURL().getHost();
+            } catch (IllegalArgumentException | MalformedURLException e) {
+                LOGGER.error("the given url {} is malformed", uri.toString());
             }
 
             String[] hosts = nonProxyHosts.split("\\|");

diff --git a/.../org.openhab.core.io.net/src/test/java/org/openhab/core/io/net/http/BaseHttpUtilTest.java b/.../org.openhab.core.io.net/src/test/java/org/openhab/core/io/net/http/BaseHttpUtilTest.java
@@ -16,6 +16,7 @@
 import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Field;
+import java.net.URI;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -55,7 +56,7 @@ public void setUp() throws Exception {
         httpClientFactory.set(null, clientFactoryMock);
 
         when(clientFactoryMock.getCommonHttpClient()).thenReturn(httpClientMock);
-        when(httpClientMock.newRequest(URL)).thenReturn(requestMock);
+        when(httpClientMock.newRequest(URI.create(URL))).thenReturn(requestMock);
         when(requestMock.method(any(HttpMethod.class))).thenReturn(requestMock);
         when(requestMock.timeout(anyLong(), any(TimeUnit.class))).thenReturn(requestMock);
         when(requestMock.send()).thenReturn(contentResponseMock);

diff --git a/...penhab.core.io.net/src/test/java/org/openhab/core/io/net/http/HttpRequestBuilderTest.java b/...penhab.core.io.net/src/test/java/org/openhab/core/io/net/http/HttpRequestBuilderTest.java
@@ -15,6 +15,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.Mockito.verify;
 
+import java.net.URI;
 import java.nio.Buffer;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
@@ -45,7 +46,7 @@ public void baseTest() throws Exception {
 
         assertEquals("Some content", result);
 
-        verify(httpClientMock).newRequest(URL);
+        verify(httpClientMock).newRequest(URI.create(URL));
         verify(requestMock).method(HttpMethod.GET);
         verify(requestMock).send();
     }

diff --git a/bundles/org.openhab.core.io.net/src/test/java/org/openhab/core/io/net/http/HttpUtilTest.java b/bundles/org.openhab.core.io.net/src/test/java/org/openhab/core/io/net/http/HttpUtilTest.java
@@ -15,6 +15,7 @@
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
+import java.net.URI;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -40,15 +41,15 @@ public void baseTest() throws Exception {
 
         assertEquals("Some content", result);
 
-        verify(httpClientMock).newRequest(URL);
+        verify(httpClientMock).newRequest(URI.create(URL));
         verify(requestMock).method(HttpMethod.GET);
         verify(requestMock).timeout(500, TimeUnit.MILLISECONDS);
         verify(requestMock).send();
     }
 
     @Test
     public void testAuthentication() throws Exception {
-        when(httpClientMock.newRequest("http://john:doe@example.org/")).thenReturn(requestMock);
+        when(httpClientMock.newRequest(URI.create("http://john:doe@example.org/"))).thenReturn(requestMock);
         mockResponse(HttpStatus.OK_200);
 
         String result = HttpUtil.executeUrl("GET", "http://john:doe@example.org/", 500);