From 9177515be224269dc0299eae809591cb373d83a0 Mon Sep 17 00:00:00 2001 From: Abhishek Radhakrishnan Date: Thu, 12 May 2022 01:06:20 -0400 Subject: [PATCH] Add IPAddress java library as dependency and migrate IPv4 functions to use the new library. (#11634) * Add ipaddress library as dependency. * IPv4 functions to use the inet.ipaddr package. * Remove unused imports. * Add new function. * Minor rename. * Add more unit tests. * IPv4 address expr utils unit tests and address options. * Adjust the IPv4Util functions. * Move the UTs a bit around. * Javadoc comments. * Add license info for IPAddress. * Fix groupId, artifact and version in license.yaml. * Remove redundant subnet in messages - fixes UT. * Remove unused commons-net dependency for /processing project. * Make class and methods public so it can be accessed. * Add initial version of benchmark * Add subnetutils package for benchmarks. * Auto generate ip addresses. * Add more v4 address representations in setup to avoid bias. * Use ThreadLocalRandom to avoid forbidden API usage. * Adjust IPv4AddressBenchmark to adhere to codestyle rules. * Update ipaddress library to latest 5.3.4 * Add ipaddress package dependency to benchmarks project. --- benchmarks/pom.xml | 10 +- .../druid/benchmark/IPv4AddressBenchmark.java | 279 ++++++++++++++++++ licenses.yaml | 10 + pom.xml | 5 + processing/pom.xml | 5 +- .../expression/IPv4AddressExprUtils.java | 75 +++-- .../expression/IPv4AddressMatchExprMacro.java | 32 +- .../expression/IPv4AddressParseExprMacro.java | 4 +- .../IPv4AddressStringifyExprMacro.java | 10 +- .../expression/IPv4AddressExprUtilsTest.java | 93 ++++-- 10 files changed, 439 insertions(+), 84 deletions(-) create mode 100644 benchmarks/src/test/java/org/apache/druid/benchmark/IPv4AddressBenchmark.java diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index f6bb53e66ac..869c6642704 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -37,6 +37,10 @@ ${jmh.version} test + + commons-net + commons-net + org.openjdk.jmh jmh-generator-annprocess @@ -174,7 +178,11 @@ com.google.inject guice - + + com.github.seancfoley + ipaddress + 5.3.4 + junit junit diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/IPv4AddressBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/IPv4AddressBenchmark.java new file mode 100644 index 00000000000..43263c6b938 --- /dev/null +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/IPv4AddressBenchmark.java @@ -0,0 +1,279 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.benchmark; + +import com.google.common.net.InetAddresses; +import inet.ipaddr.IPAddress; +import inet.ipaddr.IPAddressString; +import inet.ipaddr.ipv4.IPv4Address; +import org.apache.commons.net.util.SubnetUtils; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.query.expression.ExprUtils; +import org.apache.druid.query.expression.IPv4AddressExprUtils; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +import java.net.Inet4Address; +import java.net.InetAddress; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; + + +@State(Scope.Benchmark) +@Fork(value = 1) +@Warmup(iterations = 5) +@Measurement(iterations = 5) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +public class IPv4AddressBenchmark +{ + private static final Pattern IPV4_PATTERN = Pattern.compile( + "^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$" + ); + // Different representations of IPv4 addresses. + private static List IPV4_ADDRESS_STRS; + private static List IPV4_ADDRESSES; + private static List INET4_ADDRESSES; + private static List IPV4_ADDRESS_LONGS; + private static List IPV4_SUBNETS; + @Param({"10", "100", "1000"}) + public int numOfAddresses; + + static boolean isValidAddress(String string) + { + return string != null && IPV4_PATTERN.matcher(string).matches(); + } + + public static void main(String[] args) throws RunnerException + { + Options opt = new OptionsBuilder() + .include(IPv4AddressBenchmark.class.getSimpleName()) + .forks(1) + .build(); + new Runner(opt).run(); + } + + @Setup + public void setUp() + { + IPV4_ADDRESS_STRS = new ArrayList<>(numOfAddresses); + IPV4_ADDRESSES = new ArrayList<>(numOfAddresses); + INET4_ADDRESSES = new ArrayList<>(numOfAddresses); + IPV4_SUBNETS = new ArrayList<>(numOfAddresses); + IPV4_ADDRESS_LONGS = new ArrayList<>(numOfAddresses); + + for (int i = 0; i < numOfAddresses; i++) { + Random r = ThreadLocalRandom.current(); + String genIpAddress = r.nextInt(256) + "." + r.nextInt(256) + "." + r.nextInt(256) + "." + r.nextInt(256); + IPAddressString ipAddressString = new IPAddressString(genIpAddress); + IPAddress prefixBlock = ipAddressString.getAddress().applyPrefixLength(r.nextInt(32)).toPrefixBlock(); + IPV4_ADDRESS_STRS.add(ipAddressString.toString()); + IPV4_SUBNETS.add(prefixBlock.toString()); + + IPv4Address iPv4Address = ipAddressString.getAddress().toIPv4(); + + IPV4_ADDRESSES.add(iPv4Address); + INET4_ADDRESSES.add(iPv4Address.toInetAddress()); + IPV4_ADDRESS_LONGS.add(iPv4Address.longValue()); + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void stringContainsUsingIpAddr() + { + for (int i = 0; i < IPV4_ADDRESS_STRS.size(); i++) { + String v4Subnet = IPV4_SUBNETS.get(i); + String v4Address = IPV4_ADDRESS_STRS.get(i); + + IPAddressString subnetString = new IPAddressString(v4Subnet); + IPv4Address iPv4Address = IPv4AddressExprUtils.parse(v4Address); + if (iPv4Address != null) { + subnetString.contains(iPv4Address.toAddressString()); + } + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void stringContainsUsingSubnetUtils() throws IAE + { + for (int i = 0; i < IPV4_ADDRESS_STRS.size(); i++) { + String v4Subnet = IPV4_SUBNETS.get(i); + String v4Address = IPV4_ADDRESS_STRS.get(i); + + SubnetUtils.SubnetInfo subnetInfo = getSubnetInfo(v4Subnet); + if (isValidAddress(v4Address)) { + subnetInfo.isInRange(v4Address); + } + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void parseLongUsingIpAddr() + { + for (Long v4Address : IPV4_ADDRESS_LONGS) { + IPv4AddressExprUtils.parse(v4Address); + + } + } + + private Inet4Address parseUsingSubnetUtils(long longValue) + { + if (IPv4AddressExprUtils.overflowsUnsignedInt(longValue)) { + return InetAddresses.fromInteger((int) longValue); + } + return null; + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void parseLongUsingSubnetUtils() + { + for (Long v4Address : IPV4_ADDRESS_LONGS) { + parseUsingSubnetUtils(v4Address); + } + } + + private long toLongUsingSubnetUtils(Inet4Address address) + { + int value = InetAddresses.coerceToInteger(address); + return Integer.toUnsignedLong(value); + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void toLongUsingSubnetUtils() + { + for (Inet4Address v4InetAddress : INET4_ADDRESSES) { + toLongUsingSubnetUtils(v4InetAddress); + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void toLongUsingIpAddr() + { + for (IPv4Address v4Address : IPV4_ADDRESSES) { + IPv4AddressExprUtils.toLong(v4Address); + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void longContainsUsingSubnetUtils() + { + for (int i = 0; i < IPV4_ADDRESS_LONGS.size(); i++) { + long v4Long = IPV4_ADDRESS_LONGS.get(i); + String v4Subnet = IPV4_SUBNETS.get(i); + SubnetUtils.SubnetInfo subnetInfo = getSubnetInfo(v4Subnet); + + if (!IPv4AddressExprUtils.overflowsUnsignedInt(v4Long)) { + subnetInfo.isInRange((int) v4Long); + } + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void longContainsUsingIpAddr() + { + for (int i = 0; i < IPV4_ADDRESS_LONGS.size(); i++) { + long v4Long = IPV4_ADDRESS_LONGS.get(i); + String v4Subnet = IPV4_SUBNETS.get(i); + + IPv4Address iPv4Address = IPv4AddressExprUtils.parse(v4Long); + IPAddressString subnetString = new IPAddressString(v4Subnet); + if (iPv4Address != null) { + subnetString.contains(iPv4Address.toAddressString()); + } + } + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void parseStringUsingIpAddr() + { + for (String ipv4Addr : IPV4_ADDRESS_STRS) { + IPv4AddressExprUtils.parse(ipv4Addr); + } + } + + private Inet4Address parseUsingSubnetUtils(String string) + { + if (isValidAddress(string)) { + InetAddress address = InetAddresses.forString(string); + if (address instanceof Inet4Address) { + return (Inet4Address) address; + } + } + return null; + } + + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.NANOSECONDS) + public void parseStringUsingSubnetUtils() + { + for (String ipv4Addr : IPV4_ADDRESS_STRS) { + parseUsingSubnetUtils(ipv4Addr); + } + } + + private SubnetUtils.SubnetInfo getSubnetInfo(String subnet) + { + SubnetUtils subnetUtils; + try { + subnetUtils = new SubnetUtils(subnet); + } + catch (IllegalArgumentException e) { + throw new IAE(e, ExprUtils.createErrMsg("getSubnetInfo()", " arg has an invalid format: " + subnet)); + } + subnetUtils.setInclusiveHostCount(true); // make network and broadcast addresses match + return subnetUtils.getInfo(); + } +} diff --git a/licenses.yaml b/licenses.yaml index d977d1a36e4..2b80d04db41 100644 --- a/licenses.yaml +++ b/licenses.yaml @@ -596,6 +596,16 @@ libraries: --- +name: IPAddress +license_category: binary +module: java-core +license_name: Apache License version 2.0 +version: 5.3.4 +libraries: + - com.github.seancfoley: ipaddress + +--- + name: Apache Commons Collections license_category: binary module: java-core diff --git a/pom.xml b/pom.xml index 464222b198f..949722f22c4 100644 --- a/pom.xml +++ b/pom.xml @@ -246,6 +246,11 @@ commons-net 3.6 + + com.github.seancfoley + ipaddress + 5.3.4 + org.apache.commons commons-lang3 diff --git a/processing/pom.xml b/processing/pom.xml index fedf9a113e9..cd41197eee5 100644 --- a/processing/pom.xml +++ b/processing/pom.xml @@ -96,8 +96,9 @@ commons-math3 - commons-net - commons-net + com.github.seancfoley + ipaddress + 5.3.4 com.google.errorprone diff --git a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressExprUtils.java b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressExprUtils.java index 4d87b38b50f..f91015d8aa2 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressExprUtils.java +++ b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressExprUtils.java @@ -19,66 +19,81 @@ package org.apache.druid.query.expression; -import com.google.common.net.InetAddresses; +import inet.ipaddr.IPAddressString; +import inet.ipaddr.IPAddressStringParameters; +import inet.ipaddr.ipv4.IPv4Address; import javax.annotation.Nullable; -import java.net.Inet4Address; -import java.net.InetAddress; -import java.util.regex.Pattern; -class IPv4AddressExprUtils +public class IPv4AddressExprUtils { - private static final Pattern IPV4_PATTERN = Pattern.compile( - "^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$" - ); + + private static final IPAddressStringParameters IPV4_ADDRESS_PARAMS = new IPAddressStringParameters.Builder().allowSingleSegment(false).allow_inet_aton(false).allowIPv6(false).allowPrefix(false).allowEmpty(false).toParams(); + private static final IPAddressStringParameters IPV4_SUBNET_PARAMS = new IPAddressStringParameters.Builder().allowSingleSegment(false).allow_inet_aton(false).allowEmpty(false).allowIPv6(false).toParams(); /** * @return True if argument cannot be represented by an unsigned integer (4 bytes), else false */ - static boolean overflowsUnsignedInt(long value) + public static boolean overflowsUnsignedInt(long value) { return value < 0L || 0xff_ff_ff_ffL < value; } /** - * @return True if argument is a valid IPv4 address dotted-decimal string + * @return True if argument is a valid IPv4 address dotted-decimal string. Single segments, Inet addresses and subnets are not allowed. */ - static boolean isValidAddress(@Nullable String string) + static boolean isValidIPv4Address(@Nullable String addressString) { - return string != null && IPV4_PATTERN.matcher(string).matches(); + return addressString != null && new IPAddressString(addressString, IPV4_ADDRESS_PARAMS).isIPv4(); } - @Nullable - static Inet4Address parse(@Nullable String string) + /** + * @return True if argument is a valid IPv4 subnet address. + */ + static boolean isValidIPv4Subnet(@Nullable String subnetString) { - // Explicitly check for valid address to avoid overhead of InetAddresses#forString() potentially - // throwing IllegalArgumentException - if (isValidAddress(string)) { - // Do not use java.lang.InetAddress#getByName() as it may do DNS lookups - InetAddress address = InetAddresses.forString(string); - if (address instanceof Inet4Address) { - return (Inet4Address) address; - } + return subnetString != null && new IPAddressString(subnetString, IPV4_SUBNET_PARAMS).isPrefixed(); + } + + /** + * @return IPv4 address if the supplied string is a valid dotted-decimal IPv4 Address string. + */ + @Nullable + public static IPv4Address parse(@Nullable String string) + { + IPAddressString ipAddressString = new IPAddressString(string, IPV4_ADDRESS_PARAMS); + if (ipAddressString.isIPv4()) { + return ipAddressString.getAddress().toIPv4(); } return null; } - static Inet4Address parse(int value) + /** + * @return IPv4 address if the supplied integer is a valid IPv4 integer number. + */ + @Nullable + public static IPv4Address parse(long value) { - return InetAddresses.fromInteger(value); + if (!overflowsUnsignedInt(value)) { + return new IPv4Address((int) value); + } + return null; } /** - * @return IPv4 address dotted-decimal notated string + * @return IPv4 address dotted-decimal canonical string. */ - static String toString(Inet4Address address) + public static String toString(IPv4Address address) { - return address.getHostAddress(); + return address.toString(); } - static long toLong(Inet4Address address) + /** + * + * @return IPv4 address as an integer. + */ + public static long toLong(IPv4Address address) { - int value = InetAddresses.coerceToInteger(address); - return Integer.toUnsignedLong(value); + return address.longValue(); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacro.java index 4203a7f0c8a..33d991aa1d0 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressMatchExprMacro.java @@ -19,7 +19,8 @@ package org.apache.druid.query.expression; -import org.apache.commons.net.util.SubnetUtils; +import inet.ipaddr.IPAddressString; +import inet.ipaddr.ipv4.IPv4Address; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.math.expr.Expr; @@ -70,17 +71,17 @@ public class IPv4AddressMatchExprMacro implements ExprMacroTable.ExprMacro throw new IAE(ExprUtils.createErrMsg(name(), "must have 2 arguments")); } - SubnetUtils.SubnetInfo subnetInfo = getSubnetInfo(args); + IPAddressString subnetInfo = getSubnetInfo(args); Expr arg = args.get(0); class IPv4AddressMatchExpr extends ExprMacroTable.BaseScalarUnivariateMacroFunctionExpr { - private final SubnetUtils.SubnetInfo subnetInfo; + private final IPAddressString subnetString; - private IPv4AddressMatchExpr(Expr arg, SubnetUtils.SubnetInfo subnetInfo) + private IPv4AddressMatchExpr(Expr arg, IPAddressString subnetString) { super(FN_NAME, arg); - this.subnetInfo = subnetInfo; + this.subnetString = subnetString; } @Nonnull @@ -104,12 +105,14 @@ public class IPv4AddressMatchExprMacro implements ExprMacroTable.ExprMacro private boolean isStringMatch(String stringValue) { - return IPv4AddressExprUtils.isValidAddress(stringValue) && subnetInfo.isInRange(stringValue); + IPv4Address iPv4Address = IPv4AddressExprUtils.parse(stringValue); + return iPv4Address != null && subnetString.contains(iPv4Address.toAddressString()); } private boolean isLongMatch(long longValue) { - return !IPv4AddressExprUtils.overflowsUnsignedInt(longValue) && subnetInfo.isInRange((int) longValue); + IPv4Address iPv4Address = IPv4AddressExprUtils.parse(longValue); + return iPv4Address != null && subnetString.contains(iPv4Address.toAddressString()); } @Override @@ -135,22 +138,15 @@ public class IPv4AddressMatchExprMacro implements ExprMacroTable.ExprMacro return new IPv4AddressMatchExpr(arg, subnetInfo); } - private SubnetUtils.SubnetInfo getSubnetInfo(List args) + private IPAddressString getSubnetInfo(List args) { String subnetArgName = "subnet"; Expr arg = args.get(ARG_SUBNET); ExprUtils.checkLiteralArgument(name(), arg, subnetArgName); String subnet = (String) arg.getLiteralValue(); - - SubnetUtils subnetUtils; - try { - subnetUtils = new SubnetUtils(subnet); + if (!IPv4AddressExprUtils.isValidIPv4Subnet(subnet)) { + throw new IAE(ExprUtils.createErrMsg(name(), subnetArgName + " arg has an invalid format: " + subnet)); } - catch (IllegalArgumentException e) { - throw new IAE(e, ExprUtils.createErrMsg(name(), subnetArgName + " arg has an invalid format: " + subnet)); - } - subnetUtils.setInclusiveHostCount(true); // make network and broadcast addresses match - - return subnetUtils.getInfo(); + return new IPAddressString(subnet); } } diff --git a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressParseExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressParseExprMacro.java index ff33fd7f3be..9ccd2cd7147 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressParseExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressParseExprMacro.java @@ -19,6 +19,7 @@ package org.apache.druid.query.expression; +import inet.ipaddr.ipv4.IPv4Address; import org.apache.druid.java.util.common.IAE; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; @@ -27,7 +28,6 @@ import org.apache.druid.math.expr.ExpressionType; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.net.Inet4Address; import java.util.List; /** @@ -107,7 +107,7 @@ public class IPv4AddressParseExprMacro implements ExprMacroTable.ExprMacro private static ExprEval evalAsString(ExprEval eval) { - Inet4Address address = IPv4AddressExprUtils.parse(eval.asString()); + IPv4Address address = IPv4AddressExprUtils.parse(eval.asString()); Long value = address == null ? null : IPv4AddressExprUtils.toLong(address); return ExprEval.ofLong(value); } diff --git a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java index f26fdba52f1..b5c9a1166ee 100644 --- a/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java +++ b/processing/src/main/java/org/apache/druid/query/expression/IPv4AddressStringifyExprMacro.java @@ -19,6 +19,7 @@ package org.apache.druid.query.expression; +import inet.ipaddr.ipv4.IPv4Address; import org.apache.druid.java.util.common.IAE; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; @@ -27,7 +28,6 @@ import org.apache.druid.math.expr.ExpressionType; import javax.annotation.Nonnull; import javax.annotation.Nullable; -import java.net.Inet4Address; import java.util.List; /** @@ -106,7 +106,7 @@ public class IPv4AddressStringifyExprMacro implements ExprMacroTable.ExprMacro private static ExprEval evalAsString(ExprEval eval) { - if (IPv4AddressExprUtils.isValidAddress(eval.asString())) { + if (IPv4AddressExprUtils.isValidIPv4Address(eval.asString())) { return eval; } return ExprEval.of(null); @@ -118,12 +118,10 @@ public class IPv4AddressStringifyExprMacro implements ExprMacroTable.ExprMacro return ExprEval.of(null); } - long longValue = eval.asLong(); - if (IPv4AddressExprUtils.overflowsUnsignedInt(longValue)) { + IPv4Address address = IPv4AddressExprUtils.parse(eval.asLong()); + if (address == null) { return ExprEval.of(null); } - - Inet4Address address = IPv4AddressExprUtils.parse((int) longValue); return ExprEval.of(IPv4AddressExprUtils.toString(address)); } } diff --git a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressExprUtilsTest.java b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressExprUtilsTest.java index a13ee9b94da..84076ed6473 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressExprUtilsTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/IPv4AddressExprUtilsTest.java @@ -19,12 +19,11 @@ package org.apache.druid.query.expression; +import inet.ipaddr.IPAddressNetwork; +import inet.ipaddr.ipv4.IPv4Address; import org.junit.Assert; import org.junit.Test; -import java.net.Inet4Address; -import java.net.InetAddress; -import java.net.UnknownHostException; import java.util.Arrays; import java.util.List; @@ -57,10 +56,26 @@ public class IPv4AddressExprUtilsTest "a.2.3.4", // first octet not number "1.a.3.4", // second octet not number "1.2.c.4", // third octet not number - "1.2.3.d" // fourth octet not number + "1.2.3.d", // fourth octet not number + "1.2.3.0/24" // prefixed cidr ); private static final String IPV6_MAPPED = "::ffff:192.168.0.1"; private static final String IPV6_COMPATIBLE = "::192.168.0.1"; + private static final List VALID_IPV4_SUBNETS = Arrays.asList( + "1.1.1.0/24", + "255.255.255.0/18", + "1.2.3.0/21", + "1.0.0.0/8" + ); + + private static final List INVALID_IPV4_SUBNETS = Arrays.asList( + "1.2.3.0/45", // subnet mask too large + "1.1.1/24", // missing octet + "1/24", // missing octets + "1", // missing octets + "::/23", // IPv6 subnet + "1.1.1.1" // no subnet mask + ); @Test public void testOverflowsUnsignedIntTooLow() @@ -93,39 +108,62 @@ public class IPv4AddressExprUtilsTest } @Test - public void testIsValidAddressNull() + public void testIsValidIPv4AddressNull() { - Assert.assertFalse(IPv4AddressExprUtils.isValidAddress(null)); + Assert.assertFalse(IPv4AddressExprUtils.isValidIPv4Address(null)); } @Test - public void testIsValidAddressIPv4() + public void testIsValidIPv4Address() { for (String address : VALID_IPV4_ADDRESSES) { - Assert.assertTrue(getErrMsg(address), IPv4AddressExprUtils.isValidAddress(address)); + Assert.assertTrue(getErrMsg(address), IPv4AddressExprUtils.isValidIPv4Address(address)); } } @Test - public void testIsValidAddressIPv6Mapped() + public void testIsValidIPv4AddressIPv6Mapped() { - Assert.assertFalse(IPv4AddressExprUtils.isValidAddress(IPV6_MAPPED)); + Assert.assertFalse(IPv4AddressExprUtils.isValidIPv4Address(IPV6_MAPPED)); } @Test - public void testIsValidAddressIPv6Compatible() + public void testIsValidIPv4AddressIPv6Compatible() { - Assert.assertFalse(IPv4AddressExprUtils.isValidAddress(IPV6_COMPATIBLE)); + Assert.assertFalse(IPv4AddressExprUtils.isValidIPv4Address(IPV6_COMPATIBLE)); } @Test - public void testIsValidAddressNotIpAddress() + public void testIsValidIPv4AddressNotIpAddress() { for (String address : INVALID_IPV4_ADDRESSES) { - Assert.assertFalse(getErrMsg(address), IPv4AddressExprUtils.isValidAddress(address)); + Assert.assertFalse(getErrMsg(address), IPv4AddressExprUtils.isValidIPv4Address(address)); } } + @Test + public void testIsValidSubnetNull() + { + Assert.assertFalse(IPv4AddressExprUtils.isValidIPv4Subnet(null)); + } + + @Test + public void testIsValidIPv4SubnetValid() + { + for (String address : VALID_IPV4_SUBNETS) { + Assert.assertTrue(getErrMsg(address), IPv4AddressExprUtils.isValidIPv4Subnet(address)); + } + } + + @Test + public void testIsValidIPv4SubnetInvalid() + { + for (String address : INVALID_IPV4_SUBNETS) { + Assert.assertFalse(getErrMsg(address), IPv4AddressExprUtils.isValidIPv4Subnet(address)); + } + } + + @Test public void testParseNull() { @@ -137,9 +175,9 @@ public class IPv4AddressExprUtilsTest { for (String string : VALID_IPV4_ADDRESSES) { String errMsg = getErrMsg(string); - Inet4Address address = IPv4AddressExprUtils.parse(string); + IPv4Address address = IPv4AddressExprUtils.parse(string); Assert.assertNotNull(errMsg, address); - Assert.assertEquals(errMsg, string, address.getHostAddress()); + Assert.assertEquals(errMsg, string, address.toString()); } } @@ -164,26 +202,31 @@ public class IPv4AddressExprUtilsTest } @Test - public void testParseInt() + public void testParseLong() { - Inet4Address address = IPv4AddressExprUtils.parse((int) 0xC0A80001L); - Assert.assertArrayEquals(new byte[]{(byte) 0xC0, (byte) 0xA8, 0x00, 0x01}, address.getAddress()); + IPv4Address address = IPv4AddressExprUtils.parse(0xC0A80001L); + Assert.assertNotNull(address); + Assert.assertArrayEquals(new byte[]{(byte) 0xC0, (byte) 0xA8, 0x00, 0x01}, address.getBytes()); } @Test - public void testToString() throws UnknownHostException + public void testToString() { byte[] bytes = new byte[]{(byte) 192, (byte) 168, 0, 1}; - InetAddress address = InetAddress.getByAddress(bytes); - Assert.assertEquals("192.168.0.1", IPv4AddressExprUtils.toString((Inet4Address) address)); + + IPAddressNetwork.IPAddressGenerator generator = new IPAddressNetwork.IPAddressGenerator(); + IPv4Address iPv4Address = generator.from(bytes).toIPv4(); + Assert.assertEquals("192.168.0.1", IPv4AddressExprUtils.toString(iPv4Address)); } @Test - public void testToLong() throws UnknownHostException + public void testToLong() { byte[] bytes = new byte[]{(byte) 0xC0, (byte) 0xA8, 0x00, 0x01}; - InetAddress address = InetAddress.getByAddress(bytes); - Assert.assertEquals(0xC0A80001L, IPv4AddressExprUtils.toLong((Inet4Address) address)); + + IPAddressNetwork.IPAddressGenerator generator = new IPAddressNetwork.IPAddressGenerator(); + IPv4Address iPv4Address = generator.from(bytes).toIPv4(); + Assert.assertEquals(0xC0A80001L, IPv4AddressExprUtils.toLong(iPv4Address)); } private String getErrMsg(String msg)