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

8.1 KiB
Raw Blame History

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 等),这些不应阻止打开。

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 编码,再暴力去掉 []

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 颜色值(如 #FFFFFFrgba(...)),不含方括号,所以实际安全。但未来如果有人将包含方括号的值传入,输出会被静默破坏。

建议: 改为更直观的安全编码方式,如直接对单值做 JSONSerialization 后去掉两端引号,或添加文档注释明确标注此方法的约束前提。


P3-1 后台线程同步 hop 主线程获取布局信息

文件: EPUBUI/ReaderController/RDEPUBReaderContext.swift:160-186

现象: currentTextPageSize() 在非主线程且缓存不可用时,通过 DispatchQueue.main.sync 回到主线程获取布局信息。调用方包括 RDEPUBMetadataParseWorker后台初始化第72行RDEPUBChapterLoaderchapterLoadQueue.async第298行

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 contextweak 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 线程模型优化建议。