Copilot commented on code in PR #6253:
URL: https://github.com/apache/shenyu/pull/6253#discussion_r2601357039
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -223,6 +323,25 @@ private static boolean ipMatch(final String ip, final
String pattern) {
return dp[m][n];
}
+ /**
+ * Check if the network interface is a Docker-related interface.
+ *
+ * @param name network interface name
+ * @return true if it's a Docker network interface
+ */
+ private static boolean isDockerNetworkInterface(final String name) {
+ if (Objects.isNull(name)) {
+ return false;
+ }
+ String lowerName = name.toLowerCase(Locale.ROOT);
+ for (String prefix : DOCKER_NETWORK_PREFIXES) {
+ if (lowerName.startsWith(prefix)) {
+ return true;
+ }
+ }
+ return false;
+ }
Review Comment:
The new `isDockerNetworkInterface()` method lacks test coverage. Given that
the existing test file `IpUtilsTest.java` has comprehensive test coverage for
other methods, this new Docker filtering functionality should be tested to
verify correct identification of Docker network interfaces (docker, br-, veth
prefixes) and proper handling of null/edge cases.
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -244,9 +363,23 @@ private static String getName(final String name) {
* @return the name postfix
*/
private static Integer getNamePostfix(final String name) {
+ LOG.info("getNamePostfix name: {}", name);
Matcher matcher = NET_CARD_PATTERN.matcher(name);
if (matcher.find()) {
- return Integer.parseInt(matcher.group());
+ String numberStr = matcher.group();
+ LOG.info("getNamePostfix matcher.group(): {}", numberStr);
+ // Limit the number length to avoid parsing very large numbers
(e.g., Docker network interface names)
+ // Common network interface suffixes are usually 1-3 digits (e.g.,
eth0, enp3s0)
+ if (numberStr.length() > 4) {
+ LOG.debug("getNamePostfix: number too long, ignoring: {}",
numberStr);
+ return -1;
+ }
+ try {
+ return Integer.parseInt(numberStr);
+ } catch (NumberFormatException e) {
+ LOG.warn("getNamePostfix: failed to parse number '{}' from
interface name '{}'", numberStr, name, e);
+ return -1;
+ }
Review Comment:
The new logic for handling long numbers (> 4 digits) and
NumberFormatException in `getNamePostfix()` lacks test coverage. Given that the
existing test file `IpUtilsTest.java` has comprehensive test coverage, these
new edge case handling branches should be tested to verify behavior with Docker
network interface names that have very long numeric suffixes.
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -244,9 +363,23 @@ private static String getName(final String name) {
* @return the name postfix
*/
private static Integer getNamePostfix(final String name) {
+ LOG.info("getNamePostfix name: {}", name);
Matcher matcher = NET_CARD_PATTERN.matcher(name);
if (matcher.find()) {
- return Integer.parseInt(matcher.group());
+ String numberStr = matcher.group();
+ LOG.info("getNamePostfix matcher.group(): {}", numberStr);
Review Comment:
This INFO level log will be generated for every network interface on the
system during startup/IP detection. This could be very noisy in production and
should be at DEBUG level instead. The `getNamePostfix` method is called for
each network interface during scanning, making this potentially high-volume
logging.
```suggestion
LOG.debug("getNamePostfix matcher.group(): {}", numberStr);
```
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -105,76 +115,166 @@ public static String getHost() {
* @return the host
*/
public static String getHost(final String filterHost) {
- String hostIp = null;
- String pattern = filterHost;
- // filter matching ip
- if ("*".equals(filterHost) || "".equals(filterHost)) {
- pattern = null;
- } else if (Objects.nonNull(filterHost) && !filterHost.contains("*") &&
!isCompleteHost(filterHost)) {
- pattern = filterHost + "*";
- }
+ String pattern = normalizeFilterPattern(filterHost);
// if the progress works under docker environment
// return the host ip about this docker located from environment value
+ String dockerHostIp = getDockerHostIp();
+ if (Objects.nonNull(dockerHostIp)) {
+ return dockerHostIp;
+ }
+
+ return findHostFromNetworkInterfaces(pattern);
+ }
+
+ /**
+ * Normalize filter pattern.
+ *
+ * @param filterHost host filterHost str
+ * @return normalized pattern
+ */
+ private static String normalizeFilterPattern(final String filterHost) {
+ if ("*".equals(filterHost) || "".equals(filterHost)) {
Review Comment:
Inefficient comparison to empty string, check for zero length instead.
```suggestion
if ("*".equals(filterHost) || (filterHost != null &&
filterHost.length() == 0)) {
```
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -244,9 +363,23 @@ private static String getName(final String name) {
* @return the name postfix
*/
private static Integer getNamePostfix(final String name) {
+ LOG.info("getNamePostfix name: {}", name);
Matcher matcher = NET_CARD_PATTERN.matcher(name);
if (matcher.find()) {
- return Integer.parseInt(matcher.group());
+ String numberStr = matcher.group();
+ LOG.info("getNamePostfix matcher.group(): {}", numberStr);
+ // Limit the number length to avoid parsing very large numbers
(e.g., Docker network interface names)
+ // Common network interface suffixes are usually 1-3 digits (e.g.,
eth0, enp3s0)
Review Comment:
The comment states "Common network interface suffixes are usually 1-3
digits" but the code checks for `> 4`, which would allow up to 4 digits. Either
the comment should say "1-4 digits" or the code should check `> 3` to match the
comment. Consider updating to: "Common network interface suffixes are usually
1-4 digits (e.g., eth0, enp3s0)".
```suggestion
// Common network interface suffixes are usually 1-4 digits
(e.g., eth0, enp3s0)
```
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -244,9 +363,23 @@ private static String getName(final String name) {
* @return the name postfix
*/
private static Integer getNamePostfix(final String name) {
+ LOG.info("getNamePostfix name: {}", name);
Review Comment:
This INFO level log will be generated for every network interface on the
system during startup/IP detection. This could be very noisy in production and
should be at DEBUG level instead. The `getNamePostfix` method is called for
each network interface during scanning, making this potentially high-volume
logging.
```suggestion
LOG.debug("getNamePostfix name: {}", name);
```
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -105,76 +115,166 @@ public static String getHost() {
* @return the host
*/
public static String getHost(final String filterHost) {
- String hostIp = null;
- String pattern = filterHost;
- // filter matching ip
- if ("*".equals(filterHost) || "".equals(filterHost)) {
- pattern = null;
- } else if (Objects.nonNull(filterHost) && !filterHost.contains("*") &&
!isCompleteHost(filterHost)) {
- pattern = filterHost + "*";
- }
+ String pattern = normalizeFilterPattern(filterHost);
// if the progress works under docker environment
// return the host ip about this docker located from environment value
+ String dockerHostIp = getDockerHostIp();
+ if (Objects.nonNull(dockerHostIp)) {
+ return dockerHostIp;
+ }
+
+ return findHostFromNetworkInterfaces(pattern);
+ }
+
+ /**
+ * Normalize filter pattern.
+ *
+ * @param filterHost host filterHost str
+ * @return normalized pattern
+ */
+ private static String normalizeFilterPattern(final String filterHost) {
+ if ("*".equals(filterHost) || "".equals(filterHost)) {
+ return null;
+ }
+ if (Objects.nonNull(filterHost) && !filterHost.contains("*") &&
!isCompleteHost(filterHost)) {
+ return filterHost + "*";
+ }
+ return filterHost;
+ }
+
+ /**
+ * Get Docker host IP from environment variables.
+ *
+ * @return Docker host IP if found, null otherwise
+ */
+ private static String getDockerHostIp() {
String dockerHostIp = System.getenv(SYSTEM_ENV_DOCKER_HOST_IP);
if (Objects.nonNull(dockerHostIp) &&
StringUtils.isNoneBlank(dockerHostIp)) {
return dockerHostIp;
}
+ dockerHostIp =
System.getenv(SYSTEM_ENV_DOCKER_HOST_IP.toUpperCase(Locale.ROOT));
+ if (Objects.nonNull(dockerHostIp) &&
StringUtils.isNoneBlank(dockerHostIp)) {
+ return dockerHostIp;
+ }
+ return null;
+ }
Review Comment:
The new `getDockerHostIp()` method lacks test coverage. Given that the
existing test file `IpUtilsTest.java` has comprehensive test coverage for other
methods, this new functionality should be tested to ensure proper handling of
environment variables, including both lowercase and uppercase variants of
`docker_host_ip`.
##########
shenyu-common/src/main/java/org/apache/shenyu/common/utils/IpUtils.java:
##########
@@ -105,76 +115,166 @@ public static String getHost() {
* @return the host
*/
public static String getHost(final String filterHost) {
- String hostIp = null;
- String pattern = filterHost;
- // filter matching ip
- if ("*".equals(filterHost) || "".equals(filterHost)) {
- pattern = null;
- } else if (Objects.nonNull(filterHost) && !filterHost.contains("*") &&
!isCompleteHost(filterHost)) {
- pattern = filterHost + "*";
- }
+ String pattern = normalizeFilterPattern(filterHost);
// if the progress works under docker environment
// return the host ip about this docker located from environment value
+ String dockerHostIp = getDockerHostIp();
+ if (Objects.nonNull(dockerHostIp)) {
+ return dockerHostIp;
+ }
+
+ return findHostFromNetworkInterfaces(pattern);
+ }
+
+ /**
+ * Normalize filter pattern.
+ *
+ * @param filterHost host filterHost str
+ * @return normalized pattern
+ */
+ private static String normalizeFilterPattern(final String filterHost) {
+ if ("*".equals(filterHost) || "".equals(filterHost)) {
+ return null;
+ }
+ if (Objects.nonNull(filterHost) && !filterHost.contains("*") &&
!isCompleteHost(filterHost)) {
+ return filterHost + "*";
+ }
+ return filterHost;
+ }
+
+ /**
+ * Get Docker host IP from environment variables.
+ *
+ * @return Docker host IP if found, null otherwise
+ */
+ private static String getDockerHostIp() {
String dockerHostIp = System.getenv(SYSTEM_ENV_DOCKER_HOST_IP);
if (Objects.nonNull(dockerHostIp) &&
StringUtils.isNoneBlank(dockerHostIp)) {
return dockerHostIp;
}
+ dockerHostIp =
System.getenv(SYSTEM_ENV_DOCKER_HOST_IP.toUpperCase(Locale.ROOT));
+ if (Objects.nonNull(dockerHostIp) &&
StringUtils.isNoneBlank(dockerHostIp)) {
+ return dockerHostIp;
+ }
+ return null;
+ }
- // Traversal Network interface to scan all network interface
+ /**
+ * Find host IP from network interfaces.
+ *
+ * @param pattern IP pattern to match
+ * @return host IP address
+ */
+ private static String findHostFromNetworkInterfaces(final String pattern) {
List<NetCard> ipv4Result = new ArrayList<>();
List<NetCard> ipv6Result = new ArrayList<>();
- NetCard netCard;
try {
- Enumeration<NetworkInterface> enumeration =
NetworkInterface.getNetworkInterfaces();
- while (enumeration.hasMoreElements()) {
- final NetworkInterface networkInterface =
enumeration.nextElement();
- Enumeration<InetAddress> addresses =
networkInterface.getInetAddresses();
- while (addresses.hasMoreElements()) {
- InetAddress inetAddress = addresses.nextElement();
- if (Objects.nonNull(inetAddress) &&
!inetAddress.isLoopbackAddress()) {
- if (inetAddress instanceof Inet4Address &&
isCompleteHost(inetAddress.getHostAddress())) {
- netCard = new NetCard(inetAddress.getHostAddress(),
- getName(networkInterface.getName()),
- getNamePostfix(networkInterface.getName()),
-
Integer.parseInt(inetAddress.getHostAddress().split("\\.")[3]));
- ipv4Result.add(netCard);
- } else {
- netCard = new NetCard(inetAddress.getHostAddress(),
- getName(networkInterface.getName()),
-
getNamePostfix(networkInterface.getName()));
- ipv6Result.add(netCard);
- }
- }
+ scanNetworkInterfaces(ipv4Result, ipv6Result);
+ sortNetCards(ipv4Result, ipv6Result);
+ return selectHostIp(ipv4Result, ipv6Result, pattern);
+ } catch (SocketException ignore) {
+ return LOCALHOST;
+ }
+ }
+
+ /**
+ * Scan network interfaces and collect IP addresses.
+ *
+ * @param ipv4Result IPv4 result list
+ * @param ipv6Result IPv6 result list
+ * @throws SocketException if network interface access fails
+ */
+ private static void scanNetworkInterfaces(final List<NetCard> ipv4Result,
final List<NetCard> ipv6Result)
+ throws SocketException {
+ Enumeration<NetworkInterface> enumeration =
NetworkInterface.getNetworkInterfaces();
+ while (enumeration.hasMoreElements()) {
+ final NetworkInterface networkInterface =
enumeration.nextElement();
+ // Skip Docker network interfaces (e.g., docker0, br-xxx, vethxxx)
+ if (isDockerNetworkInterface(networkInterface.getName())) {
+ continue;
+ }
+ Enumeration<InetAddress> addresses =
networkInterface.getInetAddresses();
+ while (addresses.hasMoreElements()) {
+ InetAddress inetAddress = addresses.nextElement();
+ if (Objects.nonNull(inetAddress) &&
!inetAddress.isLoopbackAddress()) {
+ addNetCard(inetAddress, networkInterface.getName(),
ipv4Result, ipv6Result);
}
}
+ }
+ }
+
+ /**
+ * Add network card to appropriate result list.
+ *
+ * @param inetAddress inet address
+ * @param interfaceName network interface name
+ * @param ipv4Result IPv4 result list
+ * @param ipv6Result IPv6 result list
+ */
+ private static void addNetCard(final InetAddress inetAddress, final String
interfaceName,
+ final List<NetCard> ipv4Result, final
List<NetCard> ipv6Result) {
+ String hostAddress = inetAddress.getHostAddress();
+ if (inetAddress instanceof Inet4Address &&
isCompleteHost(hostAddress)) {
+ NetCard netCard = new NetCard(hostAddress,
+ getName(interfaceName),
+ getNamePostfix(interfaceName),
+ Integer.parseInt(hostAddress.split("\\.")[3]));
Review Comment:
Potential uncaught 'java.lang.NumberFormatException'.
```suggestion
int ipv4Postfix = 0;
try {
String[] segments = hostAddress.split("\\.");
ipv4Postfix = Integer.parseInt(segments[3]);
} catch (NumberFormatException | ArrayIndexOutOfBoundsException
e) {
// Log the error if desired, or leave as default 0
}
NetCard netCard = new NetCard(hostAddress,
getName(interfaceName),
getNamePostfix(interfaceName),
ipv4Postfix);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
