From e55446ab0b37cbe95d5880521403d271fabcf7ca Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sat, 2 Feb 2019 13:45:34 +0100 Subject: [PATCH 1/5] add debug logging Signed-off-by: Jan N. Klug --- .../binding/network/internal/utils/NetworkUtils.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index 152b8db9d58ef..aa18912badf87 100644 --- a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -40,6 +40,8 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.smarthome.io.net.exec.ExecUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Network utility functions for pinging and for determining all interfaces and assigned IP addresses. @@ -48,6 +50,8 @@ */ @NonNullByDefault public class NetworkUtils { + private final Logger logger = LoggerFactory.getLogger(NetworkUtils.class); + /** * Gets every IPv4 Address on each Interface except the loopback * The Address format is ip/subnet @@ -132,10 +136,14 @@ public Set getNetworkIPs(int maximumPerInterface) { public Set getNetworkIPs(Set interfaceIPs, int maximumPerInterface) { LinkedHashSet networkIPs = new LinkedHashSet<>(); + logger.info("{} interface IPs found: {}", interfaceIPs.size(), interfaceIPs); + for (String string : interfaceIPs) { + logger.info("processing IP: {}", string); try { // gets every ip which can be assigned on the given network SubnetUtils utils = new SubnetUtils(string); + logger.info("resolved to network: {}", utils.getInfo()); String[] addresses = utils.getInfo().getAllAddresses(); int len = addresses.length; if (maximumPerInterface != 0 && maximumPerInterface < len) { From 509357e5368162de114792bea19703f58a22975e Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 3 Feb 2019 10:27:16 +0100 Subject: [PATCH 2/5] fix OOM error (CIDR suffix too small results in too many addresses) Signed-off-by: Jan N. Klug --- .../network/internal/utils/NetworkUtils.java | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index aa18912badf87..6897042e53f28 100644 --- a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -33,6 +33,8 @@ import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.SystemUtils; @@ -51,6 +53,8 @@ @NonNullByDefault public class NetworkUtils { private final Logger logger = LoggerFactory.getLogger(NetworkUtils.class); + private static final Pattern IP4_ADDRESS = Pattern + .compile("^(\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3})\\/(\\d+)$"); /** * Gets every IPv4 Address on each Interface except the loopback @@ -136,27 +140,48 @@ public Set getNetworkIPs(int maximumPerInterface) { public Set getNetworkIPs(Set interfaceIPs, int maximumPerInterface) { LinkedHashSet networkIPs = new LinkedHashSet<>(); - logger.info("{} interface IPs found: {}", interfaceIPs.size(), interfaceIPs); + long minCIDRsuffix = 8; // historic Class A network, addresses = 16777214 + if (maximumPerInterface != 0) { + // calculate minimum CIDR suffix from maximumPerInterface (equals leading unset bits (Integer has 32 bits) + minCIDRsuffix = Integer.numberOfLeadingZeros(maximumPerInterface); + // if only the highest is set, increase suffix by 1 to cover all addresses + if (Integer.bitCount(maximumPerInterface) == 1) { + minCIDRsuffix++; + } + } + logger.trace("set minCIDRsuffix to {}, maximumPerInterFace is {}", minCIDRsuffix, maximumPerInterface); for (String string : interfaceIPs) { - logger.info("processing IP: {}", string); - try { - // gets every ip which can be assigned on the given network - SubnetUtils utils = new SubnetUtils(string); - logger.info("resolved to network: {}", utils.getInfo()); - String[] addresses = utils.getInfo().getAllAddresses(); - int len = addresses.length; - if (maximumPerInterface != 0 && maximumPerInterface < len) { - len = maximumPerInterface; + Matcher addressMatcher = IP4_ADDRESS.matcher(string); + if (addressMatcher.matches()) { + if (Integer.valueOf(addressMatcher.group(2)) < minCIDRsuffix) { + logger.info( + "CIDR suffix is smaller than /{} on interface with address {}, truncating to /{}, some addresses might be lost", + minCIDRsuffix, string, minCIDRsuffix); + string = addressMatcher.group(1) + "/" + String.valueOf(minCIDRsuffix); } - for (int i = 0; i < len; i++) { - networkIPs.add(addresses[i]); + try { + // gets every ip which can be assigned on the given network + SubnetUtils utils = new SubnetUtils(string); + // TODO: remove logging + logger.info("resolved to network: {}", utils.getInfo()); + String[] addresses = utils.getInfo().getAllAddresses(); + int len = addresses.length; + if (maximumPerInterface != 0 && maximumPerInterface < len) { + len = maximumPerInterface; + } + for (int i = 0; i < len; i++) { + networkIPs.add(addresses[i]); + } + } catch (Exception ex) { } - } catch (Exception ex) { + } else { + logger.info("skipping interface with address {}: not an IPv4 address", string); } } return networkIPs; + } /** From 4878dde6112b2eebd316c82f0d5e67ac6e85cbfd Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Thu, 7 Feb 2019 18:16:00 +0100 Subject: [PATCH 3/5] remove logging Signed-off-by: Jan N. Klug --- .../openhab/binding/network/internal/utils/NetworkUtils.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index 6897042e53f28..4270f490071d6 100644 --- a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -163,8 +163,6 @@ public Set getNetworkIPs(Set interfaceIPs, int maximumPerInterfa try { // gets every ip which can be assigned on the given network SubnetUtils utils = new SubnetUtils(string); - // TODO: remove logging - logger.info("resolved to network: {}", utils.getInfo()); String[] addresses = utils.getInfo().getAllAddresses(); int len = addresses.length; if (maximumPerInterface != 0 && maximumPerInterface < len) { From c3a901ad4a4fa6aa3f5c2aee03cdfc90d932f240 Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 10 Feb 2019 12:49:01 +0100 Subject: [PATCH 4/5] address review comments Signed-off-by: Jan N. Klug --- .../META-INF/MANIFEST.MF | 1 + .../network/internal/utils/NetworkUtils.java | 100 ++++++------------ 2 files changed, 33 insertions(+), 68 deletions(-) diff --git a/addons/binding/org.openhab.binding.network/META-INF/MANIFEST.MF b/addons/binding/org.openhab.binding.network/META-INF/MANIFEST.MF index cf59b41d2639f..3f446c61b8759 100644 --- a/addons/binding/org.openhab.binding.network/META-INF/MANIFEST.MF +++ b/addons/binding/org.openhab.binding.network/META-INF/MANIFEST.MF @@ -16,6 +16,7 @@ Import-Package: org.eclipse.smarthome.config.discovery, org.eclipse.smarthome.core.cache, org.eclipse.smarthome.core.library.types, + org.eclipse.smarthome.core.net, org.eclipse.smarthome.core.thing, org.eclipse.smarthome.core.thing.binding, org.eclipse.smarthome.core.types, diff --git a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index 4270f490071d6..60e28416cb4b3 100644 --- a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -18,9 +18,9 @@ import java.net.ConnectException; import java.net.DatagramPacket; import java.net.DatagramSocket; +import java.net.Inet4Address; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.InterfaceAddress; import java.net.NetworkInterface; import java.net.NoRouteToHostException; import java.net.PortUnreachableException; @@ -30,17 +30,18 @@ import java.net.SocketTimeoutException; import java.util.Enumeration; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; -import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.SystemUtils; import org.apache.commons.net.util.SubnetUtils; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; +import org.eclipse.smarthome.core.net.CidrAddress; +import org.eclipse.smarthome.core.net.NetUtil; import org.eclipse.smarthome.io.net.exec.ExecUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,39 +63,9 @@ public class NetworkUtils { * * @return The collected IPv4 Addresses */ - public Set getInterfaceIPs() { - Set interfaceIPs = new HashSet<>(); - - Enumeration interfaces; - try { - interfaces = NetworkInterface.getNetworkInterfaces(); - } catch (SocketException ignored) { - // If we are not allowed to enumerate, we return an empty result set. - return interfaceIPs; - } - - // For each interface ... - for (Enumeration en = interfaces; en.hasMoreElements();) { - NetworkInterface networkInterface = en.nextElement(); - boolean isLoopback = true; - try { - isLoopback = networkInterface.isLoopback(); - } catch (SocketException ignored) { - } - if (!isLoopback) { - // .. and for each address ... - for (Iterator it = networkInterface.getInterfaceAddresses().iterator(); it - .hasNext();) { - - // ... get IP and Subnet - InterfaceAddress interfaceAddress = it.next(); - interfaceIPs.add(interfaceAddress.getAddress().getHostAddress() + "/" - + interfaceAddress.getNetworkPrefixLength()); - } - } - } - - return interfaceIPs; + public Set getInterfaceIPs() { + return NetUtil.getAllInterfaceAddresses().stream().filter(a -> a.getAddress() instanceof Inet4Address) + .collect(Collectors.toSet()); } /** @@ -137,49 +108,42 @@ public Set getNetworkIPs(int maximumPerInterface) { * @param maximumPerInterface The maximum of IP addresses per interface or 0 to get all. * @return Every single IP which can be assigned on the Networks the computer is connected to */ - public Set getNetworkIPs(Set interfaceIPs, int maximumPerInterface) { + public Set getNetworkIPs(Set interfaceIPs, int maximumPerInterface) { LinkedHashSet networkIPs = new LinkedHashSet<>(); - long minCIDRsuffix = 8; // historic Class A network, addresses = 16777214 + short minCidrPrefixLength = 8; // historic Class A network, addresses = 16777214 if (maximumPerInterface != 0) { - // calculate minimum CIDR suffix from maximumPerInterface (equals leading unset bits (Integer has 32 bits) - minCIDRsuffix = Integer.numberOfLeadingZeros(maximumPerInterface); - // if only the highest is set, increase suffix by 1 to cover all addresses + // calculate minimum CIDR prefix length from maximumPerInterface + // (equals leading unset bits (Integer has 32 bits) + minCidrPrefixLength = (short) Integer.numberOfLeadingZeros(maximumPerInterface); if (Integer.bitCount(maximumPerInterface) == 1) { - minCIDRsuffix++; + // if only the highest is set, decrease prefix by 1 to cover all addresses + minCidrPrefixLength--; } } - logger.trace("set minCIDRsuffix to {}, maximumPerInterFace is {}", minCIDRsuffix, maximumPerInterface); + logger.trace("set minCidrPrefixLength to {}, maximumPerInterface is {}", minCidrPrefixLength, + maximumPerInterface); - for (String string : interfaceIPs) { - Matcher addressMatcher = IP4_ADDRESS.matcher(string); - if (addressMatcher.matches()) { - if (Integer.valueOf(addressMatcher.group(2)) < minCIDRsuffix) { - logger.info( - "CIDR suffix is smaller than /{} on interface with address {}, truncating to /{}, some addresses might be lost", - minCIDRsuffix, string, minCIDRsuffix); - string = addressMatcher.group(1) + "/" + String.valueOf(minCIDRsuffix); - } - try { - // gets every ip which can be assigned on the given network - SubnetUtils utils = new SubnetUtils(string); - String[] addresses = utils.getInfo().getAllAddresses(); - int len = addresses.length; - if (maximumPerInterface != 0 && maximumPerInterface < len) { - len = maximumPerInterface; - } - for (int i = 0; i < len; i++) { - networkIPs.add(addresses[i]); - } - } catch (Exception ex) { - } - } else { - logger.info("skipping interface with address {}: not an IPv4 address", string); + for (CidrAddress cidrNotation : interfaceIPs) { + if (cidrNotation.getPrefix() < minCidrPrefixLength) { + logger.info( + "CIDR prefix is smaller than /{} on interface with address {}, truncating to /{}, some addresses might be lost", + minCidrPrefixLength, cidrNotation, minCidrPrefixLength); + cidrNotation = new CidrAddress(cidrNotation.getAddress(), minCidrPrefixLength); + } + + SubnetUtils utils = new SubnetUtils(cidrNotation.toString()); + String[] addresses = utils.getInfo().getAllAddresses(); + int len = addresses.length; + if (maximumPerInterface != 0 && maximumPerInterface < len) { + len = maximumPerInterface; + } + for (int i = 0; i < len; i++) { + networkIPs.add(addresses[i]); } } return networkIPs; - } /** From 59c50bf9a926e2d713486ae74ad1f8b75f9a013b Mon Sep 17 00:00:00 2001 From: "Jan N. Klug" Date: Sun, 10 Feb 2019 17:19:23 +0100 Subject: [PATCH 5/5] remove superfluous matcher Signed-off-by: Jan N. Klug --- .../openhab/binding/network/internal/utils/NetworkUtils.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java index 60e28416cb4b3..6028b1910d1ab 100644 --- a/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java +++ b/addons/binding/org.openhab.binding.network/src/main/java/org/openhab/binding/network/internal/utils/NetworkUtils.java @@ -32,7 +32,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; -import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.commons.lang.StringUtils; @@ -54,8 +53,6 @@ @NonNullByDefault public class NetworkUtils { private final Logger logger = LoggerFactory.getLogger(NetworkUtils.class); - private static final Pattern IP4_ADDRESS = Pattern - .compile("^(\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3})\\/(\\d+)$"); /** * Gets every IPv4 Address on each Interface except the loopback