anmolnar commented on code in PR #2280:
URL: https://github.com/apache/zookeeper/pull/2280#discussion_r2240893709
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -55,9 +62,16 @@ public List<Id> handleAuthentication(HttpServletRequest
request, byte[] authData
// This is a bit weird but we need to return the address and the number of
// bytes (to distinguish between IPv4 and IPv6
private byte[] addr2Bytes(String addr) {
- byte[] b = v4addr2Bytes(addr);
- // TODO Write the v6addr2Bytes
- return b;
+ if (addr.contains(":")) {
+ LOG.info("Attempting to parse as IPv6...");
Review Comment:
Please don't log these messages at info level. We're not interested at all.
Actually there's no additional information in this log message, so you can just
remove it completely.
You might want to add a debug level message in case of a successful parsing:
```
LOG.debug("Address was parsed successfully as V4/V6 address: {}", addr);
```
but I don't think we need that either.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -55,9 +62,16 @@ public List<Id> handleAuthentication(HttpServletRequest
request, byte[] authData
// This is a bit weird but we need to return the address and the number of
// bytes (to distinguish between IPv4 and IPv6
private byte[] addr2Bytes(String addr) {
- byte[] b = v4addr2Bytes(addr);
- // TODO Write the v6addr2Bytes
- return b;
+ if (addr.contains(":")) {
+ LOG.info("Attempting to parse as IPv6...");
+ return v6addr2Bytes(addr);
+ } else if (addr.contains(".")) {
+ LOG.info("Attempting to parse as IPv4...");
+ return v4addr2Bytes(addr);
+ } else {
+ LOG.info("Input string does not resemble an IPv4 or IPv6 address:
{}", addr);
Review Comment:
Warning level.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -81,6 +95,132 @@ private byte[] v4addr2Bytes(String addr) {
return b;
}
+ /**
+ * Validates an IPv6 address string and converts it into a byte array.
+ *
+ * @param ipv6Addr The IPv6 address string to validate.
+ * @return A byte array representing the IPv6 address if valid, or null if
the address
+ * is invalid or cannot be parsed.
+ */
+ public static byte[] v6addr2Bytes(String ipv6Addr) {
+ if (ipv6Addr == null || ipv6Addr.trim().isEmpty()) {
+ LOG.info("Input IPv6 address cannot be null or empty.");
+ return null;
+ }
+
+ // Check for multiple '::' which is invalid
+ if (ipv6Addr.indexOf("::") != ipv6Addr.lastIndexOf("::")) {
+ LOG.info("IPv6 address contains multiple '::' which is invalid:
{}", ipv6Addr);
+ return null;
+ }
+
+ // Split the address by "::" to handle zero compression, -1 to keep
trailing empty strings
+ String[] parts = ipv6Addr.split("::", -1);
+
+ String[] segments1 = new String[0];
+ String[] segments2 = new String[0];
+
+ // Case 1: No "::" (full address)
+ if (parts.length == 1) {
+ segments1 = parts[0].split(":");
+ if (segments1.length != IPV6_SEGMENT_COUNT) {
+ LOG.info("IPv6 address without '::' must have " +
IPV6_SEGMENT_COUNT + " segments: {}", ipv6Addr);
+ return null;
+ }
+ } else if (parts.length == 2) {
+ // Case 2: "::" is present
+ // Handle cases like "::1" or "1::"
+ if (!parts[0].isEmpty()) {
+ segments1 = parts[0].split(":");
+ }
+ if (!parts[1].isEmpty()) {
+ segments2 = parts[1].split(":");
+ }
+
+ // Check if the total number of explicit segments exceeds 8
+ if (segments1.length + segments2.length >= IPV6_SEGMENT_COUNT) {
+ LOG.info("Too many segments in IPv6 address with '::': {}",
ipv6Addr);
+ return null;
+ }
+ } else {
+ // Case 3: Invalid number of parts after splitting by "::" (should
be 1 or 2)
+ LOG.info("Invalid IPv6 address format (unexpected '::' split
result): {}", ipv6Addr);
Review Comment:
Warn or Debug level. I'm not sure if this message is important. I think this
should be a warning message.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -112,11 +253,13 @@ public boolean matches(String id, String aclExpr) {
mask(aclAddr, bits);
byte[] remoteAddr = addr2Bytes(id);
if (remoteAddr == null) {
+ LOG.info("Address is null");
Review Comment:
I have the impression that you added these log messages for debugging
purposes, in which case please remove them.
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -81,6 +95,132 @@ private byte[] v4addr2Bytes(String addr) {
return b;
}
+ /**
+ * Validates an IPv6 address string and converts it into a byte array.
+ *
+ * @param ipv6Addr The IPv6 address string to validate.
+ * @return A byte array representing the IPv6 address if valid, or null if
the address
+ * is invalid or cannot be parsed.
+ */
+ public static byte[] v6addr2Bytes(String ipv6Addr) {
+ if (ipv6Addr == null || ipv6Addr.trim().isEmpty()) {
+ LOG.info("Input IPv6 address cannot be null or empty.");
+ return null;
+ }
+
+ // Check for multiple '::' which is invalid
+ if (ipv6Addr.indexOf("::") != ipv6Addr.lastIndexOf("::")) {
+ LOG.info("IPv6 address contains multiple '::' which is invalid:
{}", ipv6Addr);
Review Comment:
Use regex for the format verification in a single step?
##########
zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/IPAuthenticationProvider.java:
##########
@@ -81,6 +95,132 @@ private byte[] v4addr2Bytes(String addr) {
return b;
}
+ /**
+ * Validates an IPv6 address string and converts it into a byte array.
+ *
+ * @param ipv6Addr The IPv6 address string to validate.
+ * @return A byte array representing the IPv6 address if valid, or null if
the address
+ * is invalid or cannot be parsed.
+ */
+ public static byte[] v6addr2Bytes(String ipv6Addr) {
+ if (ipv6Addr == null || ipv6Addr.trim().isEmpty()) {
+ LOG.info("Input IPv6 address cannot be null or empty.");
+ return null;
+ }
+
+ // Check for multiple '::' which is invalid
+ if (ipv6Addr.indexOf("::") != ipv6Addr.lastIndexOf("::")) {
+ LOG.info("IPv6 address contains multiple '::' which is invalid:
{}", ipv6Addr);
+ return null;
+ }
+
+ // Split the address by "::" to handle zero compression, -1 to keep
trailing empty strings
+ String[] parts = ipv6Addr.split("::", -1);
+
+ String[] segments1 = new String[0];
+ String[] segments2 = new String[0];
+
+ // Case 1: No "::" (full address)
+ if (parts.length == 1) {
+ segments1 = parts[0].split(":");
+ if (segments1.length != IPV6_SEGMENT_COUNT) {
+ LOG.info("IPv6 address without '::' must have " +
IPV6_SEGMENT_COUNT + " segments: {}", ipv6Addr);
+ return null;
+ }
+ } else if (parts.length == 2) {
+ // Case 2: "::" is present
+ // Handle cases like "::1" or "1::"
+ if (!parts[0].isEmpty()) {
+ segments1 = parts[0].split(":");
+ }
+ if (!parts[1].isEmpty()) {
+ segments2 = parts[1].split(":");
+ }
+
+ // Check if the total number of explicit segments exceeds 8
+ if (segments1.length + segments2.length >= IPV6_SEGMENT_COUNT) {
+ LOG.info("Too many segments in IPv6 address with '::': {}",
ipv6Addr);
+ return null;
+ }
+ } else {
+ // Case 3: Invalid number of parts after splitting by "::" (should
be 1 or 2)
+ LOG.info("Invalid IPv6 address format (unexpected '::' split
result): {}", ipv6Addr);
+ return null;
+ }
+
+ List<Byte> byteList = new ArrayList<>();
+
+ try {
+ // Process segments before "::"
+ for (String segment : segments1) {
+ if (isInvalidSegment(segment)) {
+ LOG.info("Invalid IPv6 segment: '{}' in address: {}",
segment, ipv6Addr);
+ return null;
+ }
+ int value = Integer.parseInt(segment, 16);
+ byteList.add((byte) ((value >> 8) & 0xFF));
+ byteList.add((byte) (value & 0xFF));
+ }
+
+ // Add zero segments for "::" compression
+ int missingSegments = IPV6_SEGMENT_COUNT - (segments1.length +
segments2.length);
+ for (int i = 0; i < missingSegments; i++) {
+ byteList.add((byte) 0x00);
+ byteList.add((byte) 0x00);
+ }
+
+ // Process segments after "::"
+ for (String segment : segments2) {
+ if (isInvalidSegment(segment)) {
+ LOG.info("Invalid IPv6 segment: '{}' in address: {}",
segment, ipv6Addr);
+ return null;
+ }
+ int value = Integer.parseInt(segment, 16);
+ byteList.add((byte) ((value >> 8) & 0xFF));
+ byteList.add((byte) (value & 0xFF));
+ }
+
+ } catch (NumberFormatException e) {
+ // 3. Catch NumberFormatException if String cannot be parsed
+ LOG.info("Invalid hexadecimal format in IPv6 address: {} - {}",
ipv6Addr, e.getMessage());
+ return null;
+ }
+
+ // 4. Return null if address in out of bounds (i.e., not exactly 16
bytes)
+ if (byteList.size() != IPV6_BYTE_LENGTH) {
+ LOG.info("Parsed IPv6 address byte length is incorrect. Expected "
+ IPV6_BYTE_LENGTH + ", got {}: {}", byteList.size(), ipv6Addr);
Review Comment:
debug level
--
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: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
