ayushtkn commented on code in PR #5729: URL: https://github.com/apache/hive/pull/5729#discussion_r2030262395
########## common/src/java/org/apache/hive/common/IPStackUtils.java: ########## @@ -219,4 +219,91 @@ public static String transformToIPv6(String ipv4, int port) { return ipv4; } } + + /** + * Splits a given input string representing a Hostname or an IP address and port into an `HostPort` object. + * The input string must be in the format of IPv4/IPv6/[IPv6]/hostname:port. + * + * @param input The input string containing the Hostname/IP address and port, in the format + * "IPv4:port", "[IPv6]:port", "IPv6:port", or "hostname:port". + * @return A {@link HostPort} object containing the parsed IP address and port number. + * @throws IllegalArgumentException If the input format is invalid, if the host is null or empty, + * or if the port number is invalid. + */ + public static HostPort splitHostPort(String input) { + + String host; + int port; + + if (input == null || input.isEmpty()) { + throw new IllegalArgumentException("Input string is null or empty"); + } + + // Check if the input contains a colon, which separates the host and port + int colonIndex = input.lastIndexOf(':'); + if (colonIndex == -1) { + throw new IllegalArgumentException("Input does not contain a port."); + } + + // Extract the host and port parts + host = input.substring(0, colonIndex); + String portStr = input.substring(colonIndex + 1); + + // Check if the port is valid + if (!isValidPort(portStr)) { + throw new IllegalArgumentException("Port number out of range (0-65535)."); + } + port = Integer.parseInt(portStr); Review Comment: can change the method to `getPort()` & either return the valid port or throw the exception in that method itself, would save redundant parseInt call ########## common/src/java/org/apache/hive/common/IPStackUtils.java: ########## @@ -219,4 +219,91 @@ public static String transformToIPv6(String ipv4, int port) { return ipv4; } } + + /** + * Splits a given input string representing a Hostname or an IP address and port into an `HostPort` object. + * The input string must be in the format of IPv4/IPv6/[IPv6]/hostname:port. + * + * @param input The input string containing the Hostname/IP address and port, in the format + * "IPv4:port", "[IPv6]:port", "IPv6:port", or "hostname:port". + * @return A {@link HostPort} object containing the parsed IP address and port number. + * @throws IllegalArgumentException If the input format is invalid, if the host is null or empty, + * or if the port number is invalid. + */ + public static HostPort splitHostPort(String input) { + + String host; + int port; + + if (input == null || input.isEmpty()) { + throw new IllegalArgumentException("Input string is null or empty"); + } + + // Check if the input contains a colon, which separates the host and port + int colonIndex = input.lastIndexOf(':'); Review Comment: can we just split using `:` ?, so as to save doing the `substring` later, we can have a validation that post split there aren't more than 2 elems? ########## common/src/java/org/apache/hive/common/IPStackUtils.java: ########## @@ -219,4 +219,91 @@ public static String transformToIPv6(String ipv4, int port) { return ipv4; } } + + /** + * Splits a given input string representing a Hostname or an IP address and port into an `HostPort` object. + * The input string must be in the format of IPv4/IPv6/[IPv6]/hostname:port. + * + * @param input The input string containing the Hostname/IP address and port, in the format + * "IPv4:port", "[IPv6]:port", "IPv6:port", or "hostname:port". + * @return A {@link HostPort} object containing the parsed IP address and port number. + * @throws IllegalArgumentException If the input format is invalid, if the host is null or empty, + * or if the port number is invalid. + */ + public static HostPort splitHostPort(String input) { + + String host; + int port; + + if (input == null || input.isEmpty()) { + throw new IllegalArgumentException("Input string is null or empty"); + } + + // Check if the input contains a colon, which separates the host and port + int colonIndex = input.lastIndexOf(':'); + if (colonIndex == -1) { + throw new IllegalArgumentException("Input does not contain a port."); + } + + // Extract the host and port parts + host = input.substring(0, colonIndex); + String portStr = input.substring(colonIndex + 1); + + // Check if the port is valid + if (!isValidPort(portStr)) { + throw new IllegalArgumentException("Port number out of range (0-65535)."); + } + port = Integer.parseInt(portStr); + + // Handle IPv6 addresses enclosed in square brackets (e.g., [IPv6]:port) + if (host.startsWith("[") && host.endsWith("]")) { + host = host.substring(1, host.length() - 1); // Remove the square brackets + } + + // Check if the host is not null or empty + if (host == null || host.isEmpty()) { + throw new IllegalArgumentException("Host address is null or empty."); + } Review Comment: this should be above `` // Handle IPv6 addresses enclosed in square brackets (e.g., [IPv6]:port)`` if host is null, that startsWith will throw `NPE` ########## common/src/test/org/apache/hive/common/IPStackUtilsTest.java: ########## @@ -180,4 +180,72 @@ void testWildcardWhenHostnameIsProvided() { String result = IPStackUtils.adaptWildcardAddress("example.com"); assertEquals("example.com", result); } + + // Test cases for splitHostPort method + + @Test + void testSplitHostPortWithIPv4() { + IPStackUtils.HostPort result = IPStackUtils.splitHostPort("192.168.1.1:8080"); + assertEquals("192.168.1.1", result.getHostname()); + assertEquals(8080, result.getPort()); + } + + @Test + void testSplitHostPortWithValidIPv6WithSquaredBrackets() { + IPStackUtils.HostPort result = IPStackUtils.splitHostPort("[2001:0db8::1]:8080"); + assertEquals("2001:0db8::1", result.getHostname()); + assertEquals(8080, result.getPort()); + } + + @Test + void testSplitHostPortWithValidIPv6WithoutSquaredBrackets() { + IPStackUtils.HostPort result = IPStackUtils.splitHostPort("2001:0db8::1:8080"); + assertEquals("2001:0db8::1", result.getHostname()); + assertEquals(8080, result.getPort()); + } + + @Test + void testSplitHostPortWithHostname() { + IPStackUtils.HostPort result = IPStackUtils.splitHostPort("example.com:80"); + assertEquals("example.com", result.getHostname()); + assertEquals(80, result.getPort()); + } + + @Test + void testSplitHostPortWithInvalidPort() { + assertThrows(IllegalArgumentException.class, () -> IPStackUtils.splitHostPort("192.168.1.1:70000")); + } + + @Test + void testSplitHostPortWithMissingPort() { + assertThrows(IllegalArgumentException.class, () -> IPStackUtils.splitHostPort("192.168.1.1")); + } + + @Test + void testSplitHostPortWithMissingIP() { + assertThrows(IllegalArgumentException.class, () -> IPStackUtils.splitHostPort(":8080")); + } Review Comment: Can you assert the error message as well ########## common/src/java/org/apache/hive/common/IPStackUtils.java: ########## @@ -219,4 +219,91 @@ public static String transformToIPv6(String ipv4, int port) { return ipv4; } } + + /** + * Splits a given input string representing a Hostname or an IP address and port into an `HostPort` object. + * The input string must be in the format of IPv4/IPv6/[IPv6]/hostname:port. + * + * @param input The input string containing the Hostname/IP address and port, in the format + * "IPv4:port", "[IPv6]:port", "IPv6:port", or "hostname:port". + * @return A {@link HostPort} object containing the parsed IP address and port number. + * @throws IllegalArgumentException If the input format is invalid, if the host is null or empty, + * or if the port number is invalid. + */ + public static HostPort splitHostPort(String input) { Review Comment: can we change it to ``getHostAndPort`` ########## common/src/java/org/apache/hive/common/IPStackUtils.java: ########## @@ -219,4 +219,91 @@ public static String transformToIPv6(String ipv4, int port) { return ipv4; } } + + /** + * Splits a given input string representing a Hostname or an IP address and port into an `HostPort` object. + * The input string must be in the format of IPv4/IPv6/[IPv6]/hostname:port. + * + * @param input The input string containing the Hostname/IP address and port, in the format + * "IPv4:port", "[IPv6]:port", "IPv6:port", or "hostname:port". + * @return A {@link HostPort} object containing the parsed IP address and port number. + * @throws IllegalArgumentException If the input format is invalid, if the host is null or empty, + * or if the port number is invalid. + */ + public static HostPort splitHostPort(String input) { + + String host; + int port; + + if (input == null || input.isEmpty()) { Review Comment: Can use ``` if (StringUtils.isEmpty(input)) { ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org