Skip to content

Commit a3fd75c

Browse files
CodeCasterXclaude
CodeCasterX
and
claude
committed
fix: 修复测试端口分配的 TOCTOU 竞态条件 (#396)
**问题描述**： TestUtils.getLocalAvailablePort() 存在 TOCTOU (Time of Check, Time of Use) 竞态条件： ServerSocket 获取可用端口后立即关闭，在 Netty 绑定前可能被其他进程抢占。 **核心改进**： - 使用 Netty bind(0) 自动分配端口，彻底消除 TOCTOU 竞态 - 引入 httpBound/httpsBound 标志明确跟踪绑定状态 - 添加 getActualHttpPort() 和 getActualHttpsPort() 方法 - MockMvcListener 等待服务器启动后动态获取实际端口 - 完善的错误处理和详细的错误消息（区分自动分配和指定端口场景） - 移除 TestUtils.getLocalAvailablePort() 方法 **技术实现**： 1. NettyHttpClassicServer: 支持端口 0，bind().sync() 后从 Channel 获取实际端口 2. HttpClassicServer: 新增接口方法获取实际绑定端口 3. MockMvcListener: 移除端口预分配，改为启动后动态获取 4. DefaultFitTestService: 使用端口 0 和无参构造函数 **测试改进**： - 使用 Mockito mock HttpClassicServer，提升测试隔离性 - 添加 StubBeans 实现返回 mock server - 更新测试断言以匹配新的错误消息 **测试状态**：✅ 所有测试通过 相关 Issue: #396 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 065792c commit a3fd75c

7 files changed

Lines changed: 219 additions & 97 deletions

File tree

‎framework/fit/java/fit-builtin/plugins/fit-http-server-netty/src/main/java/modelengine/fit/http/server/netty/NettyHttpClassicServer.java‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
package modelengine.fit.http.server.netty;
88

9-
import static modelengine.fitframework.inspection.Validation.greaterThan;
9+
import static modelengine.fitframework.inspection.Validation.greaterThanOrEquals;
1010
import static modelengine.fitframework.inspection.Validation.isTrue;
1111
import static modelengine.fitframework.inspection.Validation.lessThanOrEquals;
1212
import static modelengine.fitframework.inspection.Validation.notNull;
@@ -52,6 +52,7 @@
5252
import modelengine.fitframework.value.ValueFetcher;
5353

5454
import java.io.IOException;
55+
import java.net.InetSocketAddress;
5556
import java.security.GeneralSecurityException;
5657
import java.util.Arrays;
5758
import java.util.Collections;
@@ -94,8 +95,10 @@ public class NettyHttpClassicServer implements HttpClassicServer {
9495
log.error("Failed to start netty http server.", exception);
9596
}));
9697
private final ChannelGroup channelGroup = new DefaultChannelGroup(GlobalEventExecutor.INSTANCE);
97-
private volatile int httpPort;
98-
private volatile int httpsPort;
98+
private volatile int httpPort = 0;
99+
private volatile int httpsPort = 0;
100+
private volatile boolean httpBound = false;
101+
private volatile boolean httpsBound = false;
99102
private final boolean isGracefulExit;
100103
private volatile boolean isStarted = false;
101104
private final Lock lock = LockUtils.newReentrantLock();
@@ -131,27 +134,29 @@ public HttpClassicServer bind(int port, boolean isSecure) {
131134
return this;
132135
}
133136
if (isSecure) {
134-
this.httpsPort = greaterThan(port,
137+
this.httpsPort = greaterThanOrEquals(port,
135138
0,
136-
"The port to bind to netty http server cannot be less than 1. [port={0}, isSecure={1}]",
139+
"The port to bind to netty http server cannot be negative. [port={0}, isSecure={1}]",
137140
port,
138141
true);
139142
this.httpsPort = lessThanOrEquals(port,
140143
65535,
141144
"The port to bind to netty http server cannot be more than 65535. [port={0}, isSecure={1}]",
142145
port,
143146
true);
147+
this.httpsBound = true;
144148
} else {
145-
this.httpPort = greaterThan(port,
149+
this.httpPort = greaterThanOrEquals(port,
146150
0,
147-
"The port to bind to netty http server cannot be less than 1. [port={0}, isSecure={1}]",
151+
"The port to bind to netty http server cannot be negative. [port={0}, isSecure={1}]",
148152
port,
149153
false);
150154
this.httpPort = lessThanOrEquals(port,
151155
65535,
152156
"The port to bind to netty http server cannot be more than 65535. [port={0}, isSecure={1}]",
153157
port,
154158
false);
159+
this.httpBound = true;
155160
}
156161
return this;
157162
}
@@ -161,7 +166,7 @@ public void start() {
161166
if (this.isStarted) {
162167
return;
163168
}
164-
isTrue(this.httpPort > 0 || this.httpsPort > 0,
169+
isTrue(this.httpBound || this.httpsBound,
165170
"At least 1 port should be bound to netty http server. [httpPort={0}, httpsPort={1}]",
166171
this.httpPort,
167172
this.httpsPort);
@@ -177,6 +182,22 @@ public boolean isStarted() {
177182
return this.isStarted;
178183
}
179184

185+
@Override
186+
public int getActualHttpPort() {
187+
if (!this.isStarted || !this.httpBound) {
188+
return 0;
189+
}
190+
return Math.max(this.httpPort, 0);
191+
}
192+
193+
@Override
194+
public int getActualHttpsPort() {
195+
if (!this.isStarted || !this.httpsBound) {
196+
return 0;
197+
}
198+
return Math.max(this.httpsPort, 0);
199+
}
200+
180201
@Override
181202
public void stop() {
182203
if (!this.isStarted) {
@@ -202,27 +223,32 @@ private void startServer() {
202223
EventLoopGroup workerGroup = this.createWorkerGroup();
203224
try {
204225
SSLContext sslContext = null;
205-
if (this.httpsPort > 0 && this.httpsConfig.isSslEnabled()) {
226+
if (this.httpsBound && this.httpsConfig != null && this.httpsConfig.isSslEnabled()) {
206227
sslContext = this.createSslContext();
207228
}
208-
ChannelHandler channelHandler = new ChannelInitializerHandler(this,
209-
this.getAssemblerConfig(),
210-
this.httpsPort,
211-
sslContext,
212-
this.httpsConfig);
229+
ChannelHandler channelHandler =
230+
new ChannelInitializerHandler(this, this.getAssemblerConfig(), sslContext, this.httpsConfig);
213231
ServerBootstrap serverBootstrap = new ServerBootstrap();
214232
serverBootstrap.group(bossGroup, workerGroup)
215233
.channel(NioServerSocketChannel.class)
216234
.childHandler(channelHandler);
217-
this.logServerStarted();
218-
if (this.httpPort > 0) {
235+
if (this.httpBound) {
219236
Channel channel = serverBootstrap.bind(this.httpPort).sync().channel();
220237
this.channelGroup.add(channel);
238+
if (this.httpPort == 0) {
239+
this.httpPort = ((InetSocketAddress) channel.localAddress()).getPort();
240+
log.info("HTTP server bound to auto-assigned port: {}", this.httpPort);
241+
}
221242
}
222-
if (this.httpsPort > 0) {
243+
if (this.httpsBound) {
223244
Channel channel = serverBootstrap.bind(this.httpsPort).sync().channel();
224245
this.channelGroup.add(channel);
246+
if (this.httpsPort == 0) {
247+
this.httpsPort = ((InetSocketAddress) channel.localAddress()).getPort();
248+
log.info("HTTPS server bound to auto-assigned port: {}", this.httpsPort);
249+
}
225250
}
251+
this.logServerStarted();
226252
ChannelGroupFuture channelFutures = this.channelGroup.newCloseFuture();
227253
this.isStarted = true;
228254
channelFutures.sync();
@@ -353,17 +379,17 @@ private static class ChannelInitializerHandler extends ChannelInitializer<Socket
353379
"TLS_AES_128_CCM_SHA256"))
354380
.build();
355381

356-
private final int httpsPort;
382+
private final NettyHttpClassicServer server;
357383
private final SSLContext sslContext;
358384
private final ServerConfig.Secure httpsConfig;
359385
private final ProtocolUpgrader upgrader;
360386
private final ProtocolUpgrader secureUpgrader;
361387
private final HttpClassicRequestAssembler assembler;
362388
private final HttpClassicRequestAssembler secureAssembler;
363389

364-
ChannelInitializerHandler(HttpClassicServer server, HttpClassicRequestAssembler.Config assemblerConfig,
365-
int httpsPort, SSLContext sslContext, ServerConfig.Secure httpsConfig) {
366-
this.httpsPort = httpsPort;
390+
ChannelInitializerHandler(NettyHttpClassicServer server, HttpClassicRequestAssembler.Config assemblerConfig,
391+
SSLContext sslContext, ServerConfig.Secure httpsConfig) {
392+
this.server = server;
367393
this.sslContext = sslContext;
368394
this.httpsConfig = httpsConfig;
369395
this.upgrader = new ProtocolUpgrader(server,
@@ -381,7 +407,8 @@ private static class ChannelInitializerHandler extends ChannelInitializer<Socket
381407
@Override
382408
protected void initChannel(SocketChannel ch) {
383409
ChannelPipeline pipeline = ch.pipeline();
384-
if (ch.localAddress().getPort() == this.httpsPort && this.sslContext != null
410+
int httpsPort = this.server.httpsPort;
411+
if (ch.localAddress().getPort() == httpsPort && this.sslContext != null && this.httpsConfig != null
385412
&& this.httpsConfig.isSslEnabled()) {
386413
pipeline.addLast(new SslHandler(this.buildSslEngine(this.sslContext, this.httpsConfig)));
387414
pipeline.addLast(new HttpServerCodec());

‎framework/fit/java/fit-builtin/plugins/fit-server-http/src/main/java/modelengine/fit/server/http/FitHttpServer.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
package modelengine.fit.server.http;
88

9-
import static modelengine.fitframework.inspection.Validation.greaterThan;
9+
import static modelengine.fitframework.inspection.Validation.greaterThanOrEquals;
1010
import static modelengine.fitframework.inspection.Validation.notNull;
1111

1212
import modelengine.fit.http.server.HttpClassicServer;
@@ -67,7 +67,7 @@ public FitHttpServer(HttpClassicServer httpServer, FitHttpHandlerRegistry regist
6767
if (this.httpsOpen) {
6868
Optional<ServerConfig.Secure> secure = httpConfig.secure();
6969
this.httpsPort = secure.flatMap(ServerConfig.Secure::port).orElse(DEFAULT_HTTPS_PORT);
70-
greaterThan(this.httpsPort, 0, "The server https port must be positive.");
70+
greaterThanOrEquals(this.httpsPort, 0, "The server https port must be non-negative.");
7171
log.debug("Config 'server.https.port' is {}.", this.httpsPort);
7272
this.toRegisterHttpsPort = secure.flatMap(ServerConfig.Secure::toRegisterPort).orElse(this.httpsPort);
7373
log.debug("Config 'server.https.to-register-port' is {}.", this.toRegisterHttpsPort);
@@ -78,7 +78,7 @@ public FitHttpServer(HttpClassicServer httpServer, FitHttpHandlerRegistry regist
7878
this.httpOpen = httpConfig.isProtocolEnabled() || !this.httpsOpen;
7979
if (this.httpOpen) {
8080
this.httpPort = httpConfig.port().orElse(DEFAULT_HTTP_PORT);
81-
greaterThan(this.httpPort, 0, "The server http port must be positive.");
81+
greaterThanOrEquals(this.httpPort, 0, "The server http port must be non-negative.");
8282
log.debug("Config 'server.http.port' is {}.", this.httpPort);
8383
this.toRegisterHttpPort = httpConfig.toRegisterPort().orElse(this.httpPort);
8484
log.debug("Config 'server.http.to-register-port' is {}.", this.toRegisterHttpPort);

‎framework/fit/java/fit-builtin/services/fit-http-classic/definition/src/main/java/modelengine/fit/http/server/HttpClassicServer.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,24 @@ public interface HttpClassicServer extends HttpResource {
4949
*/
5050
boolean isStarted();
5151

52+
/**
53+
* 获取 HTTP 服务实际绑定的端口。
54+
* <p>注意：此方法应在服务器启动后（{@link #isStarted()} 返回 {@code true}）调用。</p>
55+
* <p>如果在 {@link #bind(int)} 时传入 {@code 0}，此方法返回操作系统自动分配的实际端口。</p>
56+
*
57+
* @return 表示 HTTP 服务实际绑定的端口号，未绑定或未启动时返回 {@code 0}。
58+
*/
59+
int getActualHttpPort();
60+
61+
/**
62+
* 获取 HTTPS 服务实际绑定的端口。
63+
* <p>注意：此方法应在服务器启动后（{@link #isStarted()} 返回 {@code true}）调用。</p>
64+
* <p>如果在 {@link #bind(int, boolean)} 时传入 {@code 0}，此方法返回操作系统自动分配的实际端口。</p>
65+
*
66+
* @return 表示 HTTPS 服务实际绑定的端口号，未绑定或未启动时返回 {@code 0}。
67+
*/
68+
int getActualHttpsPort();
69+
5270
/**
5371
* 停止 Http 服务器。
5472
*/

‎framework/fit/java/fit-test/fit-test-framework/src/main/java/modelengine/fitframework/test/domain/listener/MockMvcListener.java‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import modelengine.fit.http.client.HttpClassicClientResponse;
1010
import modelengine.fit.http.entity.TextEntity;
1111
import modelengine.fitframework.exception.ClientException;
12+
import modelengine.fit.http.server.HttpClassicServer;
1213
import modelengine.fitframework.test.annotation.EnableMockMvc;
1314
import modelengine.fitframework.test.domain.TestContext;
1415
import modelengine.fitframework.test.domain.mvc.MockController;
@@ -43,16 +44,7 @@ public class MockMvcListener implements TestListener {
4344
private static final long MIN_STARTUP_TIMEOUT = 1_000L;
4445
private static final long MAX_STARTUP_TIMEOUT = 600_000L;
4546

46-
private final int port;
47-
48-
/**
49-
* 通过插件端口初始化 {@link MockMvcListener} 的实例。
50-
*
51-
* @param port 表示插件启动端口的 {code int}。
52-
*/
53-
public MockMvcListener(int port) {
54-
this.port = port;
55-
}
47+
public MockMvcListener() {}
5648

5749
@Override
5850
public Optional<TestContextConfiguration> config(Class<?> clazz) {
@@ -73,18 +65,33 @@ public void beforeTestClass(TestContext context) {
7365
if (AnnotationUtils.getAnnotation(testClass, EnableMockMvc.class).isEmpty()) {
7466
return;
7567
}
76-
MockMvc mockMvc = new MockMvc(this.port);
77-
context.plugin().container().registry().register(mockMvc);
7868
long timeout = this.getStartupTimeout();
7969
long startTime = System.currentTimeMillis();
80-
boolean started = this.isStarted(mockMvc);
70+
HttpClassicServer server = this.getHttpServer(context);
71+
while (!server.isStarted()) {
72+
long elapsed = System.currentTimeMillis() - startTime;
73+
if (elapsed > timeout) {
74+
throw new IllegalStateException(this.buildTimeoutErrorMessage(elapsed, 0));
75+
}
76+
ThreadUtils.sleep(100);
77+
}
78+
int actualPort = server.getActualHttpPort();
79+
if (actualPort <= 0) {
80+
throw new IllegalStateException(StringUtils.format(
81+
"Failed to resolve actual HTTP port from server. [started={0}, httpPort={1}]",
82+
server.isStarted(),
83+
actualPort));
84+
}
85+
MockMvc mockMvc = new MockMvc(actualPort);
86+
context.plugin().container().registry().register(mockMvc);
87+
boolean started = this.isStarted(mockMvc, actualPort);
8188
while (!started) {
8289
long elapsed = System.currentTimeMillis() - startTime;
8390
if (elapsed > timeout) {
84-
throw new IllegalStateException(this.buildTimeoutErrorMessage(elapsed, this.port));
91+
throw new IllegalStateException(this.buildTimeoutErrorMessage(elapsed, actualPort));
8592
}
8693
ThreadUtils.sleep(100);
87-
started = this.isStarted(mockMvc);
94+
started = this.isStarted(mockMvc, actualPort);
8895
}
8996
}
9097

@@ -108,37 +115,62 @@ private long getStartupTimeout() {
108115
}
109116

110117
private String buildTimeoutErrorMessage(long elapsed, int port) {
118+
if (port > 0) {
119+
return StringUtils.format("""
120+
Mock MVC server failed to start within {0}ms. [port={1}]
121+
122+
Possible causes:
123+
1. Port {1} is already in use by another process
124+
2. Network configuration issues
125+
3. Server startup is slower than expected in this environment
126+
127+
Troubleshooting steps:
128+
- Check if port {1} is in use:
129+
* macOS/Linux: lsof -i :{1}
130+
* Windows: netstat -ano | findstr :{1}
131+
- Check server logs for detailed error messages
132+
- If running in a slow environment, increase timeout:
133+
mvn test -D{2}=60000""",
134+
elapsed,
135+
port,
136+
TIMEOUT_PROPERTY_KEY);
137+
}
111138
return StringUtils.format("""
112-
Mock MVC server failed to start within {0}ms. [port={1}]
139+
Mock MVC server failed to start within {0}ms. [auto-assigned port]
113140
114141
Possible causes:
115-
1. Port {1} is already in use by another process
142+
1. Port conflict with another process
116143
2. Network configuration issues
117144
3. Server startup is slower than expected in this environment
118145
119146
Troubleshooting steps:
120-
- Check if port {1} is in use:
121-
* macOS/Linux: lsof -i :{1}
122-
* Windows: netstat -ano | findstr :{1}
123147
- Check server logs for detailed error messages
124148
- If running in a slow environment, increase timeout:
125-
mvn test -D{2}=60000""",
149+
mvn test -D{1}=60000""",
126150
elapsed,
127-
port,
128151
TIMEOUT_PROPERTY_KEY);
129152
}
130153

131-
protected boolean isStarted(MockMvc mockMvc) {
154+
protected boolean isStarted(MockMvc mockMvc, int port) {
132155
MockRequestBuilder builder = MockMvcRequestBuilders.get(MockController.PATH).responseType(String.class);
133156
try (HttpClassicClientResponse<String> response = mockMvc.perform(builder)) {
134157
String content = response.textEntity()
135158
.map(TextEntity::content)
136159
.orElseThrow(() -> new IllegalStateException(StringUtils.format(
137160
"Failed to start mock http server. [port={0}]",
138-
this.port)));
161+
port)));
139162
return Objects.equals(content, MockController.OK);
140163
} catch (IOException | ClientException e) {
141164
return false;
142165
}
143166
}
167+
168+
private HttpClassicServer getHttpServer(TestContext context) {
169+
HttpClassicServer server = context.plugin().container().beans().get(HttpClassicServer.class);
170+
if (server == null) {
171+
throw new IllegalStateException("HttpClassicServer not found in container.");
172+
}
173+
return server;
174+
}
175+
144176
}

‎framework/fit/java/fit-test/fit-test-framework/src/main/java/modelengine/fitframework/test/domain/util/TestUtils.java‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)