You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "xucq07 (via GitHub)" <gi...@apache.org> on 2026/07/17 06:11:59 UTC
[PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
xucq07 opened a new pull request, #8167:
URL: https://github.com/apache/incubator-seata/pull/8167
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines.
- [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes).
### Ⅰ. Describe what this PR did
When `SEATA_IP` (or `--host`) is set to a host name, the xid carries the host name instead of an ip address, e.g. `seata.aaa.com:8091:123456`.
`Server#start` validates the host with `NetUtil.isValidIp`, which internally calls `convertIpIfNecessary` to resolve a host name to its ip address before validating it. But the validation result is thrown away and the **original** string is stored:
```java
if (NetUtil.isValidIp(parameterParser.getHost(), false)) {
XID.setIpAddress(parameterParser.getHost()); // the raw host name
}
```
The host name then propagates into every xid, and `XIDLoadBalance#select` has to build an `InetSocketAddress` from it on every branch register to compare against the resolved addresses in the channel list. That has three consequences:
1. A DNS lookup on the branch register hot path. The JVM positive cache defaults to 30s, so this is a recurring cost rather than a one-off.
2. If the lookup fails at select time, the `InetSocketAddress` is unresolved. `InetSocketAddress#equals` then compares host name strings against resolved invokers, never matches, and every branch register silently degrades to the random fallback — sticky routing to the tc owning the global transaction is lost.
3. If the name has multiple A records, `getByName` returns the first one, which may point at a different tc than the one that created the xid.
This PR resolves the host name once, at startup, so the xid always carries an ip literal:
- Store `NetUtil.convertIpIfNecessary(host)` instead of the raw host. `isValidIp` on the preceding line already performed the same resolution, so this hits the JVM cache and adds no startup cost.
- Log the `host name -> ip` mapping at INFO, which makes case 3 above diagnosable.
- Make `NetUtil#convertIpIfNecessary` public (it was private and only used by `isValidIp`); behaviour unchanged.
`XIDLoadBalance` is deliberately left alone. Resolving there would repeat work already done by the `InetSocketAddress` constructor and would keep the DNS lookup on the hot path; fixing the value at the source removes the problem for all three cases at once.
The other three `XID.setIpAddress` call sites already pass ip literals (`NetUtil.getIgnoredInterfacesLocalIp` / `NetUtil.getLocalIp`), so `Server.java` is the only place where a host name can reach an xid.
### Ⅱ. Does this pull request fix one issue?
fixes #8158
Note that the root cause differs from the analysis in the issue: `new InetSocketAddress(host, port)` does resolve the name, and `InetSocketAddress#equals` compares the resolved `InetAddress` while ignoring the host name, so a host name and an ip do match in the common case. The failure only appears when resolution fails at select time or when the name resolves to a different address, as described above.
### Ⅲ. Why don't you add test cases (unit test/integration test)?
Added `NetUtilTest#testConvertIpIfNecessary`, covering the ipv4/ipv6 literal pass-through, host name resolution (using `localhost`, so it does not depend on external DNS) and the unresolvable host case.
### Ⅳ. Describe how to verify it
`mvn -pl common -am test -Dtest=NetUtilTest`
End to end: start a server with `SEATA_IP` set to a resolvable host name and check that the generated xid carries the ip address rather than the name, and that the startup log records the resolution.
### Ⅴ. Special notes for reviews
`convertIpIfNecessary` throws an unchecked `RuntimeException` wrapping `UnknownHostException` when the host name cannot be resolved, so an unresolvable `SEATA_IP` fails server startup instead of falling back to the local ip branch. That behaviour is pre-existing — `isValidIp` on the preceding line already throws for the same input — and this PR does not change it. It seems worth addressing separately.
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "xucq07 (via GitHub)" <gi...@apache.org>.
xucq07 closed pull request #8167: bugfix: fix xid carrying the host name when SEATA_IP is set to a host name
URL: https://github.com/apache/incubator-seata/pull/8167
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8167:
URL: https://github.com/apache/incubator-seata/pull/8167#discussion_r3649981293
##########
server/src/test/java/org/apache/seata/server/ServerXidHostTest.java:
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.seata.server;
+
+import org.apache.seata.common.util.NetUtil;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+/**
+ * Test the host selection that feeds {@link org.apache.seata.common.XID}.
+ */
+public class ServerXidHostTest {
+
+ /**
+ * An ip literal is stored as is, no resolution happens.
+ */
+ @Test
+ public void testResolveXidHostKeepsIpLiteral() {
+ Assertions.assertEquals("8.210.212.91", Server.resolveXidHost("8.210.212.91"));
+ }
+
+ /**
+ * Guards the regression reported in #8158: a configured host name must be resolved to its ip literal
+ * before it is stored, otherwise the xid carries the host name and XIDLoadBalance cannot match the tc.
+ */
+ @Test
+ public void testResolveXidHostConvertsHostNameToIpLiteral() throws UnknownHostException {
+ // the local host name is the only name guaranteed to resolve without external dns
+ String hostName = InetAddress.getLocalHost().getHostName();
+ // in some environments the host name is already an ip literal, then there is nothing to convert
+ Assumptions.assumeFalse(NetUtil.isValidIp(hostName, true), "host name is already an ip literal");
+
+ String resolved = Server.resolveXidHost(hostName);
+
+ // the value stored in the xid must be the resolved ip literal, never the host name
+ Assertions.assertNotEquals(hostName, resolved);
+ Assertions.assertTrue(NetUtil.isValidIp(resolved, true), "resolved host is not an ip literal: " + resolved);
Review Comment:
In this test, `Assumptions.assumeFalse(NetUtil.isValidIp(hostName, true), ...)` is incorrect for detecting whether `hostName` is already an IP literal. `isValidIp` resolves host names via `convertIpIfNecessary`, so it will often return `true` for a *non-literal* host name and skip the test, leaving the regression untested in typical environments. Consider instead computing the resolved address via `InetAddress.getByName(hostName)` and assuming it resolves to a non-forbidden literal, then asserting `resolveXidHost(hostName)` equals that literal.
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8167:
URL: https://github.com/apache/incubator-seata/pull/8167#issuecomment-5065665131
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/8167?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.06%. Comparing base ([`e6d0860`](https://app.codecov.io/gh/apache/incubator-seata/commit/e6d0860a4345b10cb59c65c78215ec51d67f59d1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`05d93f9`](https://app.codecov.io/gh/apache/incubator-seata/commit/05d93f9565ffcc6e97bdd6651cc8bc798a5aa014?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on 2.x.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #8167 +/- ##
============================================
+ Coverage 72.88% 73.06% +0.18%
Complexity 1141 1141
============================================
Files 1151 1151
Lines 42272 42272
Branches 5045 5045
============================================
+ Hits 30810 30888 +78
+ Misses 8996 8906 -90
- Partials 2466 2478 +12
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/8167?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ain/java/org/apache/seata/common/util/NetUtil.java](https://app.codecov.io/gh/apache/incubator-seata/pull/8167?src=pr&el=tree&filepath=common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Fcommon%2Futil%2FNetUtil.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zZWF0YS9jb21tb24vdXRpbC9OZXRVdGlsLmphdmE=) | `63.85% <ø> (+0.60%)` | :arrow_up: |
... and [24 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/8167/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/8167?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8167:
URL: https://github.com/apache/incubator-seata/pull/8167#discussion_r3601362905
##########
common/src/test/java/org/apache/seata/common/util/NetUtilTest.java:
##########
@@ -227,6 +227,25 @@ 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");
Review Comment:
`InetAddress#getHostAddress()` may return the IPv6 loopback as either `0:0:0:0:0:0:0:1` or the compressed form `::1` depending on JDK/OS. The current assertion can be flaky on environments that return `::1`.
##########
common/src/test/java/org/apache/seata/common/util/NetUtilTest.java:
##########
@@ -227,6 +227,25 @@ 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");
+
+ // a reserved name from RFC 2606, so it cannot be resolved by a dns search domain
+ assertThatThrownBy(() -> {
+ NetUtil.convertIpIfNecessary("unresolvable.invalid");
+ })
Review Comment:
`unresolvable.invalid` can still be affected by DNS search domains (e.g. resolving `unresolvable.invalid.<searchDomain>`). Adding a trailing dot forces an absolute FQDN and makes the test more robust.
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] bugfix: fix xid carrying the host name when SEATA_IP is set to a host name [incubator-seata]
Posted by "pyshiweijia (via GitHub)" <gi...@apache.org>.
pyshiweijia commented on PR #8167:
URL: https://github.com/apache/incubator-seata/pull/8167#issuecomment-5001997799
I checked the current head (`a329bcaf`) locally with:
`mvn -pl common -am test -Dtest=NetUtilTest -Dsurefire.failIfNoSpecifiedTests=false -Dlicense.skip=true -B`
All 31 `NetUtilTest` cases pass. I found two follow-ups before merge:
1. Both `changes/en-us/2.x.md` and `changes/zh-cn/2.x.md` link to `#8159`, but this PR is `#8167`.
2. The new test covers `convertIpIfNecessary`, whose resolution behavior already existed; it would still pass if `Server` accidentally kept passing `rawHost` to `XID.setIpAddress`. Could you add focused coverage that configures a hostname and asserts that the value assigned to `XID` is the resolved IP literal? Extracting the host-selection/assignment logic into a package-private method may make that path easy to test.
That second test would directly guard the regression reported in #8158.
--
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-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org