Add guest joining, P2P support, and update documentation for Minecraft LAN bridge#42
Add guest joining, P2P support, and update documentation for Minecraft LAN bridge#42Sakura520222 merged 18 commits intomainfrom
Conversation
- Create comprehensive guide for LAN bridge mod development targeting Minecraft 1.21.1 / NeoForge 21.1.219 - Document IntegratedServer openToLAN invocation and LAN port retrieval methods - List NeoForge client lifecycle events including login, logout, and tick events - Provide event registration examples using @EventBusSubscriber annotation - Document multiplayer server list UI classes (JoinMultiplayerScreen, ServerData, ServerList) - Describe ScreenEvent.Init.Post usage for UI extension and Mixin injection points for lower-level control
- Add RelayControlMessage class for parsing relay control responses with GSON - Add RelayGuestJoiner class to handle joining public relay rooms - Implement join() method to request guest session via WebSocket and start local proxy - Add startProxyAndConnect() to launch RelayGuestProxy and connect to 127.0.0.1 - Add RelayGuestProxy class for local TCP proxy with per-stream socket management - Implement start() to bind to an ephemeral port and accept client connections - Implement handleBinary() to route binary messages to stream-specific sockets
- Remove unused imports TransferState, ResourceLocation, and Map in RelayGuestJoiner - Replace TransferState parameter with null in ConnectScreen.startTransfer call - Add lastLobbyListRequestAt field to track last request timestamp - Skip lobby list refresh if already refreshing or within 1 second cooldown - Set usesAuthentication and preventProxyConnections to false in SingleplayerRelayManager for both published and non-published paths
- Add RELAY_BUFFER_SIZE=64KB and SOCKET_BUFFER_SIZE=256KB constants in RelayGuestProxy and RelayHostBridge - Add guestToRelayBytes and relayToGuestBytes AtomicLong counters in RelayGuestProxy - Add hostToRelayBytes and relayToHostBytes AtomicLong counters in RelayHostBridge - Set receive and send buffer sizes on accepted guest sockets and created host sockets - Replace fixed 8192-byte buffer with RELAY_BUFFER_SIZE in pipeLocalToRelay and pipeLanToRelay - Calculate payloadLen and use it in binary data processing for both guest and host - Add debug logging for payload over 1000 bytes with per-direction cumulative KB totals - Add connection-closed debug log when no stream found for binary data - Add final info log in RelayGuestProxy pipeLocalToRelay showing total sent/received KB per stream
- Replace singleplayer condition with dedicated server check for ItemCleanupManager and PlaytimeManager start in onServerStarted - Add dedicated server condition to stop ItemCleanupManager and PlaytimeManager in onServerStopped - Change HUDRenderer singleplayer check to include published LAN condition for HUD rendering
- Change getPlayerCount to getOnlinePlayerNames and use list size for count - Adjust player list height calculation based on visible player count instead of constant MAX_VISIBLE_PLAYERS - Remove unused getPlayerCount method - Update renderPlayerList to accept playerNames list instead of LocalPlayer - Modify renderPlayerList call to pass pre-fetched playerNames
- Update mod_version from 2.4.1 to 2.5.0 in gradle.properties - Change VERSION constant from 2.4.1 to 2.5.0 in FireflyMCMod.java - Update console output from "Loading FireflyMC 2.4.1" to "Loading FireflyMC 2.5.0" in FireflyMCMod.java - Update SERVER_NAME component from "FireflyMC 2.4.1" to "FireflyMC 2.5.0" in HUDRenderer.java - Change currentVer string from "当前版本 2.4.1" to "当前版本 2.5.0" in ModUpdateScreen.java - Update comment in RulesLoader.java from "ireflymc-2.4.1.jar" to "ireflymc-2.5.0.jar" - Update example comment in UpdateChecker.java from "ireflymc-2.4.1.jar" to "ireflymc-2.5.0.jar" - Change Implementation-Version from 2.4.1 to 2.5.0 in MANIFEST.MF
- Register singleplayer world open-to-LAN event and rename related comment - Update ClientState field comment for singleplayer relay hosting - Add pendingRoomId and pendingGuestSessionId fields in RelayGuestJoiner - Send guest_leave control message when proxy start fails in RelayGuestJoiner - Add stopActiveRelay method to handle pending or active guest relay termination - Store pending room and session IDs during proxy start in RelayGuestJoiner - Add reason parameter and leaveSent flag to RelayGuestProxy stop method - Send guest_leave message on proxy stop and when last stream closes - Add guestSessionId and roomId accessors in RelayGuestProxy - Add clearGuestProxy method in RelayLobbyWebSocketClient - Create guestLeave factory method in RelayLobbyMessage - Simplify RelayLobbyWebSocketClient class documentation
- Introduce scheduled executor and timeout constants for managing join timeout - Add AtomicBoolean state to track pending connection to relay room - Implement scheduleConnectTimeout and cancelConnectTimeout methods - Add forceStopActiveRelay to cleanly abort on timeout and release room slot - Modify stopActiveRelay to ignore disconnect events during connection phase unless proxy accepted - Add markProxyAcceptedConnection and hasAcceptedClientConnection to signal successful client handshake - Set acceptedClientConnection flag and notify joiner when socket is accepted in handleBinary
- Add six P2P-related config fields in Config.java with default values and constraints - Extend RelayControlMessage.java with P2P negotiation fields, inner P2PCandidate class, and getters - Implement P2P connection attempt logic in RelayGuestJoiner.java using Config and P2PJoinInfo
- Add P2PGuestProxy creation and start after successful P2P handshake in RelayGuestJoiner - Add "host_open_ack" and "guest_joined" to P2P control message handling in RelayLobbyWebSocketClient - Call P2PConnectionManager stopHost and prepareHost in SingleplayerRelayManager - Add P2P host state fields and prepareHost method in P2PConnectionManager - Change P2P guest connection result from failure to success with channel - Add logging for P2P UDP observation and peer candidate in handleControlMessage
- Add loggedCandidates map to prevent repeated candidate log messages - Replace null guestSessionId check with type null check in handleControlMessage - Add guestSessionId null guard before processing p2p_candidate and guest_joined - Trigger host probe when candidate matches hostSessionId in p2p_candidate handler - Avoid duplicate channel initialization in startHostConnection when channel already exists - Introduce effectiveUdpHost method with fallback to resolve relay host from config - Replace 0.0.0.0/localhost udpHost with relay host fallback in effectiveUdpHost - Add loggedObserved and loggedPunch flags to suppress duplicate logs in ReliableUdpChannel - Use effectiveUdpHost for server address in ReliableUdpChannel start method - Enhance probe start log to include rawUdpHost for debugging - Add raw udp host info to guest connection log message
- Change P2PConnectionManager from single hostBridge to hostBridges map - Add bridge cleanup in removeGuestSession for associated session ID - Add localToP2pBytes counter and periodic logging in P2PGuestProxy pipeLocalToP2P - Add lanToP2pBytes counter and periodic logging in P2PHostBridge pipeLanToP2P - Add info logs for bridge start and connection events in P2PHostBridge - Add import for ArrayList, Collections, List, and AtomicLong in ReliableUdpChannel
- Add whenComplete callback to probeAndPunch in P2PConnectionManager to stop guest session on host probe failure - Introduce nextSentLogAt and nextReceivedLogAt counters in ReliableUdpChannel for logging every 64 KB - Replace verbose per-chunk logging with total progress logs in sendData method - Replace verbose per-chunk logging with total progress logs in handleData method
- Modify P2PConnectionManager.startHostBridge to accept a sessionId parameter, defaulting to hostSessionId when null, and use it for channel lookup, bridge storage, and logging - Add SendWindow instance to ReliableUdpChannel for ACK-based retransmission - Remove temporary double send in sendData, replace with sendWindow.record - Integrate sendWindow.start when hole punching completes, and sendWindow.close on channel close - Move ACK packet sending to immediate handling instead of only when payload is written - Add SendWindow class with sliding window, retransmission task, and pending packet cleanup
- Add channel.close() call in P2PGuestProxy's close() method to release channel - Check channel.isRunning() before sending FIN in P2PGuestProxy finally block - Introduce running AtomicBoolean in P2PHostBridge to control loop termination - Add running.get() check in LAN-to-P2P read loop and break when not running - Conditionally log IOException only if running flag is still true - Check channel.isRunning() before sending FIN in P2PHostBridge finally block - Add lastReceivedAt timestamp and 30s idle timeout check in ReliableUdpChannel - Start idle check task via scheduleAtFixedRate after punch completion - Skip sendData and sendFin when channel is not running - Add isRunning() method to expose channel health status - Add running flag CAS in close() to prevent double closing with log
There was a problem hiding this comment.
🌸 Sakura AI 审查报告 - 🔬 深度审查
❌ 请求变更
📋 查看详细审查报告
Now I have a comprehensive understanding of the codebase. Let me compile the detailed review.
📊 整体评价
代码质量评分:6/10
简短评价:PR 实现了完整的单人世界公开大厅、WebSocket 中继和 P2P UDP 打洞功能,架构分层清晰,但存在多处线程安全隐患、大量代码重复(UI 绘制代码完全复制),以及 RelayLobbyMessage 类设计过度膨胀的问题。
🔴 关键问题(优先处理)
无确认会导致程序崩溃、安全漏洞或数据丢失的严重问题。代码在功能正确性上整体可用,线程安全问题属于竞态条件,目前可能不会立即触发崩溃。
🟡 重要改进
1. 线程安全:RelayLobbyWebSocketClient 的 webSocket 字段无线程安全访问控制
文件:src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:36
webSocket 字段在 executor 线程中被赋值(第 249 行 ensureConnected),但在其他线程中也可能被读取(如 sendBinary 在调用线程中直接访问)。sendText().join() 是阻塞调用,在单线程 executor 中执行可能导致死锁:executor.execute(() -> ensureConnected() -> send() -> webSocket.sendText().join())——join() 会阻塞 executor 线程本身,后续任务无法执行。
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:344
问题: send() 方法在 executor 单线程中调用 webSocket.sendText(json, true).join(),这会阻塞 executor 线程直到发送完成。由于 send() 从 publishRoom、requestLobbyList、joinRoom 等方法中通过 executor.execute() 调用,如果 sendText().join() 阻塞(网络延迟时可能发生),整个 executor 会被卡住,后续的心跳、消息处理等任务全部延迟。
建议: 将 sendText 的异步 CompletableFuture 链式处理,或改用非阻塞发送方式:webSocket.sendText(json, true).exceptionally(ex -> { LOGGER.warn("发送失败", ex); return null; });。
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:249
问题: ensureConnected() 方法中 connectFuture.join() 在 executor 线程内同步等待 WebSocket 连接完成,连接超时 10 秒 + waitUntilOpen() 5 秒,总共最多阻塞 executor 线程 15 秒。在此期间所有排队到 executor 的任务(如心跳、消息处理)都会被阻塞。
建议: 考虑使用异步回调或独立的连接线程,避免在 executor 内部长时间阻塞。
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:42
问题: textAccumulator(StringBuilder)在 onText 回调中被追加和清空,onText 回调可能由 HttpClient 的内部线程池调用,而 textAccumulator 没有任何同步保护。如果多条消息并发到达,可能导致数据交错损坏。
建议: 将 textAccumulator 的访问改为 synchronized,或改用线程局部变量。
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:43
问题: binaryAccumulator(ByteBuffer)同样在 onBinary 回调中读写,没有同步保护。onBinary 可能由不同线程并发调用,多帧分片到达时可能产生竞态条件。
建议: 添加 synchronized 或使用原子引用。
2. 架构设计:RelayLobbyMessage 类职责过载
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java:13
问题: 该类承担了太多职责——既是消息 DTO(20+ 字段),又是工厂类(16 个静态工厂方法),还包含序列化逻辑(toJson)。4 个构造函数链条式调用,参数最多的构造函数有 15 个参数。这种"上帝类"难以维护和扩展,每新增一种消息类型都要加字段和构造函数链。
建议: 考虑使用继承层次或策略模式,例如创建 HostOpenMessage、GuestJoinMessage 等子类,或至少使用 Builder 模式减少构造函数参数数量。
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java:112
问题: Minecraft 版本号 "1.21.1" 被硬编码在两个构造函数中。将来升级 Minecraft 版本时容易遗漏修改。
建议: 提取为常量或从运行时获取,如 SharedConstants.getCurrentVersion().getName()。
3. 代码重复:两个 Screen 类大量 UI 绘制代码完全重复
🟡 src/main/java/firefly520/fireflymc/client/screen/SingleplayerSharePromptScreen.java:133-217
问题: SingleplayerSharePromptScreen 和 RelayLobbyScreen 中有大量完全相同的绘制代码:drawRoundedRect、fillCircle、drawFrostedGlassBackground、drawGradientBorder、drawGradientLine、drawStarIcon、lerpColor、drawCentered——这些方法在两个文件中逐字相同。同样,两个类的颜色常量(ACCENT_PRIMARY、ACCENT_SECONDARY、TEXT_PRIMARY 等)也完全相同。
建议: 提取公共父类(如 FireflyScreenBase)或工具类 FireflyUIShared,将公共绘制方法和颜色常量统一存放。这不仅减少重复代码,还便于未来统一调整 UI 风格。
4. P2P 通道资源泄漏风险
🟡 src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java:85-87
问题: receiverThread.start() 在 probeAndPunch 中被调用时,没有检查线程是否已被启动过(isAlive() 判断在已终止但未重新创建的情况下不够充分)。如果 probeAndPunch 被意外调用两次(如 P2P 重试),第二次 start() 会抛出 IllegalThreadStateException。Thread 对象不能复用。
建议: 在构造函数中就启动 receiver thread,或添加更严格的 guard 防止重复调用。
🟡 src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java:53
问题: pendingBeforeRegister Map 使用 Collections.synchronizedList(new ArrayList<>()) 作为值,但 registerStream 中先 remove 再遍历,存在 TOCTOU 竞态。如果数据在 remove 之后、遍历之前有新的到达,可能会丢失。更重要的是,此 Map 中的 List 永远不会被清理(除了 unregisterStream),如果一个 stream 始终未被 register,数据会无限累积到 256 的限制。
建议: 为 pendingBeforeRegister 添加定期清理机制,或在 close() 中清理。
5. SendWindow 配置值未实际使用
🟡 src/main/java/firefly520/fireflymc/client/relay/p2p/SendWindow.java:22
问题: MAX_WINDOW = 128 是硬编码常量,但 Config 中定义了 SINGLEPLAYER_RELAY_P2P_WINDOW_SIZE(默认 64),这个配置项在此 PR 中完全没有被使用。record() 方法中用 MAX_WINDOW * 4 做清理阈值,但没有实际的流控限制——record() 不检查当前窗口大小是否超过限制就直接写入。
建议: 要么使用配置值替代硬编码常量,要么在 record() 中添加窗口满时的背压控制(如等待 ACK),避免无限发送导致内存溢出。
6. RelayGuestJoiner 静态可变状态缺乏同步
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayGuestJoiner.java:34-38
问题: activeProxy、pendingRoomId、pendingGuestSessionId 等静态字段在多个线程间访问(渲染线程、网络线程、executor 线程),但没有 volatile 或 synchronized 保护。例如 stopActiveRelay() 可能从渲染线程调用,而 startProxyAndConnect() 从 CompletableFuture 回调线程调用,两者同时操作 activeProxy 可能导致竞态。
建议: 至少将 activeProxy 声明为 volatile,或者将相关操作统一收束到 executor 线程中执行。
7. RelayLobbyWebSocketClient 没有断线重连机制
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:235-246
问题: onClose 和 onError 只是将 connected 设为 false 并打印日志,没有任何重连逻辑。一旦连接断开,心跳停止,房间状态无法更新,Guest 端的二进制数据流也会中断。Host 侧发布房间后如果网络抖动断开,房间在中继服务端不会被及时清理。
建议: 在 onClose 中根据场景决定是否自动重连(如 hosting 状态时自动重连),或在 ensureConnected() 中检测到已断开后重新建立连接。
💡 优化建议
💡 src/main/java/firefly520/fireflymc/client/relay/p2p/UdpPacketCodec.java:29-50
问题: probe() 和 punch() 方法的实现几乎完全相同,只有 type 字段不同("probe" vs "punch")。这是代码重复。
建议: 提取一个私有方法 buildJsonPacket(String type, ...) 并在 probe 和 punch 中分别调用。
💡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java:204-258
问题: toJson() 方法手写 JSON 序列化,逐字段 if-null 检查,代码冗长且易遗漏。类已经有 @SerializedName 注解,直接用 GSON.toJson(this) 即可。
建议: toJson() 方法可直接简化为 return GSON.toJson(this);(除非有特殊需求如某些字段不需要序列化到 JSON)。
💡 src/main/java/firefly520/fireflymc/client/relay/SingleplayerRelayClientEvents.java:73-83
问题: resolveWorldName() 方法在 SingleplayerRelayClientEvents 和 SingleplayerRelayManager 中完全重复实现。
建议: 提取为公共工具方法。
💡 src/main/java/firefly520/fireflymc/client/relay/p2p/P2PJoinInfo.java:12
问题: RELAY_HOST_FALLBACK 是在类加载时通过 resolveRelayHostFallback() 计算的静态字段。如果配置在运行时被修改(热重载),这个值不会更新。
建议: 改为每次调用 effectiveUdpHost() 时动态计算,或使用 lazy holder。
💡 src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java:219
问题: text.contains("probe_ack") 和 text.contains("punch") 用字符串包含匹配 UDP 控制包类型。如果服务端发送的正常数据包中恰好包含这些字符串(例如聊天消息),会被误判。
建议: 使用 JSON 解析来识别 probe/punch 包,或至少检查字符串以 "{" 开头并且包含 "type":"probe_ack" 的精确匹配。
💡 src/main/java/firefly520/fireflymc/client/screen/RelayLobbyScreen.java:93
问题: 房间列表固定最多显示 6 个房间(Math.min(rooms.size(), 6)),没有滚动支持。如果公开房间很多,用户无法查看全部。
建议: 至少增加简单的滚动条或翻页机制。
💡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:160-161
问题: ensureConnected() 每次都创建新的 HttpClient 实例。HttpClient 是重量级对象,应该复用。
建议: 将 HttpClient 作为类字段在构造函数中创建一次。
💡 src/main/java/firefly520/fireflymc/client/relay/RelayHostBridge.java:106
问题: pipeLanToRelay 的 while 循环没有检查 running 状态。如果 Host bridge 被 stop,pipe 线程会继续从 LAN socket 读取数据直到 socket 关闭。
建议: 在 while 循环条件中加入 running.get() 检查,与 P2PHostBridge.pipeLanToP2P 保持一致。
⚠️ 警告
⚠️ 单人世界安全风险
⚠️ src/main/java/firefly520/fireflymc/client/relay/SingleplayerRelayManager.java:58-60
问题: server.setUsesAuthentication(false) 禁用了身份验证,server.setPreventProxyConnections(false) 允许代理连接。公开到联机大厅后,任何知道房间信息的人都可以通过中继直接连接到该玩家的 LAN 服务器。这本身是功能需求,但缺少对连接来源的任何验证(如连接数限制、速率限制)。恶意用户可能通过快速连接/断开来影响性能。
建议: 考虑添加基本的速率限制或最大并发连接数检查。
⚠️ P2P Token 在日志中可能泄露
⚠️ src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java:95
问题: probeAndPunch 的 info 参数包含 p2pToken,日志中输出了 info.udpHost() 但当前未直接输出 token。不过 P2PConnectionManager 中的某些 debug 日志可能间接暴露连接信息。确保生产环境日志不会泄露安全凭证。
建议: 审查所有日志输出,确保 token 不会被意外记录。
📝 总结性建议
整体改进方向
-
线程安全是最大短板:核心的
RelayLobbyWebSocketClient存在多处竞态条件和潜在的 executor 线程阻塞问题。建议重新审视 executor 的使用模式,区分 I/O 线程和调度线程的职责。 -
UI 代码需要抽象:两个 Screen 类的绘制代码大量重复,应提取公共基类或工具类。这不仅是代码卫生问题,还影响未来 UI 一致性维护。
-
RelayLobbyMessage 需要重构:当前的单类多构造函数模式已达到维护极限。建议按消息类型拆分或引入 Builder 模式。
-
P2P 可靠传输是 MVP 级别但已很完善:重排序缓冲区、滑动窗口重传、ACK 机制的基本框架已到位,只是缺少流控和背压机制。这是一个好的起点,后续可以逐步增强。
-
断线重连机制必须补充:WebSocket 连接没有任何重连策略,这在实际网络环境中是不可接受的。
-
缺少单元测试:整个 PR 新增约 3400 行代码,但没有看到任何测试代码。特别是 P2P 协议编解码(
UdpPacketCodec)、重排序缓冲区(ReorderBuffer)等纯逻辑组件非常适合单元测试。
{"schema_version":1,"overall_score":6,"decision":"request_changes","decision_reason":"代码实现了完整的单人世界公开大厅和中继/P2P通信功能,但存在多处线程安全隐患(WebSocket客户端的executor阻塞、StringBuilder无线程安全保护)、大量UI代码重复、以及缺少断线重连机制。建议优先修复线程安全问题再合并。","issues":[{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java","line_number":344,"title":"send()阻塞executor线程","description":"send()方法在executor单线程中调用webSocket.sendText().join(),会阻塞整个executor线程直到网络发送完成,导致后续心跳、消息处理等任务被延迟","suggestion":"使用异步方式发送:webSocket.sendText(json, true).exceptionally(ex -> { LOGGER.warn(...); return null; })"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java","line_number":42,"title":"textAccumulator无线程安全保护","description":"StringBuilder在WebSocket回调中被追加和清空,但WebSocket回调可能由不同线程并发调用,存在数据交错风险","suggestion":"将textAccumulator的访问改为synchronized或使用ThreadLocal"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java","line_number":43,"title":"binaryAccumulator无线程安全保护","description":"ByteBuffer在onBinary回调中读写没有同步保护,多帧分片并发到达时可能产生竞态条件","suggestion":"添加synchronized保护或使用AtomicReference"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java","line_number":235,"title":"缺少WebSocket断线重连机制","description":"onClose和onError只设置connected为false,没有重连逻辑。host发布房间后如果网络抖动断开,房间在中继服务端不会被及时清理","suggestion":"在onClose中根据hosting状态决定是否自动重连,或在ensureConnected中检测断开后重建连接"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayGuestJoiner.java","line_number":34,"title":"静态可变状态缺乏同步","description":"activeProxy、pendingRoomId等静态字段在渲染线程、网络线程、executor线程间访问,没有volatile或synchronized保护","suggestion":"至少将activeProxy声明为volatile,或将相关操作统一收束到executor线程"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/screen/SingleplayerSharePromptScreen.java","line_number":133,"title":"两个Screen类UI绘制代码大量重复","description":"SingleplayerSharePromptScreen和RelayLobbyScreen中drawRoundedRect、fillCircle、drawFrostedGlassBackground等方法完全相同,颜色常量也重复定义","suggestion":"提取公共父类FireflyScreenBase或工具类FireflyUIShared"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java","line_number":13,"title":"RelayLobbyMessage类职责过载","description":20+字段、16个静态工厂方法、4个构造函数(最长15参数)、手写JSON序列化,单一类承担过多职责","suggestion":"考虑使用继承层次或Builder模式重构,或至少简化toJson()为GSON.toJson(this)"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java","line_number":85,"title":"Thread对象不能复用但存在重复调用风险","description":"receiverThread.start()在probeAndPunch中被调用,如果被调用两次会抛出IllegalThreadStateException","suggestion":"在构造函数中启动receiver thread,或添加更严格的guard防止重复调用"},{"severity":"major","file_path":"src/main/java/firefly520/fireflymc/client/relay/p2p/SendWindow.java","line_number":22,"title":"配置值SINGLEPLAYER_RELAY_P2P_WINDOW_SIZE未使用","description":"MAX_WINDOW=128硬编码,Config中定义了p2pWindowSize但完全未使用。record()方法没有流控限制,可能无限发送导致内存溢出","suggestion":"使用配置值替代硬编码常量,并在record()中添加窗口满时的背压控制"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/p2p/UdpPacketCodec.java","line_number":29,"title":"probe()和punch()方法代码重复","description":"两个方法实现几乎完全相同,只有type字段不同","suggestion":"提取私有方法buildJsonPacket(String type, ...)统一实现"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java","line_number":204,"title":"手写JSON序列化冗长","description":"toJson()手写逐字段if-null检查序列化,类已有@SerializedName注解","suggestion":"直接使用GSON.toJson(this)简化"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/SingleplayerRelayClientEvents.java","line_number":73,"title":"resolveWorldName()方法重复","description":"与SingleplayerRelayManager中完全相同的实现","suggestion":"提取为公共工具方法"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/p2p/ReliableUdpChannel.java","line_number":219,"title":"字符串包含匹配UDP包类型不可靠","description":"text.contains(\"probe_ack\")和text.contains(\"punch\")可能误判正常数据包","suggestion":"使用JSON解析精确匹配type字段"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/screen/RelayLobbyScreen.java","line_number":93,"title":"房间列表没有滚动支持","description":"固定最多显示6个房间,无法查看更多","suggestion":"添加滚动条或翻页机制"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java","line_number":160,"title":"HttpClient每次创建新实例","description":"ensureConnected()中每次创建新的HttpClient,是重量级对象应复用","suggestion":"将HttpClient作为类字段在构造函数中创建一次"},{"severity":"suggestion","file_path":"src/main/java/firefly520/fireflymc/client/relay/RelayLobbyMessage.java","line_number":112,"title":"Minecraft版本号硬编码","description":"\"1.21.1\"在两个构造函数中硬编码","suggestion":"提取为常量或从运行时获取"}],"summary":"PR实现了完整的单人世界公开大厅、WebSocket中继和P2P UDP打洞功能,架构分层清晰。主要问题集中在:(1)RelayLobbyWebSocketClient存在多处线程安全隐患,executor可能被阻塞;(2)两个Screen类UI绘制代码大量重复;(3)RelayLobbyMessage类职责过载需要重构;(4)缺少断线重连机制和流控机制。建议优先修复线程安全问题后合并。"}代码质量评分:6/10
AI审查决策:需要修复后重新提交
阻断原因:发现 1 个严重问题必须修复后才能合并
🔴 严重问题 (1个)
- 无确认会导致程序崩溃、安全漏洞或数据丢失的严重问题。代码在功能正确性上整体可用,线程安全问题属于竞态条件,目前可能不会立即触发崩溃。
🟡 重要问题 (12个)
-
src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:344
-
src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:249
-
src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:42
-
...还有 9 个重要问题
🔵 次要问题 (4个)
💡 优化建议 (12条)
🏷️ 标签建议
✅ 已自动应用的标签
- enhancement (95%) - PR标题明确提及添加客座加入和P2P支持,代码新增32个文件共3410行,属于新功能实现
- documentation (70%) - PR标题提到文档更新,虽然变更摘要未显示具体文档文件,但基于标题描述推荐
| throw new IllegalStateException("Relay lobby WebSocket is not connected"); | ||
| } | ||
| String json = message.toJson(); | ||
| webSocket.sendText(json, true).join(); |
There was a problem hiding this comment.
send() 方法在 executor 单线程中调用 webSocket.sendText(json, true).join(),这会阻塞 executor 线程直到发送完成。由于 send() 从 publishRoom、requestLobbyList、joinRoom 等方法中通过 executor.execute() 调用,如果 sendText().join() 阻塞(网络延迟时可能发生),整个 executor 会被卡住,后续的心跳、消息处理等任务全部延迟。
建议: 将 sendText 的异步 CompletableFuture 链式处理,或改用非阻塞发送方式:webSocket.sendText(json, true).exceptionally(ex -> { LOGGER.warn("发送失败", ex); return null; });。
| } | ||
| }); | ||
|
|
||
| webSocket = connectFuture.join(); |
There was a problem hiding this comment.
ensureConnected() 方法中 connectFuture.join() 在 executor 线程内同步等待 WebSocket 连接完成,连接超时 10 秒 + waitUntilOpen() 5 秒,总共最多阻塞 executor 线程 15 秒。在此期间所有排队到 executor 的任务(如心跳、消息处理)都会被阻塞。
建议: 考虑使用异步回调或独立的连接线程,避免在 executor 内部长时间阻塞。
| private RelayGuestProxy guestProxy; | ||
| private RelayHostBridge hostBridge; | ||
| private CompletableFuture<RelayControlMessage> pendingJoin; | ||
| private final StringBuilder textAccumulator = new StringBuilder(); |
There was a problem hiding this comment.
textAccumulator(StringBuilder)在 onText 回调中被追加和清空,onText 回调可能由 HttpClient 的内部线程池调用,而 textAccumulator 没有任何同步保护。如果多条消息并发到达,可能导致数据交错损坏。
建议: 将 textAccumulator 的访问改为 synchronized,或改用线程局部变量。
| private RelayHostBridge hostBridge; | ||
| private CompletableFuture<RelayControlMessage> pendingJoin; | ||
| private final StringBuilder textAccumulator = new StringBuilder(); | ||
| private ByteBuffer binaryAccumulator = null; |
There was a problem hiding this comment.
binaryAccumulator(ByteBuffer)同样在 onBinary 回调中读写,没有同步保护。onBinary 可能由不同线程并发调用,多帧分片到达时可能产生竞态条件。
建议: 添加 synchronized 或使用原子引用。
| * | ||
| * 阶段二先实现房间注册/关闭的 JSON 协议骨架;后续二进制流量中继会使用独立 stream 消息。 | ||
| */ | ||
| public final class RelayLobbyMessage { |
There was a problem hiding this comment.
该类承担了太多职责——既是消息 DTO(20+ 字段),又是工厂类(16 个静态工厂方法),还包含序列化逻辑(toJson)。4 个构造函数链条式调用,参数最多的构造函数有 15 个参数。这种"上帝类"难以维护和扩展,每新增一种消息类型都要加字段和构造函数链。
建议: 考虑使用继承层次或策略模式,例如创建 HostOpenMessage、GuestJoinMessage 等子类,或至少使用 Builder 模式减少构造函数参数数量。
| int udpPort, | ||
| int timeoutSeconds | ||
| ) { | ||
| private static final String RELAY_HOST_FALLBACK = resolveRelayHostFallback(); |
There was a problem hiding this comment.
RELAY_HOST_FALLBACK 是在类加载时通过 resolveRelayHostFallback() 计算的静态字段。如果配置在运行时被修改(热重载),这个值不会更新。
建议: 改为每次调用 effectiveUdpHost() 时动态计算,或使用 lazy holder。
| handleDataPacket(packetBytes, packet); | ||
| continue; | ||
| } | ||
| String text = new String(packet.getData(), packet.getOffset(), packet.getLength()); |
There was a problem hiding this comment.
text.contains("probe_ack") 和 text.contains("punch") 用字符串包含匹配 UDP 控制包类型。如果服务端发送的正常数据包中恰好包含这些字符串(例如聊天消息),会被误判。
建议: 使用 JSON 解析来识别 probe/punch 包,或至少检查字符串以 "{" 开头并且包含 "type":"probe_ack" 的精确匹配。
|
|
||
| if (rooms.isEmpty()) { | ||
| drawCentered(guiGraphics, "暂无可加入的公开单人世界", listY + 36, TEXT_SECONDARY); | ||
| } else { |
There was a problem hiding this comment.
房间列表固定最多显示 6 个房间(Math.min(rooms.size(), 6)),没有滚动支持。如果公开房间很多,用户无法查看全部。
建议: 至少增加简单的滚动条或翻页机制。
|
|
||
| URI uri = URI.create(Config.CLIENT.SINGLEPLAYER_RELAY_URL.get()); |
There was a problem hiding this comment.
ensureConnected() 每次都创建新的 HttpClient 实例。HttpClient 是重量级对象,应该复用。
建议: 将 HttpClient 作为类字段在构造函数中创建一次。
| private void pipeLanToRelay(String streamId, Socket socket) { | ||
| byte[] buffer = new byte[RELAY_BUFFER_SIZE]; | ||
| try (InputStream input = socket.getInputStream()) { | ||
| int read; |
There was a problem hiding this comment.
pipeLanToRelay 的 while 循环没有检查 running 状态。如果 Host bridge 被 stop,pipe 线程会继续从 LAN socket 读取数据直到 socket 关闭。
建议: 在 while 循环条件中加入 running.get() 检查,与 P2PHostBridge.pipeLanToP2P 保持一致。
- Add Pattern constants P2P_UDP_HOST_PATTERN and P2P_CANDIDATE_ADDRESS_PATTERN in RelayLobbyWebSocketClient - Implement sanitizeRelayJsonForLog method to replace p2pUdpHost and address values with <hidden> - Replace raw JSON with sanitized JSON in all LOGGER calls for debug, warn, and info levels in RelayLobbyWebSocketClient - Remove effectiveUdpHost and raw udpHost from log messages in P2PConnectionManager for incomplete join info - Simplify P2P Guest connection log to omit udpHost details - Remove address from p2p_udp_observed and p2p_candidate log entries, keeping only port
There was a problem hiding this comment.
🌸 Sakura AI 审查报告 - ⚡️ 快速审查
✅ 审查通过
📋 查看详细审查报告
评分:8/10
本次增量变更专注于 日志脱敏,系统性地移除了 P2P 通信中的 IP 地址泄露。整体改动方向正确、覆盖全面,仅有一处小遗漏。
✅ 已修复问题(来自上轮审查)
本次变更是对上轮审查中提出的「日志中 P2P 地址泄露」问题的针对性修复,核心目标已达成:
RelayLobbyWebSocketClient新增sanitizeRelayJsonForLog()方法,通过正则替换 JSON 中的p2pUdpHost和address字段值P2PConnectionManager所有日志不再输出 IP 地址,仅保留端口ReliableUdpChannel所有日志从完整InetSocketAddress改为仅输出端口
🟡 建议(可选)
🟡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:349
问题: send() 方法中,heartbeat 和 relay 流控制消息(stream_open/stream_close/guest_leave)的 debug 日志仍然直接输出原始 json,未经过 sanitizeRelayJsonForLog() 处理。虽然这些日志级别为 debug(生产环境通常不开启),且心跳/流控制消息大概率不包含 P2P 地址字段,但与同方法第353行的 info 级别日志处理方式不一致。如果流控制消息未来扩展包含地址信息,会造成泄露。
建议: 统一使用 sanitizeRelayJsonForLog(json) 处理所有日志分支,保持一致性:
LOGGER.debug("[FireflyMC] 已发送公开大厅心跳: {}", sanitizeRelayJsonForLog(json));
// ...
LOGGER.debug("[FireflyMC] 已发送 relay 流控制消息: {}", sanitizeRelayJsonForLog(json));💡 优化建议
💡 src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:30
问题: P2P_CANDIDATE_ADDRESS_PATTERN 匹配所有 "address" JSON 字段,范围较广。如果大厅消息中存在非敏感的 address 字段(如房间元数据),也会被误脱敏。
建议: 目前保守脱敏的策略是合理的(宁可多脱敏也不能泄露),但如果未来需要区分,可以考虑更精确的匹配模式。当前无需修改。
💡 src/main/java/firefly520/fireflymc/client/relay/p2p/P2PConnectionManager.java:99
问题: String candidateKey = raw.address() + ":" + raw.port(); 这行虽然仅用于内存比较(不输出日志),但仍将完整地址拼接到字符串中。
建议: 仅用于去重判断,无需修改,但可以关注后续 GC 压力(此处影响极小)。
总结:本次变更是一次聚焦的日志脱敏改进,P2P IP 地址从日志输出中已基本清除。唯一不一致点是 send() 方法中两个 debug 分支遗漏了脱敏处理,建议补全以保持一致性。
代码质量评分:8/10
AI审查决策:批准合并
🟡 重要问题 (2个)
-
debug日志未脱敏
-
src/main/java/firefly520/fireflymc/client/relay/RelayLobbyWebSocketClient.java:349
💡 优化建议 (3条)
🏷️ 标签建议
| private static final Logger LOGGER = LoggerFactory.getLogger(RelayLobbyWebSocketClient.class); | ||
| private static final RelayLobbyWebSocketClient INSTANCE = new RelayLobbyWebSocketClient(); | ||
| private static final Pattern P2P_UDP_HOST_PATTERN = Pattern.compile("(\\\"p2pUdpHost\\\"\\s*:\\s*\\\")([^\\\"]*)(\\\")"); | ||
| private static final Pattern P2P_CANDIDATE_ADDRESS_PATTERN = Pattern.compile("(\\\"address\\\"\\s*:\\s*\\\")([^\\\"]*)(\\\")"); |
There was a problem hiding this comment.
匹配所有JSON中的address字段,可能误脱敏非敏感地址。当前保守策略合理,仅作记录。
| if (message.guestSessionId().equals(hostSessionId)) { | ||
| startHostProbeIfReady(); | ||
| } | ||
| String candidateKey = raw.address() + ":" + raw.port(); |
There was a problem hiding this comment.
String candidateKey = raw.address() + ":" + raw.port(); 这行虽然仅用于内存比较(不输出日志),但仍将完整地址拼接到字符串中。
建议: 仅用于去重判断,无需修改,但可以关注后续 GC 压力(此处影响极小)。
总结:本次变更是一次聚焦的日志脱敏改进,P2P IP 地址从日志输出中已基本清除。唯一不一致点是 send() 方法中两个 debug 分支遗漏了脱敏处理,建议补全以保持一致性。
- Replace raw JSON with sanitized version in debug log for heartbeat messages using sanitizeRelayJsonForLog - Replace raw JSON with sanitized version in debug log for stream_open, stream_close, and guest_leave messages using sanitizeRelayJsonForLog
There was a problem hiding this comment.
🌸 Sakura AI 审查报告 - ⚡️ 快速审查
✅ 审查通过
📋 查看详细审查报告
评分:9/10
审查概要
本次变更仅涉及 2 行修改,是对上一轮审查中提出的 "heartbeat 和 stream 控制消息的 debug 日志未脱敏" 问题的定向修复。
上一轮问题修复确认
✅ 问题已修复:上一轮审查中指出的 RelayLobbyWebSocketClient.send() 方法内,heartbeat 和 stream 控制消息的 debug 日志直接输出原始 JSON(与 info 级别日志处理不一致)的问题,现在已统一使用 sanitizeRelayJsonForLog(json) 进行脱敏处理。修复方式简洁准确,与同方法内 else 分支的 info 级别日志保持了一致的脱敏策略。
详细分析
变更内容非常清晰:
- 第 349 行:heartbeat 消息日志 →
json→sanitizeRelayJsonForLog(json)✅ - 第 351 行:stream 控制消息日志 →
json→sanitizeRelayJsonForLog(json)✅
修复后的 send() 方法中,所有日志输出路径(debug 和 info)均统一通过 sanitizeRelayJsonForLog() 进行脱敏,消除了敏感信息(如 P2P 地址)通过 debug 日志泄露的风险。
无新增问题,代码质量良好。
代码质量评分:9/10
AI审查决策:批准合并
💡 优化建议 (4条)
🏷️ 标签建议
✅ 已自动应用的标签
- refactor (95%) - 本次提交将日志中的原始JSON替换为经过清理的版本,属于代码重构,不影响功能
|
📝 已创建 Release v2.5.0 描述 ✅ 变更已记录到 GitHub Releases 📦 附件正在打包中,请稍候... |
Enhance the LAN bridge mod with guest joining capabilities, P2P connection support, and improved relay management. Update documentation for Minecraft 1.21.1 / NeoForge 21.1.219, including event handling and UI components. Bump version to 2.5.0 to reflect these changes.
🌸 Sakura AI Reviewer的总结
变更概览
本次 PR 为 Minecraft LAN 桥接模组新增了访客加入与 P2P 中继支持,并更新相关文档。代码变更量庞大(+3920/-44),主要集中在新增中继与 P2P 通信模块。后续提交对日志中的 P2P 地址进行了脱敏处理,提升了安全性。
主要改动列表
RelayGuestJoiner、RelayGuestProxy,允许访客通过中继服务器加入 LAN 世界P2PConnectionManager、ReliableUdpChannel、ReorderBuffer等,实现可靠 UDP 点对点通信RelayHostBridge与P2PHostBridge负责主机端连接转发RelayLobbyWebSocketClient、RelayLobbyScreen等,支持大厅列表与消息交互SingleplayerRelayManager与事件监听,让单机世界也能被远程访客加入RelayLobbyWebSocketClient(+2/-2 行)Config.java新增配置项,HUDRenderer简化,新增分享提示界面影响范围
🔗 Sakura AI Reviewer 依赖图
graph TD FireflyMCMod["FireflyMCMod.java"] SingleplayerRelayClientEvents["SingleplayerRelayClientEvents.java"] SingleplayerRelayManager["SingleplayerRelayManager.java"] RelayLobbyScreen["RelayLobbyScreen.java"] SingleplayerSharePromptScreen["SingleplayerSharePromptScreen.java"] RelayGuestJoiner["RelayGuestJoiner.java"] RelayLobbyWebSocketClient["RelayLobbyWebSocketClient.java"] P2PConnectionManager["P2PConnectionManager.java"] RelayLobbyMessage["RelayLobbyMessage.java"] FireflyMCMod --> SingleplayerRelayClientEvents SingleplayerRelayClientEvents --> SingleplayerSharePromptScreen SingleplayerRelayClientEvents --> RelayLobbyScreen SingleplayerRelayManager --> P2PConnectionManager RelayLobbyScreen --> RelayLobbyWebSocketClient RelayLobbyScreen --> RelayGuestJoiner RelayLobbyWebSocketClient --> P2PConnectionManager RelayLobbyWebSocketClient --> RelayLobbyMessage FireflyMCMod -.-> Config["Config.java"] FireflyMCMod -.-> UpdateChecker["UpdateChecker.java"] ModEventHandler["ModEventHandler.java"] -.-> ModPayloadHandler["ModPayloadHandler.java"] RelayGuestJoiner -.-> P2PConnectionManager RelayLobbyMessage -.-> FireflyMCMod style FireflyMCMod stroke-width:2px style SingleplayerRelayClientEvents stroke-width:2px style SingleplayerRelayManager stroke-width:2px style RelayLobbyScreen stroke-width:2px style SingleplayerSharePromptScreen stroke-width:2px style RelayGuestJoiner stroke-width:2px style RelayLobbyWebSocketClient stroke-width:2px style P2PConnectionManager stroke-width:2px style RelayLobbyMessage stroke-width:2px