Skip to content

Commit cfc9f63

Browse files
BarbatosBarbatos
Barbatos
authored and
Barbatos
committed
fix(p2p): replace Math with StrictMath and fix flaky network tests
- Replace Math.min/max with StrictMath.min/max in NodeEntry and NodeTable to satisfy CI check-math rule (integer operations, results are identical) - Rewrite NetUtilTest.testGetIP with mocked URLConnection instead of calling external IP services (checkip.amazonaws.com, ifconfig.me), covering: valid IP, connection failure, invalid IP, empty response - Remove testExternalIp (covered by mock tests) - Fix testGetLanIP to not depend on www.baidu.com connectivity
1 parent 2e620d2 commit cfc9f63

6 files changed

Lines changed: 125 additions & 68 deletions

File tree

‎p2p/src/main/java/org/tron/p2p/connection/business/detect/NodeDetectService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void work() {
9898
n = MAX_NODE_SLOW_DETECT;
9999
}
100100

101-
n = Math.min(n, nodeStats.size());
101+
n = StrictMath.min(n, nodeStats.size());
102102

103103
for (int i = 0; i < n; i++) {
104104
detect(nodeStats.get(i));

‎p2p/src/main/java/org/tron/p2p/connection/business/pool/ConnPoolService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ private void connect(boolean isFilterActiveNodes) {
160160
// calculate lackSize exclude config activeNodes
161161
int activeLackSize = p2pConfig.getMinActiveConnections() - connectingPeersCount.get();
162162
int size =
163-
Math.max(
163+
StrictMath.max(
164164
p2pConfig.getMinConnections() - connectingPeersCount.get() - passivePeersCount.get(),
165165
activeLackSize);
166166
if (p2pConfig.getMinConnections() <= activePeers.size() && activeLackSize <= 0) {

‎p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeEntry.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public static int distance(byte[] ownerId, byte[] targetId) {
1919
byte[] h1 = targetId;
2020
byte[] h2 = ownerId;
2121

22-
byte[] hash = new byte[Math.min(h1.length, h2.length)];
22+
byte[] hash = new byte[StrictMath.min(h1.length, h2.length)];
2323

2424
for (int i = 0; i < hash.length; i++) {
2525
hash[i] = (byte) (h1[i] ^ h2[i]);

‎p2p/src/main/java/org/tron/p2p/discover/protocol/kad/table/NodeTable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public int getBucketsCount() {
8080

8181
public int getBucketId(NodeEntry e) {
8282
int id = e.getDistance() - 1;
83-
return Math.max(id, 0);
83+
return StrictMath.max(id, 0);
8484
}
8585

8686
public synchronized int getNodesCount() {

‎p2p/src/main/java/org/tron/p2p/dns/tree/Tree.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ private Entry build(List<Entry> leafs) {
5757
List<Entry> subtrees = new ArrayList<>();
5858
while (!leafs.isEmpty()) {
5959
int total = leafs.size();
60-
int n = Math.min(MaxChildren, total);
60+
int n = StrictMath.min(MaxChildren, total);
6161
Entry branch = build(leafs.subList(0, n));
6262

6363
leafs = leafs.subList(n, total);

‎p2p/src/test/java/org/tron/p2p/utils/NetUtilTest.java‎

Lines changed: 120 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
package org.tron.p2p.utils;
22

3+
import static org.mockito.Mockito.mockStatic;
4+
5+
import java.io.ByteArrayInputStream;
36
import java.io.IOException;
47
import java.lang.reflect.Method;
58
import java.net.InetSocketAddress;
6-
import java.net.Socket;
9+
import java.net.URL;
10+
import java.net.URLConnection;
11+
import java.nio.charset.StandardCharsets;
712
import org.junit.Assert;
813
import org.junit.Test;
9-
import org.tron.p2p.base.Constant;
14+
import org.mockito.MockedConstruction;
15+
import org.mockito.MockedStatic;
16+
import org.mockito.Mockito;
1017
import org.tron.p2p.discover.Node;
1118
import org.tron.p2p.protos.Discover;
1219

@@ -49,113 +56,155 @@ public void testValidNode() {
4956

5057
@Test
5158
public void testGetNode() {
52-
Discover.Endpoint endpoint = Discover.Endpoint.newBuilder().setPort(100).build();
59+
Discover.Endpoint endpoint =
60+
Discover.Endpoint.newBuilder().setPort(100).build();
5361
Node node = NetUtil.getNode(endpoint);
5462
Assert.assertEquals(100, node.getPort());
5563
}
5664

5765
@Test
58-
public void testExternalIp() {
59-
String ip = NetUtil.getExternalIpV4();
60-
Assert.assertFalse(ip.startsWith("10."));
61-
Assert.assertFalse(ip.startsWith("192.168."));
62-
Assert.assertFalse(ip.startsWith("172.16."));
63-
Assert.assertFalse(ip.startsWith("172.17."));
64-
Assert.assertFalse(ip.startsWith("172.18."));
65-
Assert.assertFalse(ip.startsWith("172.19."));
66-
Assert.assertFalse(ip.startsWith("172.20."));
67-
Assert.assertFalse(ip.startsWith("172.21."));
68-
Assert.assertFalse(ip.startsWith("172.22."));
69-
Assert.assertFalse(ip.startsWith("172.23."));
70-
Assert.assertFalse(ip.startsWith("172.24."));
71-
Assert.assertFalse(ip.startsWith("172.25."));
72-
Assert.assertFalse(ip.startsWith("172.26."));
73-
Assert.assertFalse(ip.startsWith("172.27."));
74-
Assert.assertFalse(ip.startsWith("172.28."));
75-
Assert.assertFalse(ip.startsWith("172.29."));
76-
Assert.assertFalse(ip.startsWith("172.30."));
77-
Assert.assertFalse(ip.startsWith("172.31."));
66+
public void testGetExternalIpWithMock() throws Exception {
67+
String fakeIp = "203.0.113.42";
68+
URLConnection mockConn = Mockito.mock(URLConnection.class);
69+
Mockito.when(mockConn.getInputStream())
70+
.thenReturn(new ByteArrayInputStream(
71+
fakeIp.getBytes(StandardCharsets.UTF_8)));
72+
73+
try (MockedConstruction<URL> urlMock = Mockito.mockConstruction(URL.class,
74+
(mock, context) -> Mockito.when(mock.openConnection())
75+
.thenReturn(mockConn))) {
76+
77+
Method method = NetUtil.class.getDeclaredMethod(
78+
"getExternalIp", String.class, boolean.class);
79+
method.setAccessible(true);
80+
81+
String ip = (String) method.invoke(null,
82+
"http://mock-service.test", true);
83+
Assert.assertEquals(fakeIp, ip);
84+
}
7885
}
7986

8087
@Test
81-
public void testGetIP() {
82-
// notice: please check that you only have one externalIP
83-
String ip1 = null;
84-
String ip2 = null;
85-
String ip3 = null;
86-
try {
87-
Method method = NetUtil.class.getDeclaredMethod("getExternalIp", String.class, boolean.class);
88+
public void testGetExternalIpReturnsNullOnFailure() throws Exception {
89+
URLConnection mockConn = Mockito.mock(URLConnection.class);
90+
Mockito.when(mockConn.getInputStream())
91+
.thenThrow(new IOException("Connection refused"));
92+
93+
try (MockedConstruction<URL> urlMock = Mockito.mockConstruction(URL.class,
94+
(mock, context) -> Mockito.when(mock.openConnection())
95+
.thenReturn(mockConn))) {
96+
97+
Method method = NetUtil.class.getDeclaredMethod(
98+
"getExternalIp", String.class, boolean.class);
8899
method.setAccessible(true);
89-
ip1 = (String) method.invoke(NetUtil.class, Constant.ipV4Urls.get(0), true);
90-
ip2 = (String) method.invoke(NetUtil.class, Constant.ipV4Urls.get(1), true);
91-
ip3 = (String) method.invoke(NetUtil.class, Constant.ipV4Urls.get(2), true);
92-
} catch (Exception e) {
93-
Assert.fail();
100+
101+
String ip = (String) method.invoke(null,
102+
"http://unreachable.test", true);
103+
Assert.assertNull(ip);
104+
}
105+
}
106+
107+
@Test
108+
public void testGetExternalIpRejectsInvalidIp() throws Exception {
109+
String invalidIp = "not-an-ip";
110+
URLConnection mockConn = Mockito.mock(URLConnection.class);
111+
Mockito.when(mockConn.getInputStream())
112+
.thenReturn(new ByteArrayInputStream(
113+
invalidIp.getBytes(StandardCharsets.UTF_8)));
114+
115+
try (MockedConstruction<URL> urlMock = Mockito.mockConstruction(URL.class,
116+
(mock, context) -> Mockito.when(mock.openConnection())
117+
.thenReturn(mockConn))) {
118+
119+
Method method = NetUtil.class.getDeclaredMethod(
120+
"getExternalIp", String.class, boolean.class);
121+
method.setAccessible(true);
122+
123+
String ip = (String) method.invoke(null,
124+
"http://bad-service.test", true);
125+
Assert.assertNull(ip);
94126
}
95-
String ip4 = NetUtil.getExternalIpV4();
96-
Assert.assertEquals(ip1, ip4);
97-
Assert.assertEquals(ip2, ip4);
98-
Assert.assertEquals(ip3, ip4);
99127
}
100128

101-
private String getLanIP2() {
102-
String lanIP;
103-
try (Socket s = new Socket("www.baidu.com", 80)) {
104-
lanIP = s.getLocalAddress().getHostAddress();
105-
} catch (IOException e) {
106-
lanIP = "127.0.0.1";
129+
@Test
130+
public void testGetExternalIpRejectsEmptyResponse() throws Exception {
131+
URLConnection mockConn = Mockito.mock(URLConnection.class);
132+
Mockito.when(mockConn.getInputStream())
133+
.thenReturn(new ByteArrayInputStream(
134+
"".getBytes(StandardCharsets.UTF_8)));
135+
136+
try (MockedConstruction<URL> urlMock = Mockito.mockConstruction(URL.class,
137+
(mock, context) -> Mockito.when(mock.openConnection())
138+
.thenReturn(mockConn))) {
139+
140+
Method method = NetUtil.class.getDeclaredMethod(
141+
"getExternalIp", String.class, boolean.class);
142+
method.setAccessible(true);
143+
144+
String ip = (String) method.invoke(null,
145+
"http://empty-service.test", true);
146+
Assert.assertNull(ip);
107147
}
108-
return lanIP;
109148
}
110149

111150
@Test
112151
public void testGetLanIP() {
113152
String lanIpv4 = NetUtil.getLanIP();
114153
Assert.assertNotNull(lanIpv4);
115-
String lanIpv4Old = getLanIP2();
116-
Assert.assertEquals(lanIpv4, lanIpv4Old);
154+
// verify it's a valid IPv4 format (not relying on external network)
155+
Assert.assertTrue(
156+
"LAN IP should be valid IPv4 or loopback",
157+
NetUtil.validIpV4(lanIpv4) || "127.0.0.1".equals(lanIpv4));
117158
}
118159

119160
@Test
120161
public void testIPv6Format() {
121162
String std = "fe80:0:0:0:204:61ff:fe9d:f156";
122163
int randomPort = 10001;
123164
String ip1 =
124-
new InetSocketAddress("fe80:0000:0000:0000:0204:61ff:fe9d:f156", randomPort)
165+
new InetSocketAddress(
166+
"fe80:0000:0000:0000:0204:61ff:fe9d:f156", randomPort)
125167
.getAddress()
126168
.getHostAddress();
127169
Assert.assertEquals(ip1, std);
128170

129171
String ip2 =
130-
new InetSocketAddress("fe80::204:61ff:fe9d:f156", randomPort).getAddress().getHostAddress();
172+
new InetSocketAddress("fe80::204:61ff:fe9d:f156", randomPort)
173+
.getAddress()
174+
.getHostAddress();
131175
Assert.assertEquals(ip2, std);
132176

133177
String ip3 =
134-
new InetSocketAddress("fe80:0000:0000:0000:0204:61ff:254.157.241.86", randomPort)
178+
new InetSocketAddress(
179+
"fe80:0000:0000:0000:0204:61ff:254.157.241.86", randomPort)
135180
.getAddress()
136181
.getHostAddress();
137182
Assert.assertEquals(ip3, std);
138183

139184
String ip4 =
140-
new InetSocketAddress("fe80:0:0:0:0204:61ff:254.157.241.86", randomPort)
185+
new InetSocketAddress(
186+
"fe80:0:0:0:0204:61ff:254.157.241.86", randomPort)
141187
.getAddress()
142188
.getHostAddress();
143189
Assert.assertEquals(ip4, std);
144190

145191
String ip5 =
146-
new InetSocketAddress("fe80::204:61ff:254.157.241.86", randomPort)
192+
new InetSocketAddress(
193+
"fe80::204:61ff:254.157.241.86", randomPort)
147194
.getAddress()
148195
.getHostAddress();
149196
Assert.assertEquals(ip5, std);
150197

151198
String ip6 =
152-
new InetSocketAddress("FE80::204:61ff:254.157.241.86", randomPort)
199+
new InetSocketAddress(
200+
"FE80::204:61ff:254.157.241.86", randomPort)
153201
.getAddress()
154202
.getHostAddress();
155203
Assert.assertEquals(ip6, std);
156204

157205
String ip7 =
158-
new InetSocketAddress("[fe80:0:0:0:204:61ff:fe9d:f156]", randomPort)
206+
new InetSocketAddress(
207+
"[fe80:0:0:0:204:61ff:fe9d:f156]", randomPort)
159208
.getAddress()
160209
.getHostAddress();
161210
Assert.assertEquals(ip7, std);
@@ -164,48 +213,56 @@ public void testIPv6Format() {
164213
@Test
165214
public void testParseIpv6() {
166215
InetSocketAddress address1 =
167-
NetUtil.parseInetSocketAddress("[2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:18888");
216+
NetUtil.parseInetSocketAddress(
217+
"[2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:18888");
168218
Assert.assertNotNull(address1);
169219
Assert.assertEquals(18888, address1.getPort());
170220
Assert.assertEquals(
171-
"2600:1f13:908:1b00:e1fd:5a84:251c:a32a", address1.getAddress().getHostAddress());
221+
"2600:1f13:908:1b00:e1fd:5a84:251c:a32a",
222+
address1.getAddress().getHostAddress());
172223

173224
try {
174-
NetUtil.parseInetSocketAddress("[2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:abcd");
225+
NetUtil.parseInetSocketAddress(
226+
"[2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:abcd");
175227
Assert.fail();
176228
} catch (RuntimeException e) {
177229
Assert.assertTrue(true);
178230
}
179231

180232
try {
181-
NetUtil.parseInetSocketAddress("2600:1f13:908:1b00:e1fd:5a84:251c:a32a:18888");
233+
NetUtil.parseInetSocketAddress(
234+
"2600:1f13:908:1b00:e1fd:5a84:251c:a32a:18888");
182235
Assert.fail();
183236
} catch (RuntimeException e) {
184237
Assert.assertTrue(true);
185238
}
186239

187240
try {
188-
NetUtil.parseInetSocketAddress("[2600:1f13:908:1b00:e1fd:5a84:251c:a32a:18888");
241+
NetUtil.parseInetSocketAddress(
242+
"[2600:1f13:908:1b00:e1fd:5a84:251c:a32a:18888");
189243
Assert.fail();
190244
} catch (RuntimeException e) {
191245
Assert.assertTrue(true);
192246
}
193247

194248
try {
195-
NetUtil.parseInetSocketAddress("2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:18888");
249+
NetUtil.parseInetSocketAddress(
250+
"2600:1f13:908:1b00:e1fd:5a84:251c:a32a]:18888");
196251
Assert.fail();
197252
} catch (RuntimeException e) {
198253
Assert.assertTrue(true);
199254
}
200255

201256
try {
202-
NetUtil.parseInetSocketAddress("2600:1f13:908:1b00:e1fd:5a84:251c:a32a");
257+
NetUtil.parseInetSocketAddress(
258+
"2600:1f13:908:1b00:e1fd:5a84:251c:a32a");
203259
Assert.fail();
204260
} catch (RuntimeException e) {
205261
Assert.assertTrue(true);
206262
}
207263

208-
InetSocketAddress address5 = NetUtil.parseInetSocketAddress("192.168.0.1:18888");
264+
InetSocketAddress address5 =
265+
NetUtil.parseInetSocketAddress("192.168.0.1:18888");
209266
Assert.assertNotNull(address5);
210267
}
211268
}

0 commit comments

Comments
 (0)