Copilot commented on code in PR #8167:
URL: https://github.com/apache/incubator-seata/pull/8167#discussion_r3600876417
##########
common/src/test/java/org/apache/seata/common/util/NetUtilTest.java:
##########
@@ -227,6 +227,24 @@ public void testIsValidIp() {
.hasMessageContaining("UnknownHostException");
}
+ @Test
+ public void testConvertIpIfNecessary() {
+ // an ip literal is returned as is
+
assertThat(NetUtil.convertIpIfNecessary("127.0.0.1")).isEqualTo("127.0.0.1");
+
assertThat(NetUtil.convertIpIfNecessary("8.210.212.91")).isEqualTo("8.210.212.91");
+
assertThat(NetUtil.convertIpIfNecessary("2000:0000:0000:0000:0001:2345:6789:abcd"))
+ .isEqualTo("2000:0000:0000:0000:0001:2345:6789:abcd");
+
+ // a host name is resolved to its ip address
+
assertThat(NetUtil.convertIpIfNecessary("localhost")).isIn("127.0.0.1",
"0:0:0:0:0:0:0:1");
+
+ assertThatThrownBy(() -> {
+ NetUtil.convertIpIfNecessary("knownHost");
+ })
+ .isInstanceOf(RuntimeException.class)
+ .hasMessageContaining("UnknownHostException");
+ }
Review Comment:
The test uses `"knownHost"` as the unresolvable hostname. Unqualified
single-label names can resolve unexpectedly in environments with DNS search
domains (making the test flaky). Prefer a reserved non-existent name like
`*.invalid` (RFC 2606) for a deterministic UnknownHostException.
##########
server/src/main/java/org/apache/seata/server/Server.java:
##########
@@ -80,7 +84,14 @@ public void start(String[] args) {
// 127.0.0.1 and 0.0.0.0 are not valid here.
if (NetUtil.isValidIp(parameterParser.getHost(), false)) {
- XID.setIpAddress(parameterParser.getHost());
+ // a host name must be converted to its ip address, otherwise the
xid carries the host name and
+ // XIDLoadBalance has to resolve it on every branch register, or
fails to match the tc at all.
+ String host =
NetUtil.convertIpIfNecessary(parameterParser.getHost());
+ if (!host.equals(parameterParser.getHost())) {
Review Comment:
`parameterParser.getHost()` is resolved/validated multiple times here:
`isValidIp()` already calls `convertIpIfNecessary()`, and the subsequent
explicit `convertIpIfNecessary()` may trigger a second DNS lookup (or even
resolve to a different A record if DNS caching is disabled). Resolve once,
reuse the raw value for logging, and validate the resolved literal to ensure
the value you store in `XID` is the one that was validated.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
