Removes Apache HTTP URIBuilder (#397)

* removes apache http URIBuilder * reverts to ParseException
2024-11-14 11:43:11 +02:00 · 2024-11-14 11:43:11 +02:00 · d1db64a1be
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -22,6 +22,8 @@ based on the size, format and compression of data. This will also allow users to
    to enable. Default changed to 0.
 - [BREAKING] Added exception to signature of ResourceAlgorithms.postToQueueWithRetries.
 - [BREAKING] Removed maxConnectionsPerRoute as it was not easily provided by azure-core.
+- [BREAKING] IPv6 addresses must now be enclosed in brackets ( [] ) within URLs.
+- Removed the dependency on Apache HTTP client URIBuilder for URL parsing.

 ## [5.2.0] - 2024-08-27
 ### Fixed
--- a/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java
+++ b/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java
@ -11,29 +11,32 @@ import com.microsoft.azure.kusto.data.auth.ConnectionStringBuilder;
 import com.microsoft.azure.kusto.data.auth.TokenProviderBase;
 import com.microsoft.azure.kusto.data.auth.TokenProviderFactory;
 import com.microsoft.azure.kusto.data.auth.endpoints.KustoTrustedEndpoints;
-import com.microsoft.azure.kusto.data.http.*;
-import com.microsoft.azure.kusto.data.instrumentation.*;
+import com.microsoft.azure.kusto.data.exceptions.DataClientException;
+import com.microsoft.azure.kusto.data.exceptions.DataServiceException;
+import com.microsoft.azure.kusto.data.exceptions.ExceptionsUtils;
+import com.microsoft.azure.kusto.data.exceptions.KustoServiceQueryError;
+import com.microsoft.azure.kusto.data.http.HttpClientFactory;
+import com.microsoft.azure.kusto.data.http.HttpClientProperties;
+import com.microsoft.azure.kusto.data.http.HttpRequestBuilder;
+import com.microsoft.azure.kusto.data.http.HttpTracing;
+import com.microsoft.azure.kusto.data.http.UncloseableStream;
+import com.microsoft.azure.kusto.data.instrumentation.MonitoredActivity;
+import com.microsoft.azure.kusto.data.instrumentation.SupplierOneException;
+import com.microsoft.azure.kusto.data.instrumentation.SupplierTwoExceptions;
+import com.microsoft.azure.kusto.data.instrumentation.TraceableAttributes;
 import com.microsoft.azure.kusto.data.req.KustoRequest;
 import com.microsoft.azure.kusto.data.req.KustoRequestContext;
 import com.microsoft.azure.kusto.data.res.JsonResult;
-import com.microsoft.azure.kusto.data.exceptions.DataClientException;
-import com.microsoft.azure.kusto.data.exceptions.DataServiceException;
-import com.microsoft.azure.kusto.data.exceptions.KustoServiceQueryError;
-import com.microsoft.azure.kusto.data.exceptions.ExceptionsUtils;
-import com.microsoft.azure.kusto.data.http.HttpClientFactory;
-import com.microsoft.azure.kusto.data.http.UncloseableStream;
-import com.microsoft.azure.kusto.data.instrumentation.MonitoredActivity;
-import com.microsoft.azure.kusto.data.instrumentation.SupplierTwoExceptions;
-import com.microsoft.azure.kusto.data.instrumentation.TraceableAttributes;
 import org.apache.commons.lang3.StringUtils;
-
 import org.apache.http.ParseException;
 import org.jetbrains.annotations.NotNull;

 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URISyntaxException;
-import java.util.*;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;

 class ClientImpl extends BaseClient {
--- a/data/src/main/java/com/microsoft/azure/kusto/data/UriUtils.java
+++ b/data/src/main/java/com/microsoft/azure/kusto/data/UriUtils.java
@ -1,7 +1,6 @@
 package com.microsoft.azure.kusto.data;

 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.client.utils.URIBuilder;

 import java.io.File;
 import java.net.URI;
@ -25,30 +24,37 @@ public class UriUtils {
        if (host == null) {
            host = StringUtils.removeEndIgnoreCase(auth, FEDERATED_SECURITY_SUFFIX);
        }
-        URIBuilder uriBuilder = new URIBuilder()
-                .setScheme(clusterUrlForParsing.getScheme())
-                .setHost(host);
+
        String path = clusterUrlForParsing.getPath();
        if (path != null && !path.isEmpty()) {
            path = StringUtils.removeEndIgnoreCase(path, FEDERATED_SECURITY_SUFFIX);
            path = StringUtils.removeEndIgnoreCase(path, "/");
-
-            uriBuilder.setPath(path);
        }

-        if (clusterUrlForParsing.getPort() != -1) {
-            uriBuilder.setPort(clusterUrlForParsing.getPort());
-        }
-        return uriBuilder.build().toString();
+        String clusterUri = String.format("%s://%s%s%s",
+                clusterUrlForParsing.getScheme(),
+                host,
+                clusterUrlForParsing.getPort() != -1 ? ":" + clusterUrlForParsing.getPort() : StringUtils.EMPTY,
+                path);
+        return new URI(clusterUri).toString();
    }

    public static String setPathForUri(String uri, String path, boolean ensureTrailingSlash) throws URISyntaxException {
        path = StringUtils.prependIfMissing(path, "/");

-        String pathString = new URIBuilder(uri).setPath(path).build().toString();
+        URI baseUri = new URI(uri);
+
+        URI newUri = new URI(
+                baseUri.getScheme(),
+                baseUri.getAuthority(),
+                path,
+                baseUri.getQuery(),
+                baseUri.getFragment());
+        String pathString = newUri.toString();
        if (ensureTrailingSlash) {
            pathString = StringUtils.appendIfMissing(pathString, "/");
        }
+
        return pathString;
    }

@ -57,7 +63,7 @@ public class UriUtils {
    }

    public static String appendPathToUri(String uri, String path) throws URISyntaxException {
-        String existing = new URIBuilder(uri).getPath();
+        String existing = new URI(uri).getPath();
        return setPathForUri(uri, StringUtils.appendIfMissing(existing == null ? "" : existing, "/") + path);
    }

--- a/data/src/test/java/com/microsoft/azure/kusto/data/ClientTest.java
+++ b/data/src/test/java/com/microsoft/azure/kusto/data/ClientTest.java
@ -9,7 +9,7 @@ import java.net.URISyntaxException;
 import java.util.HashMap;
 import java.util.Map;

-public class ClientTest {
+class ClientTest {

    @Test
    @DisplayName("test url parsing")
--- a/ingest/src/main/java/com/microsoft/azure/kusto/ingest/IngestClientBase.java
+++ b/ingest/src/main/java/com/microsoft/azure/kusto/ingest/IngestClientBase.java
@ -44,8 +44,16 @@ public abstract class IngestClientBase implements IngestClient {
        if (!uri.isAbsolute()) {
            return true;
        }
+
        String authority = uri.getAuthority().toLowerCase();
-        boolean isIPFlag = InetAddressUtils.isIPv4Address(authority) || InetAddressUtils.isIPv6Address(authority);
+        boolean isIPFlag;
+        if (authority.startsWith("[") && authority.endsWith("]")) {
+            authority = authority.substring(1, authority.length() - 1);
+            isIPFlag = true;
+        } else {
+            isIPFlag = InetAddressUtils.isIPv4Address(authority);
+        }
+
        boolean isLocalFlag = authority.contains("localhost");

        return isLocalFlag || isIPFlag || authority.equalsIgnoreCase("onebox.dev.kusto.windows.net");
--- a/ingest/src/main/java/com/microsoft/azure/kusto/ingest/StreamingIngestClient.java
+++ b/ingest/src/main/java/com/microsoft/azure/kusto/ingest/StreamingIngestClient.java
@ -27,14 +27,15 @@ import com.microsoft.azure.kusto.ingest.source.ResultSetSourceInfo;
 import com.microsoft.azure.kusto.ingest.source.StreamSourceInfo;
 import com.microsoft.azure.kusto.ingest.utils.IngestionUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.apache.http.client.utils.URIBuilder;
 import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

 import java.io.*;
 import java.lang.invoke.MethodHandles;
+import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.Objects;
 import java.util.zip.GZIPOutputStream;

 public class StreamingIngestClient extends IngestClientBase implements IngestClient {
@ -66,13 +67,20 @@ public class StreamingIngestClient extends IngestClientBase implements IngestCli
        this.streamingClient = streamingClient;
    }

-    public static String generateEngineUriSuggestion(URIBuilder existingEndpoint) {
-        if (!existingEndpoint.getHost().toLowerCase().startsWith(IngestClientBase.INGEST_PREFIX)) {
+    public static String generateEngineUriSuggestion(URI existingEndpoint) throws URISyntaxException {
+        if (!Objects.requireNonNull(existingEndpoint.getHost()).toLowerCase().startsWith(IngestClientBase.INGEST_PREFIX)) {
            throw new IllegalArgumentException("The URL is already formatted as the suggested Engine endpoint, so no suggestion can be made");
        }

-        existingEndpoint.setHost(existingEndpoint.getHost().substring(IngestClientBase.INGEST_PREFIX.length()));
-        return existingEndpoint.toString();
+        String host = existingEndpoint.getHost().substring(IngestClientBase.INGEST_PREFIX.length());
+        URI newUri = new URI(
+                existingEndpoint.getScheme(),
+                host,
+                existingEndpoint.getPath(),
+                existingEndpoint.getQuery(),
+                existingEndpoint.getFragment());
+
+        return newUri.toString();
    }

    @Override
--- a/ingest/src/test/java/com/microsoft/azure/kusto/ingest/E2ETest.java
+++ b/ingest/src/test/java/com/microsoft/azure/kusto/ingest/E2ETest.java
@ -5,7 +5,6 @@ package com.microsoft.azure.kusto.ingest;

 import com.azure.core.http.HttpClient;
 import com.azure.core.http.HttpResponse;
-import com.azure.core.util.Context;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ArrayNode;
@ -59,7 +58,6 @@ import java.util.concurrent.ThreadLocalRandom;
 import static com.microsoft.azure.kusto.ingest.IngestClientBase.*;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.atLeast;

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
--- a/ingest/src/test/java/com/microsoft/azure/kusto/ingest/IngestClientBaseTest.java
+++ b/ingest/src/test/java/com/microsoft/azure/kusto/ingest/IngestClientBaseTest.java
@ -8,7 +8,7 @@ import java.util.stream.Stream;

 import static org.junit.jupiter.api.Assertions.assertEquals;

-public class IngestClientBaseTest {
+class IngestClientBaseTest {
    private static Stream<Arguments> provideStringsForAutoCorrectEndpointTruePass() {
        return Stream.of(
                Arguments.of("https://testendpoint.dev.kusto.windows.net", "https://ingest-testendpoint.dev.kusto.windows.net"),
@ -17,7 +17,7 @@ public class IngestClientBaseTest {
                Arguments.of("https://2345:shouldwork:0425", "https://ingest-2345:shouldwork:0425"),
                Arguments.of("https://376.568.1564.1564", "https://ingest-376.568.1564.1564"),
                Arguments.of("https://192.168.1.1", "https://192.168.1.1"),
-                Arguments.of("https://2345:0425:2CA1:0000:0000:0567:5673:23b5", "https://2345:0425:2CA1:0000:0000:0567:5673:23b5"),
+                Arguments.of("https://[2345:0425:2CA1:0000:0000:0567:5673:23b5]", "https://[2345:0425:2CA1:0000:0000:0567:5673:23b5]"),
                Arguments.of("https://127.0.0.1", "https://127.0.0.1"),
                Arguments.of("https://localhost", "https://localhost"),
                Arguments.of("https://onebox.dev.kusto.windows.net", "https://onebox.dev.kusto.windows.net"));
--- a/ingest/src/test/java/com/microsoft/azure/kusto/ingest/StreamingIngestClientTest.java
+++ b/ingest/src/test/java/com/microsoft/azure/kusto/ingest/StreamingIngestClientTest.java
@ -597,7 +597,7 @@ class StreamingIngestClientTest {
                Arguments.of("https://2345:shouldwork:0425", "https://2345:shouldwork:0425"),
                Arguments.of("https://376.568.1564.1564", "https://376.568.1564.1564"),
                Arguments.of("https://192.168.1.1", "https://192.168.1.1"),
-                Arguments.of("https://2345:0425:2CA1:0000:0000:0567:5673:23b5", "https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]"),
+                Arguments.of("https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]", "https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]"),
                Arguments.of("https://127.0.0.1", "https://127.0.0.1"),
                Arguments.of("https://localhost", "https://localhost"),
                Arguments.of("https://onebox.dev.kusto.windows.net", "https://onebox.dev.kusto.windows.net"));
@ -619,7 +619,7 @@ class StreamingIngestClientTest {
                Arguments.of("https://2345:shouldwork:0425", "https://2345:shouldwork:0425"),
                Arguments.of("https://376.568.1564.1564", "https://376.568.1564.1564"),
                Arguments.of("https://192.168.1.1", "https://192.168.1.1"),
-                Arguments.of("https://2345:0425:2CA1:0000:0000:0567:5673:23b5", "https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]"),
+                Arguments.of("https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]", "https://[2345:0425:2ca1:0000:0000:0567:5673:23b5]"),
                Arguments.of("https://127.0.0.1", "https://127.0.0.1"),
                Arguments.of("https://localhost", "https://localhost"),
                Arguments.of("https://onebox.dev.kusto.windows.net", "https://onebox.dev.kusto.windows.net"));