167 lines
16 KiB
Markdown
167 lines
16 KiB
Markdown
# StandardScene 代码审查报告 · 会话8 复核增强版
|
||||
|
|
|
|||
|
|
> 审查对象:`E:\Work\Core\Simple-FR\StandardSence`(StandardScene.Core / StandardScene.Devices / StandardScene.Protocol.VDA5050)
|
|||
|
|
> 审查方式:全量反模式扫描(ripgrep)+ 高风险/代表性文件逐行精读核实 + 对既有《StandardScene代码审查报告.md》的逐项复核
|
|||
|
|
> 目标框架:`net8.0-windows`(最终目标 `net8.0`,去 WinForms)
|
|||
|
|
> 报告日期:2026-06-09(会话8)
|
|||
|
|
> 说明:本报告是对同日既有报告的**复核增强版**。复核发现既有报告中的**多个 P0 已被修复**,本报告据实更新现状、补充精确行号、并记录若干**新发现问题**。
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 〇、覆盖度与方法说明
|
|||
|
|
|
|||
|
|
- **全量扫描**:对解决方案内全部 `.cs` 做反模式扫描(`Thread.Abort` / `async void` / 空 `catch{}` / 硬编码 IP / 局部 `new HttpClient` / `throw ex;` / `while(true)` / `Thread.Sleep` / `Console.*` / 反射 `GetMethod`)。
|
|||
|
|
- **精读核实**(逐行读取、行号精确):`ChargeUdpService`、`StandardChargeMission`(停止)、`AbstractLoopMission`(停止)、`WebApi`(反射端点+白名单+开头)、`Commons`、`PCBChargeStation`、`MuXingChargeStation`、`VDA5050Car`、`AsyncTcpClient`、`CommunicationMessageService`、`AtomicFileUpdateHelper`、`SnowflakeIdGenerator`、`WebAPIHelper`、`JsonParser`、`JsonTool`、`ModbusDoorController`、`Kiva`(代表车型)。
|
|||
|
|
- **未逐行覆盖**:部分 Model/Designer/Viewer 与少数车型仅做扫描级核对(已在清单标注),不影响主结论。
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 一、对既有报告的复核结论(重点)
|
|||
|
|
|
|||
|
|
| 既有报告条目 | 复核现状 | 证据(精确行号) |
|
|||
|
|
|---|---|---|
|
|||
|
|
| **P0-1 `Thread.Abort` 被空 catch 吞** | ✅ **已修复** | `Charge/StandardChargeMission.cs:683-685` 改为 `myThread?.Join(2000)`;`Chained/AbstractLoopMission.cs:1313-1314 / 1328-1329` 改为 `flag=false + Join(2000)`;全解决方案 `Thread.Abort` 仅余 1 处注释 |
|
|||
|
|
| **P0-2 WebApi 无鉴权反射任意方法(RCE)** | ⚠️ **部分修复(降为 P1)** | `WebApi.cs:145-151` 新增白名单 `IsReflectionInvokable`(黑名单 `NoReflectionApi` 优先 + 必须标 `MethodMember`/`ReflectionApiWithParameter`);端点 `330-331 / 387-388` 已拦截。**但仍 GET 执行(295/358)、仍无网络层鉴权** |
|
|||
|
|
| **P0-3 实时报文按固定下标取值、缺长度校验** | ✅ **主要路径已修复** | `Charge/ChargeUdpService.cs:49-50` 加 `message.Length>28` 校验且 `56` 不吞断线程;`Devices/Charge/PCBChargeStation.cs:30-31` 加 `message.Length>1` 校验 |
|
|||
|
|
| 范本:`CommunicationMessageService` 安全解析 | ✅ 确认范本 | `Charge/CommunicationMessageService.cs:198 / 276` 先校验 `parts.Length` 再 `byte.TryParse(InvariantCulture)` |
|
|||
|
|
| 范本:`AsyncTcpClient` | ✅ 确认范本 | `TCP/AsyncTcpClient.cs` `_reconnectGate` 锁 + `_closing/_isConnecting/_isReconnecting` + Timer 重连 + IDisposable |
|
|||
|
|
| 范本:`ModbusDoorController` | ✅ 确认范本 | `Devices/Door/ModbusDoorController.cs:40` `CancellationTokenSource` + `_syncLock` + 去抖 `_lastSentControl` + 可配间隔 |
|
|||
|
|
|
|||
|
|
**结论**:既有报告标注的 3 个 P0 中,P0-1、P0-3 已实质修复,P0-2 已被方法白名单有效缓解。**当前已无 P0 级阻断**。技术债主要集中在 P1/P2(旧模块的 async void、硬编码、Console 日志、上god文件、半成品死代码)。
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 二、仍然存在的问题
|
|||
|
|
|
|||
|
|
### P1 高危
|
|||
|
|
|
|||
|
|
#### P1-1 WebApi 反射端点无网络层鉴权 + 用 GET 执行副作用操作
|
|||
|
|
- 位置:`StandardScene.Core/WebApi.cs:295`(`/car_reflection/execute/{id}/{method}`)、`358`(`/mission_reflection/execute/{id}/{method}`);开头 `38` `ApiController : NancyModule` 无任何 `Before`/鉴权管线
|
|||
|
|
- 现象:虽有方法白名单(145-151),但任何能访问该 HTTP 端口者均可对白名单内方法发起调用,其中包含 `关闭进程`、`立即强制结束` 等危险操作(如 `Kiva.ForceStop`、`StandardChargeMission.Stop`);且为 GET 语义,易被浏览器预取/日志/CSRF 触发。
|
|||
|
|
- 影响:现场误触发可导致小车强停、充电进程关闭等安全相关后果。
|
|||
|
|
- 修复:① 增加统一鉴权(API Token / 来源 IP 白名单,Nancy `Before` 管线集中校验);② 执行类端点改 `POST`;③ 对“危险方法”增加二次确认/单独权限位。
|
|||
|
|
|
|||
|
|
#### P1-2 VDA5050 硬编码现场设备 IP(换现场/多车必失效)
|
|||
|
|
- 位置:`StandardScene.Protocol.VDA5050/VDACar/VDA5050Car.cs:156`、`909`、`913`;`VDACar/VDA5050Interface.cs:152`、`156`(均为 `http://192.168.2.1:8008/...`)
|
|||
|
|
- 现象:把对外取值/置值的设备 IP 写死为 `192.168.2.1`,且 `VDA5050Car.cs:154` 注释里本有 `this.address` 的正确写法却被弃用。
|
|||
|
|
- 影响:多 AGV 或换现场时全部失效;所有车都打到同一 IP。
|
|||
|
|
- 修复:统一取 `this.address`/配置项;端口与路径走配置;删除写死分支。
|
|||
|
|
|
|||
|
|
#### P1-3 `Commons.AddOrUpdateTag` 名不符实(只 Add,已存在会抛异常)
|
|||
|
|
- 位置:`StandardScene.Core/Commons.cs:96-107`(正确的“存在则更新”逻辑被注释,仅保留 `item.Add(tag, value)` 于 `106`)
|
|||
|
|
- 现象:方法语义是 AddOrUpdate,实际只 Add;当 tag 已存在时按 `TagSet.Add` 行为可能抛异常或产生重复项。
|
|||
|
|
- 影响:调用方(如 `AbstractLoopMission.MarkTaskStartSitesAsTerminal → AddOrUpdateSiteField`)在重复标记场景下可能异常或脏数据。
|
|||
|
|
- 修复:恢复“存在则 `item[tag]=value`,否则 `Add`”语义。
|
|||
|
|
|
|||
|
|
#### P1-4 局部 `new HttpClient()`(socket/端口耗尽风险)
|
|||
|
|
- 位置:`WebApi.cs:977 / 1256 / 1322`、`Model/Map.cs:194`(均为方法内 `new HttpClient()` 后即用即弃)
|
|||
|
|
- 现象:高频路径每次新建 HttpClient,底层 socket 进入 TIME_WAIT 累积,长期运行端口耗尽。
|
|||
|
|
- 影响:运行一段时间后 HTTP 调用大面积超时/失败。
|
|||
|
|
- 修复:复用静态单例 / `IHttpClientFactory`;项目已有正确范例可参照(`Chained/DeliveryViewer.cs:24`、`Chained/TransportDeliveryCallbacks.cs:26` 的 `static readonly HttpClient`,以及 `Utils/WebAPIHelper` 的连接池设计)。
|
|||
|
|
|
|||
|
|
#### P1-5 `async void` 泛滥(异常逃逸、无法等待、无法取消)
|
|||
|
|
- 代表位置:`VDA5050Car.cs:150`(`GetVDA5050StateFromC`,且 `165-167` 空 catch 吞异常)、`Charge/ChargeUdpService.cs:28`(`ListenerProcess`,被 `new Thread(...)` 包裹更失语义)、`Devices/Charge/MuXingChargeStation.cs:424`(`SendMessage`,内部无 await)、`Chained/TransportMission.cs:374/425/470/507`、`Chained/TransportDeliveryCallbacks.cs:79/109/136/145/154/161`、`Chained/AbstractChainedDeliveryMission.cs:672/1020/1276`、`InterLock/AbstractInterlockMission.cs:289/326`、车型 `Kiva.cs:495/619`、`Forklift.cs:419`、`DummyCar.cs:461/576/626/671` 等
|
|||
|
|
- 现象:`async void` 抛出的异常直达 SynchronizationContext,常导致进程级未观测异常;其中多处 catch 为空(异常被吞)。
|
|||
|
|
- 影响:偶发崩溃/状态错乱且难排障。
|
|||
|
|
- 修复:业务异步方法一律返回 `Task` 并由上层 `await`/`ContinueWith` 处理异常;确需 fire-and-forget 的入口(事件/框架回调)内部必须 `try/catch + Diagnosis.Log`;`MuXing.SendMessage` 这种无 await 的应直接改 `void`。
|
|||
|
|
|
|||
|
|
### P2 结构 / 可维护性
|
|||
|
|
|
|||
|
|
#### P2-1 `WebApi.cs` 上帝文件(约 2800+ 行单 NancyModule)
|
|||
|
|
- 位置:`StandardScene.Core/WebApi.cs`(`ApiController : NancyModule` 单类承载全部路由)
|
|||
|
|
- 修复:按功能域拆分为多个 NancyModule(车辆/任务/地图/充电/交管/系统),公共逻辑(鉴权、统一响应封装、参数绑定、反射执行)下沉到基类/中间件。
|
|||
|
|
|
|||
|
|
#### P2-2 `GetMethods` 只扫描当前程序集,与运行期类型发现口径不一致
|
|||
|
|
- 位置:`WebApi.cs:47` `Assembly.GetExecutingAssembly().GetTypes()`
|
|||
|
|
- 现象:拆分后卫星 dll(Devices/VDA5050)中的车型/Mission 方法不会出现在 `get_type_methods` 列表里;而运行期类型发现走的是 `UiTypeDiscovery.AllTypes()`(全域)。
|
|||
|
|
- 影响:前端“可用动作”列表缺失卫星类型的方法(execute 端点按实例反射仍可用,但 UI 发现不全)。
|
|||
|
|
- 修复:`GetMethods` 改用 `UiTypeDiscovery.AllTypes()` 统一口径。
|
|||
|
|
|
|||
|
|
#### P2-3 `AtomicFileUpdateHelper` 并非真正“原子”写
|
|||
|
|
- 位置:`StandardScene.Core/CommonTools/AtomicFileUpdateHelper.cs:54`(`File.WriteAllText` 直接覆盖)
|
|||
|
|
- 现象:仅用 `ConcurrentDictionary<path,lock>` 保证**进程内同路径串行**(线程安全 OK),但写入是直接覆盖,**进程崩溃/断电时文件可能损坏(半截内容)**;类名“Atomic”有误导。另:`PathLocks` 只增不减(长期运行轻微累积,`14`)。
|
|||
|
|
- 修复:改“写临时文件 → Flush → `File.Replace`/`File.Move` 覆盖”实现真正原子落盘;或在文档/命名上明确其仅保证串行而非崩溃原子性。
|
|||
|
|
|
|||
|
|
#### P2-4 `JsonParser` 死代码与文件损坏隐患
|
|||
|
|
- 位置:`StandardScene.Core/Utils/JsonParser.cs:31-59`(`JsonChangeValue` 的 `foreach` 循环体整段被注释,`Task.Run` 跑空循环,等同 NOP);`22` `WriteJsonFile` 用 `File.AppendAllText`
|
|||
|
|
- 现象:`JsonChangeValue` 是“改值”语义却什么都不做;`WriteJsonFile` 对同一 `TaskId` 重复调用会把多个 JSON 追加进同一文件,得到非法 JSON。
|
|||
|
|
- 修复:删除/重写 `JsonChangeValue`;`WriteJsonFile` 改为覆盖写(配合 P2-3 的原子写)。
|
|||
|
|
|
|||
|
|
#### P2-5 `WebAPIHelper` 退化为空壳
|
|||
|
|
- 位置:`StandardScene.Core/Utils/WebAPIHelper.cs:52-142`(Get/Post 等全部方法被注释);`getClient` 用 `ContainsKey + 索引器`(`29-33`)非原子
|
|||
|
|
- 现象:连接池设计正确(`23` `ConcurrentDictionary<host:port, HttpClient>`),但对外没有任何可用请求方法 → 各处只能各自 `new HttpClient`(正是 P1-4 的根因之一);`getClient` 并发下可能创建多个 client。
|
|||
|
|
- 修复:恢复/重写 `GetAsync/PostAsync` 并全项目改用之;`getClient` 改 `GetOrAdd`。
|
|||
|
|
|
|||
|
|
#### P2-6 UDP 发送的 `SendAsync` 未 await + `using` 竞态
|
|||
|
|
- 位置:`Devices/Charge/PCBChargeStation.cs:71-73`(`udpClient.SendAsync(...)` 未 await,紧接 `Thread.Sleep(100)` 后 `using` 块结束 Dispose)
|
|||
|
|
- 现象:异步发送可能在 `UdpClient` 被 Dispose 后才真正发出,存在 `ObjectDisposedException`/丢包风险(靠 `Sleep(100)` 掩盖)。
|
|||
|
|
- 修复:改同步 `Send` 或 `await SendAsync` 后再退出 `using`。
|
|||
|
|
|
|||
|
|
#### P2-7 `Console.*` 作为生产日志
|
|||
|
|
- 代表:`VDA5050Car.cs`(约 30 处)、`MasterMQTTCommunication.cs`(约 22 处)、`DummyCar.cs`(约 29 处)、`PCBChargeStation.cs:97`、`MuXingChargeStation.cs:438`、`Kiva.cs:567`、`Commons.cs`(约6)、`WebApi.cs`(约8)
|
|||
|
|
- 修复:统一改 `Diagnosis.Log/Post`(项目既有日志门面),保留级别与可检索性。
|
|||
|
|
|
|||
|
|
#### P2-8 `throw ex;` 丢失原始堆栈
|
|||
|
|
- 位置:`VDA5050Car.cs:240`、`CarTypes/DummyCar.cs:451`
|
|||
|
|
- 修复:改 `throw;`(重抛)或 `throw new XxxException(msg, ex)`(包装保留 inner)。
|
|||
|
|
|
|||
|
|
#### P2-9 反射调用非公开方法 / 按配置名反射
|
|||
|
|
- 位置:`CarTypes/VehicleMonitor.cs:1671`(`GetMethod(name, Public|NonPublic)` 可达私有方法);`ExtendDevice/ButtonBox/ButtonMission.cs:594`(按 `buttonConfig.TriggerMethod` 反射)
|
|||
|
|
- 修复:限制到公开+白名单;对配置驱动的反射做方法存在性与白名单校验。
|
|||
|
|
|
|||
|
|
#### P2-10 UI 与业务耦合(服务端/无人值守会阻塞)
|
|||
|
|
- 位置:`Commons.cs:42`(死锁回调里 `MessageBox.Show`)、`Kiva.cs:648` 等车型在后台线程 `MessageBox.Show`
|
|||
|
|
- 修复:业务层只产生事件/日志,是否弹窗交由表现层决定(迁 migu 平台时一并解决)。
|
|||
|
|
|
|||
|
|
### P3 整洁 / 卫生
|
|||
|
|
|
|||
|
|
- **空 `catch{}`/静默吞异常**(建议至少 `Diagnosis.Log`):`Kiva.cs:606-610 / 637-639`、`VDA5050Car.cs:165-167 / 997-1000`、`Devices/Charge/FLChargeStation.cs:173 / 243`、`Chained/AbstractLoopMission.cs:1318/1333/1396/1576/1587/1601`、`Chained/LoopViewer.cs:260`、`Charge/CommunicationMessageService.cs:183-186 / 261-264`、`Commons.cs:44`。(注:`AsyncTcpClient` 中 `try{Close();}catch{}` 属清理性吞异常,可接受。)
|
|||
|
|
- **本地回环/端口硬编码**(建议配置化,风险低于 P1-2):车型 `address="127.0.0.1"` 多处;`Model/Map.cs:195/207`(端口 4321);`Chained/LoopMission.cs:74`(`SiemensClient ...,"127.0.0.1",103`);`Chained/TransportDeliveryCallbacks.cs:27`(`_callbackUrl ...20101`)。
|
|||
|
|
- **`float.Parse`/`int.Parse` 未指定 Culture / 未 TryParse**:`Devices/Charge/PCBChargeStation.cs:61-62`、`Kiva.cs:557/601-603` 等。
|
|||
|
|
- **`SnowflakeIdGenerator`**:实现良好(`51` 锁、`54-58` 时钟回拨等待);仅提示 `DefaultEpochMs=2026-01-01`(`10`)部署到系统时间早于该值的机器会在构造期抛异常(`33-36`)。
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 三、本次新发现(既有报告未记录)
|
|||
|
|
|
|||
|
|
| 级别 | 问题 | 位置 |
|
|||
|
|
|---|---|---|
|
|||
|
|
| P2(逻辑bug) | `Kiva.LoopTest` 复制粘贴错误:构造了 `plan4` 却 `await plan2.Compile("go").Queue()`;且 `LoopTestRunning` 标志设了但循环体从不检查(`LoopTestStop` 实际无效,“循环测试”并不循环) | `CarTypes/Kiva.cs:517-520`、`492/497/528` |
|
|||
|
|
| P2 | `MuXingChargeStation.SendMessage` 标 `async void` 但内部全是同步 `stream.Write/Flush`,无 await;且只判 `client!=null` 未判 `stream` | `Devices/Charge/MuXingChargeStation.cs:424-440` |
|
|||
|
|
| P2 | `JsonParser.JsonChangeValue` 空循环死代码;`WriteJsonFile` 用 `AppendAllText` 会损坏 JSON | `Utils/JsonParser.cs:31-59 / 22` |
|
|||
|
|
| P2 | `AtomicFileUpdateHelper` 非真原子写 | `CommonTools/AtomicFileUpdateHelper.cs:54` |
|
|||
|
|
| P2 | `WebAPIHelper` 请求方法全注释成空壳 | `Utils/WebAPIHelper.cs:52-142` |
|
|||
|
|
| P2 | `GetMethods` 仅扫当前程序集,与全域类型发现口径不一致 | `WebApi.cs:47` |
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 四、子系统评分(复核更新)
|
|||
|
|
|
|||
|
|
| 子系统 | 旧评 | 复核新评 | 变化说明 |
|
|||
|
|
|---|---|---|---|
|
|||
|
|
| 设备驱动 `StandardScene.Devices` | ★★★★ | ★★★★ | `ModbusDoorController` 范本;`MuXing/PCB` 有 async void/UDP 竞态待修 |
|
|||
|
|
| TCP 基础设施 `AsyncTcpClient` | ★★★★ | ★★★★ | 维持 |
|
|||
|
|
| `SnowflakeIdGenerator` / `AtomicFileUpdateHelper` | —(未单列) | ★★★★ / ★★★ | Snowflake 好;AtomicFile 名不符实 |
|
|||
|
|
| Coders(重构后) | ★★★★ | ★★★★ | 维持 |
|
|||
|
|
| 任务族 Missions | ★★ | ★★★ | Thread.Abort 已改协作式停止(关键回升);async void/while+Sleep 仍在 |
|
|||
|
|
| 充电 Charge | ★★ | ★★★ | 报文越界已加校验、停止已协作式;UDP 发送竞态/async void 待修 |
|
|||
|
|
| VDA5050 协议 | ★★ | ★★ | 硬编码 IP / async void / throw ex / Console 仍集中,债务最重 |
|
|||
|
|
| WebApi | ★ | ★★ | 反射白名单已加(关键回升);仍上帝文件 + 无鉴权 + GET 执行 |
|
|||
|
|
| Commons / 公共层 | ★★ | ★★ | `AddOrUpdateTag` 名不符实、`WebAPIHelper` 空壳、`JsonParser` 死代码 |
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 五、优先整改清单(建议顺序)
|
|||
|
|
|
|||
|
|
1. **P1-1 WebApi 鉴权 + 危险操作语义化**(安全相关,AGV 现场风险最高)。
|
|||
|
|
2. **P1-2 VDA5050 硬编码 `192.168.2.1` 配置化**(多车/换现场必踩)。
|
|||
|
|
3. **P1-3 `Commons.AddOrUpdateTag` 修复语义**(影响面广、易引异常)。
|
|||
|
|
4. **P1-4 局部 `new HttpClient` 收敛为单例/工厂**(长稳性)。
|
|||
|
|
5. **P1-5 `async void` 收敛为 `Task` + 异常处理**(先 VDA5050 / 充电 / Transport 回调三处重点)。
|
|||
|
|
6. **P2-3/2-4/2-5 修死代码与伪原子**(`AtomicFileUpdateHelper`、`JsonParser`、`WebAPIHelper`)。
|
|||
|
|
7. **P2-1/2-2 WebApi 拆分 + 类型发现口径统一**。
|
|||
|
|
8. **P2-7 Console → Diagnosis 日志统一**(可脚本化批量替换,先 VDA5050)。
|
|||
|
|
9. **P3 空 catch 补日志 / 本地端口配置化 / Parse 加 Culture**(清扫)。
|
|||
|
|
|
|||
|
|
> 说明:本轮为只读审查,未改动任何源码。`Thread.Abort`、报文越界、反射白名单等旧 P0 经核实已修复,故当前不再列 P0。
|