ReadViewSDK/CODE_REVIEW.md
shenlei 22e7e44220 feat: chapter runtime refactoring and related updates
- Refactor chapter runtime: replace window coordinator/snapshot with warmup orchestrator
- Update EPUB core: parser, reading session, JS bridge, navigator layout
- Update reader controller: data source, location resolution, persistence
- Update chapter runtime: data cache, loader, runtime store, disk cache, warmup orchestrator
- Remove deprecated navigation state machine and pagination state
- Update text rendering: book cache, HTML normalizer
- Update UI: text content view, dark image adjuster, text selection controller
- Update settings and reader configuration
- Add CODE_REVIEW.md and AUDIT_FINAL.md documentation
- Update pod dependencies (remove SSAlertSwift, SnapKit)
- Update podspec and pod configuration files

Co-Authored-By: Claude <noreply@anthropic.com>
2026-06-26 18:50:07 +09:00

151 lines
8.1 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# ReadViewSDK 代码审查报告
> 审查日期2026-06-26
> 审查范围Sources/RDReaderView 全部源码
> 审查方法:逐文件阅读 + 交叉验证 + 线程模型分析
---
## ✅ 确认成立的问题
### P1-1 WKURLSchemeHandler 同步文件 I/O 阻塞调用线程
**文件**: `EPUBCore/RDEPUBResourceURLSchemeHandler.swift:51-174`
**现象**: `webView(_:start:)` 回调中,所有文件读取均同步执行:
- `respondWithInMemoryData`第119行使用 `Data(contentsOf:)` 全量读入 ≤512KB 的文件
- `respondWithStreaming`第143行`while true` 循环中同步读 64KB 块并逐块回调 `urlSchemeTask.didReceive(data)`
Apple 未保证 WKURLSchemeHandler 回调线程为主线程,实际在 WKWebView 内部队列上执行。同步阻塞该线程会:
- 若在主线程:直接造成 UI 卡顿/ANR
- 若在 WKWebView 内部队列:阻塞资源加载管线,影响渲染时序
**建议**: 将文件读取移至后台队列,通过 `DispatchQueue.main.async` 或回调队列回传 `didReceive`/`didFinish`。流式场景可改为分批异步读取。
---
### P1-2 EPUB 解压缓存目录无清理策略
**文件**: `EPUBCore/RDEPUBParser+Archive.swift:79-90`
**现象**: `temporaryExtractionDirectory(for:)``(slug)-(fileSize)-(modificationTimestamp)` 生成缓存目录,存放在 `Caches/ssreaderview-epub/`。全局搜索确认不存在任何清理逻辑——无 LRU 淘汰、无总量上限、无 `removeItem` 调用指向该目录。
一本 50MB 的 EPUB 解压后约 100-200MB用户打开多本书后缓存目录无限增长。
**建议**: 添加缓存清理策略:
1. 提供 `clearCache()` 公开方法供宿主 App 在低存储时调用
2. 每次打开新书时按访问时间淘汰超出上限的旧缓存
3. 或采用 `FileManager.default.urls(for: .cachesDirectory:)` 依赖系统自动清理
---
### P1-3 搜索在主线程同步执行
**文件**: `EPUBUI/ReaderController/RDEPUBReaderSearchCoordinator.swift:15-36`
**现象**: `search(keyword:)` 直接调用 `resolvedSearchMatches(for:)`,该方法遍历整本书的所有 spine 条目执行 HTML→文本转换或章节同步加载 + NSRange 搜索,全程在主线程完成。
对于大型 EPUB数百章
- UI 完全冻结直到搜索完成
- 用户无法取消或看到进度
- 可能触发 iOS 看门狗终止0x8badf00d
`resolvedOnDemandSearchMatches(for:)`第125行更严重它同步加载每个章节`loadChapterSynchronouslyForMigration`),整本书搜索意味着逐章同步加载。
**建议**: 将搜索逻辑移至后台队列,对大型书籍实施分批搜索 + 渐进结果回调,并在 UI 层添加搜索进度指示和取消能力。
---
### P2-1 ZIP 无效条目导致整本 EPUB 打开失败
**文件**: `EPUBCore/RDEPUBParser+Archive.swift:41-44`
**现象**: 解压循环中,若 `validatedExtractionDestination` 返回 `nil`(条目路径验证失败),直接 `throw` 终止整本书的解析。实际 EPUB 中常包含无害条目macOS `__MACOSX/` 目录、`.DS_Store`、Thumbs.db 等),这些不应阻止打开。
```swift
for entry in archive {
guard let destinationURL = validatedExtractionDestination(for: entry.path, ...) else {
throw RDEPUBParserError.invalidArchiveEntryPath(entry.path) // 应改为 continue
}
```
**建议**: 将验证失败的条目改为 `continue` + 日志警告,并增加对已知无害条目路径的跳过逻辑。
---
### P2-2 javaScriptStringLiteral 实现依赖隐式假设
**文件**: `EPUBCore/RDEPUBJavaScriptBridge.swift:169-174`
**现象**: `javaScriptStringLiteral` 将值包装进单元素数组做 JSON 编码,再暴力去掉 `[``]`
```swift
private static func javaScriptStringLiteral(_ value: String?) -> String {
guard let value else { return "null" }
return jsonString(from: [value], fallback: "[null]")
.replacingOccurrences(of: "[", with: "")
.replacingOccurrences(of: "]", with: "")
}
```
这不是 XSS/注入问题(`JSONSerialization` 已正确转义引号/反斜杠等危险字符),但该方法依赖"值不含 `[``]`"的隐式假设。当前调用场景传入的是 CSS 颜色值(如 `#FFFFFF`、`rgba(...)`),不含方括号,所以实际安全。但未来如果有人将包含方括号的值传入,输出会被静默破坏。
**建议**: 改为更直观的安全编码方式,如直接对单值做 `JSONSerialization` 后去掉两端引号,或添加文档注释明确标注此方法的约束前提。
---
### P3-1 后台线程同步 hop 主线程获取布局信息
**文件**: `EPUBUI/ReaderController/RDEPUBReaderContext.swift:160-186`
**现象**: `currentTextPageSize()` 在非主线程且缓存不可用时,通过 `DispatchQueue.main.sync` 回到主线程获取布局信息。调用方包括 `RDEPUBMetadataParseWorker`后台初始化第72行`RDEPUBChapterLoader``chapterLoadQueue.async`第298行
```swift
func currentTextPageSize() -> CGSize {
if Thread.isMainThread {
// 主线程路径 — 安全
...
} else if let lastTextPaginationPageSize, ... {
// 缓存路径 — 无需 hop
return lastTextPaginationPageSize
} else {
let mainThreadSize = DispatchQueue.main.sync { [weak self] in
self?.currentTextPageSize() ?? .zero
}
```
`Thread.isMainThread` 保护使得从主线程调用不会死锁。实际风险是**时序耦合**:后台任务阻塞等待主线程布局信息,如果主线程正在忙于 UI 操作(如翻页动画),后台线程会被卡住直到主线程空闲。这是一个线程模型/可维护性问题,而非可直接触发的死锁。
**建议**: 将布局参数作为初始化参数传入后台任务,而非在后台线程通过 `DispatchQueue.main.sync` 获取。这样后台线程完全自主,不依赖主线程时序。
---
## ✅ 已撤回的原始误判
以下条目经逐条代码验证后确认不成立或定级过高:
| 原始编号 | 原始结论 | 修正结论 |
|---------|---------|---------|
| ~~P0 引用循环~~ | Runtime/Coordinator 强引用 context 形成循环 | **不成立**。全量搜索确认所有 Coordinator/Runtime 均使用 `unowned let context``weak var context` |
| ~~P0 XSS 注入~~ | `javaScriptStringLiteral``]` 替换导致注入 | **不成立**。`JSONSerialization` 编码已处理引号/反斜杠等危险字符。实际是健壮性问题,非安全问题(已调整为 P2-2 |
| ~~P1 Publication 暴露 parser~~ | 外部可直接修改 parser 内部状态 | **不成立**。`RDEPUBParser` 关键属性均为 `public internal(set)`,外部模块无法修改 |
| ~~P2 NSRange 越界~~ | UTF-16 和 Swift String 混用导致偏移 | **不成立**。搜索代码全程在 `NSString` + `NSRange` 域内操作,未混合 Swift String 索引 |
| ~~P2 后台 deinit 崩溃~~ | teardownWebView 可能在后台线程执行 | **证据不足**。`RDEPUBWebView` 是 UIView 子类,正常在主线程释放,无证据表明会后台释放 |
| ~~P0 ZIP 路径遍历绕过~~ | `%2e%2e` 可绕过路径校验 | **论证过头**。当前实现有组件级 `..` 检查 + `standardizedFileURL` 前缀校验,双层防护有效 |
---
## 📊 问题汇总
| 严重度 | 编号 | 问题 | 类型 |
|--------|------|------|------|
| 🟠 P1 | P1-1 | WKURLSchemeHandler 同步文件 I/O 阻塞调用线程 | 性能 |
| 🟠 P1 | P1-2 | EPUB 解压缓存目录无清理策略 | 存储 |
| 🟠 P1 | P1-3 | 搜索在主线程同步执行,大书可致 ANR | 性能 |
| 🟡 P2 | P2-1 | ZIP 无效条目导致整本打开失败 | 鲁棒性 |
| 🟡 P2 | P2-2 | javaScriptStringLiteral 实现依赖隐式假设 | 可维护性 |
| 🔵 P3 | P3-1 | 后台线程同步 hop 主线程获取布局信息 | 线程模型 |
**整体评价**: SDK 架构设计良好模块分层清晰引用管理unowned/weak使用正确分页取消机制有 `paginationToken` + `cancellationController` 兜底。主要值得修复的是三个性能类 P1 问题(主线程阻塞搜索、资源加载阻塞、缓存无清理),两个 P2 鲁棒性/可维护性问题,以及一个 P3 线程模型优化建议。