addressed the review comments

Author: vyommani

hive-agent/src/main/java/org/apache/ranger/services/hive/client/JdbcUrlValidator.java

@@ -22,6 +22,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
@@ -34,6 +35,7 @@ public final class JdbcUrlValidator {
                     "socketfactory", "socketfactoryarg", "sslfactory", "sslfactoryarg",
                     "sslhostnameverifier", "authenticationpluginclassname", "loggerclassname",
                     "kerberosservername", "gssdelegatecred", "sslpasswordcallback")));
+    private static final String[] DANGEROUS_PATTERNS = {"socketfactory", "sslfactory", "autodeserialize"};
 
     private JdbcUrlValidator() {
     }
@@ -46,10 +48,7 @@ public static void validate(String jdbcUrl) throws HadoopException {
             throw e;
         }
         String trimmed = jdbcUrl.trim();
-        int queryStart = trimmed.indexOf('?');
-        if (queryStart == -1) {
-            queryStart = trimmed.indexOf(';');
-        }
+        int queryStart = findQueryStart(trimmed);
         if (queryStart != -1) {
             String queryString = trimmed.substring(queryStart + 1);
             validateQueryString(queryString, trimmed);
@@ -58,7 +57,7 @@ public static void validate(String jdbcUrl) throws HadoopException {
     }
 
     private static void validateQueryString(String queryString, String fullUrl) throws HadoopException {
-        String[] tokens = queryString.split("[&;]");
+        String[] tokens = queryString.split("[&;?]");
         for (String token : tokens) {
             if (token.trim().isEmpty()) {
                 continue;
@@ -67,7 +66,7 @@ private static void validateQueryString(String queryString, String fullUrl) thro
             String paramName = (eqIdx >= 0 ? token.substring(0, eqIdx) : token).trim();
             String decodedParamName = paramName;
             try {
-                decodedParamName = URLDecoder.decode(paramName, "UTF-8");
+                decodedParamName = URLDecoder.decode(paramName, StandardCharsets.UTF_8);
             } catch (Exception e) {
                 LOG.warn("Failed to decode parameter name: {}", paramName);
             }
@@ -76,8 +75,7 @@ private static void validateQueryString(String queryString, String fullUrl) thro
             if (BLOCKED_PARAMS.contains(normalized)) {
                 logAndThrow("blocked parameter", normalized, paramName, fullUrl);
             }
-            String[] dangerPatterns = {"socketfactory", "sslfactory", "autodeserialize"};
-            for (String danger : dangerPatterns) {
+            for (String danger : DANGEROUS_PATTERNS) {
                 if (normalized.contains(danger)) {
                     logAndThrow("dangerous pattern '" + danger + "'", normalized, paramName, fullUrl);
                 }
@@ -94,7 +92,21 @@ static String sanitizeForLog(String url) {
         if (url == null) {
             return "<null>";
         }
-        return url.replaceAll("[?;].*", "?<params_redacted>");
+        int idx = findQueryStart(url);
+        return idx >= 0 ? url.substring(0, idx) + "?<params_redacted>" : url;
+    }
+
+    private static int findQueryStart(String url) {
+        int qIdx = url.indexOf('?');
+        int sIdx = url.indexOf(';');
+        if (qIdx >= 0 && sIdx >= 0) {
+            return Math.min(qIdx, sIdx);
+        } else if (qIdx >= 0) {
+            return qIdx;
+        } else if (sIdx >= 0) {
+            return sIdx;
+        }
+        return -1;
     }
 
     private static void logAndThrow(String reason, String normalized, String originalParam, String fullUrl) {

hive-agent/src/test/java/org/apache/ranger/services/hive/client/JdbcUrlValidatorTest.java

@@ -347,6 +347,42 @@ public void testSanitizeForLogWithMixedSeparators() {
         }
     }
 
+    @Nested
+    @DisplayName("Mixed Parameter Delimiters")
+    class MixedDelimiters {
+        @Test
+        @DisplayName("Blocks dangerous param in semicolon when ? appears later")
+        void blocksSemicolonBeforeQuestion() {
+            String url = "jdbc:hive2://host:10000/db;socketFactory=evil?user=test";
+            HadoopException ex = assertThrows(HadoopException.class, () -> JdbcUrlValidator.validate(url));
+            assertTrue(ex.getMessage().contains("socketFactory"));
+        }
+
+        @Test
+        @DisplayName("Blocks dangerous param in question mark when ; appears later")
+        void blocksQuestionBeforeSemicolon() {
+            String url = "jdbc:hive2://host:10000/db?socketFactory=evil;user=test";
+            HadoopException ex = assertThrows(HadoopException.class, () -> JdbcUrlValidator.validate(url));
+            assertTrue(ex.getMessage().contains("socketFactory"));
+        }
+
+        @Test
+        @DisplayName("Handles semicolon-only Hive URLs")
+        void handlesSemicolonOnly() {
+            String url = "jdbc:hive2://host:10000/db;socketFactory=evil";
+            HadoopException ex = assertThrows(HadoopException.class, () -> JdbcUrlValidator.validate(url));
+            assertTrue(ex.getMessage().contains("socketFactory"));
+        }
+
+        @Test
+        @DisplayName("Handles question-only URLs")
+        void handlesQuestionOnly() {
+            String url = "jdbc:hive2://host:10000/db?socketFactory=evil";
+            HadoopException ex = assertThrows(HadoopException.class, () -> JdbcUrlValidator.validate(url));
+            assertTrue(ex.getMessage().contains("socketFactory"));
+        }
+    }
+
     @Nested
     @DisplayName("Integration Tests")
     class IntegrationTests {