来自 #288 的架构级评审建议。不阻塞合入,仅供参考是否有更好的架构解法。
⚠️ [重要 · 兼容] DataPack 帧长上限校验移除后无全局兜底 bannet/datapack.go:41
问题根因:帧长上限校验 DataLen > MaxPackageSize 原先在 UnPack 中执行,对所有 UnPack 调用方生效。现在该校验被移到 Connection.StartReader,意味着 UnPack 不再有任何帧长校验。而 UnPack 是一个公开方法,任何其他调用方(当前无,但未来第三方或本仓库其它代码)如果直接调用 UnPack 并按返回的 DataLen 做 make([]byte, DataLen) 分配,就会引入与 PR 想消除的完全相同的无界内存分配漏洞——只不过这次是从单入口防御退化为了只对连接路径防御。边界生效是好事,但把防御从解码器里拆除意味着防御依赖每个调用方自己想起来做校验,这是把安全责任从单一可信点分散到了所有调用点。
为什么低级解法不够:通用评审者可能只是补一条注释'调用方必须自行校验帧长'。注释无法阻止未来调用方遗漏校验——防御缺失是架构问题,不是文档问题。
架构级方案:保留双保险:把帧长上限作为 Options/参数传入 NewDataPack()(或让 UnPack 接受一个可选的最大长度参数),使解码器对'超限帧'返回明确错误,但不再读全局配置——由调用方构造 DataPack 时传入上限值。这样既维持'解码器无状态、不依赖全局'的目标,又保留了解码器这一层兜底防护。连接侧 StartReader 的上限校验仍先于分配执行(更快、避免多余分配),解码器校验只是最后一道防线。
代价/收益:代价:DataPack 需要持有一个上限字段,牺牲一点'无状态'纯度(但上限是构造时固定的,非运行时可变状态)。收益:帧长上限的安全保证重新回到单一可信点,任何 UnPack 调用方都无法绕过——而非依赖每个调用方记得在校验后再分配。
bannet/datapack.go:41问题根因:帧长上限校验
DataLen > MaxPackageSize原先在UnPack中执行,对所有UnPack调用方生效。现在该校验被移到Connection.StartReader,意味着UnPack不再有任何帧长校验。而UnPack是一个公开方法,任何其他调用方(当前无,但未来第三方或本仓库其它代码)如果直接调用UnPack并按返回的DataLen做make([]byte, DataLen)分配,就会引入与 PR 想消除的完全相同的无界内存分配漏洞——只不过这次是从单入口防御退化为了只对连接路径防御。边界生效是好事,但把防御从解码器里拆除意味着防御依赖每个调用方自己想起来做校验,这是把安全责任从单一可信点分散到了所有调用点。为什么低级解法不够:通用评审者可能只是补一条注释'调用方必须自行校验帧长'。注释无法阻止未来调用方遗漏校验——防御缺失是架构问题,不是文档问题。
架构级方案:保留双保险:把帧长上限作为
Options/参数传入NewDataPack()(或让UnPack接受一个可选的最大长度参数),使解码器对'超限帧'返回明确错误,但不再读全局配置——由调用方构造 DataPack 时传入上限值。这样既维持'解码器无状态、不依赖全局'的目标,又保留了解码器这一层兜底防护。连接侧StartReader的上限校验仍先于分配执行(更快、避免多余分配),解码器校验只是最后一道防线。代价/收益:代价:DataPack 需要持有一个上限字段,牺牲一点'无状态'纯度(但上限是构造时固定的,非运行时可变状态)。收益:帧长上限的安全保证重新回到单一可信点,任何
UnPack调用方都无法绕过——而非依赖每个调用方记得在校验后再分配。