fix(#791): namespace on-disk preview cache by source URL hash to avoid file collisions by wylovelyi · Pull Request #796 · kekingcn/kkFileView

wylovelyi · 2026-09-22T01:39:34Z

关联 Issue

Closes #791

审查后修正（post-review fix）

代码审查中发现一处 P0 阻断性回归，已修复并推送到本分支：

问题：初版只把物理路径（originFilePath / outFilePath / getCacheName()）改成 physicalName，却保留 attribute.setName(originFileName)（basename）。而 kkFileView 里 getName() 既当展示名、又当 DownloadUtils.downLoad(fileAttribute, fileName) 的下载落地文件名、还当转换缓存 key——所有 *FilePreviewImpl（Pdf/Office/Common/SimText/Cad/Media/Tiff 共 7 处）都是 fileName = fileAttribute.getName()。于是下载把文件存成 test.docx，前端/转换却去读 fileKey_test.docx → 找不到 → 预览全面 404。
修复：将 setName(originFileName) 改为 setName(physicalName)，使 getName() 与物理路径统一带 fileKey。下载 / 转换 / 引用三处路径一致，不再失配。
同步把 urlCacheKey 的哈希长度从 8 位提升到 12 位 hex，降低大规模下的生日碰撞概率；测试补一条 physicalNameDiffersAcrossUrlsWithSameBasename 断言防护此回归。

问题根因

转换产物在 file.dir 下只按源文件 basename 命名，不包含源路径、也不含完整 URL 的哈希：

FileHandlerService.getFileAttribute() 通过 WebUtils.getFileNameFromURL(url) 取 URL 最后一段作为 originFileName，再派生 cacheFileName / outFilePath / originFilePath（FileHandlerService.java 约 L274–288）。
getCacheFileName() 对 PDF / COMPRESS / 图片 / 文本等类型直接返回 originFileName，OFFICE 返回 cacheFilePrefixName + pdf。

因此两个不同 URL 但同名文件（如 .../pathA/test.docx 与 .../pathB/test.docx）会写同一物理路径，后预览的覆盖先预览的转换结果，把 A 用户的文档内容吐给 B 用户（多后端 / 多租户 / AList 等场景下的内容污染）。

修复方案

在两处做最小改动，把物理落地名按「完整源 URL 的短哈希」命名空间隔离，同时保持前端展示名不变：

KkFileUtils 新增纯静态工具 urlCacheKey(url)：取完整 URL 的 MD5 前 8 位十六进制，结果不含 /、\、.. 等路径分隔符，可直接拼入文件名。
FileHandlerService.getFileAttribute() 中：
- 增加 String fileKey = KkFileUtils.urlCacheKey(url);
- 物理唯一名 physicalName = fileKey + "_" + originFileName;
- cacheFilePrefixName / cacheFileName（传给 getCacheFileName）/ originFilePath 全部基于 physicalName 派生；
- attribute.setName(physicalName)：展示名同步改为 physicalName（即 fileKey_原始名）。
注意：kkFileView 中 getName() 同时承担「前端展示名 / 下载落地文件名 / 转换缓存 key」三种角色。
若只把物理路径改成 physicalName 而保留 getName() 为 basename，会导致下载写 test.docx、前端/转换却去读 fileKey_test.docx → 文件找不到、预览全面 404（详见下方「审查后修正」）。因此显示名也必须带 fileKey，三处路径才能一致。代价是前端显示的文件名带哈希前缀，这是缓存隔离的固有代价，可接受。

这样同一 URL 始终映射到同一物理名（缓存可命中），不同路径同名文件各自隔离，互不覆盖。

回归测试

新增 server/src/test/java/cn/keking/utils/KkFileUtilsUrlCacheKeyTests.java：

相同 URL 生成稳定 key；
不同路径同名文件生成不同 key（[ISSUE] Preview cache is keyed by basename only, causing content pollution between different files that share a filename #791 核心复现）；
key 长度为 8、仅含 [0-9a-f]、无路径分隔符（可作文件名）。

兼容性说明

仅改变 file.dir 下的物理文件名与 PDF 缓存 map 的 key，不改变任何对外接口签名，各 *FilePreviewImpl 消费 getOutFilePath() / getCacheName() 等已生成字段，无需改动。
升级后旧版本生成的 basename 缓存文件不会被新命名命中，会在首次访问时自动重新下载 / 转换一次（行为正确，仅首次略有开销）。
无法在本地完整编译验证（仓库含 LibreOfficePortable 等大二进制，沙箱无法 clone 全量构建）；改动为单方法级、经人工审查，测试为纯静态方法、零 Spring 依赖，CI 风险低。

复现 / 验证步骤（本地）

准备 http://host/pathA/test.docx（内容 AAAA）与 http://host/pathB/test.docx（内容 BBBB）。
预览 pathA → 确认 AAAA；预览 pathB → 确认 BBBB；再预览 pathA → 修复前应显示 BBBB（bug），修复后仍为 AAAA。
检查 file.dir 下生成 xxxxxxxx_pathA/test.docx 与 xxxxxxxx_pathB/test.docx 两个独立文件。

…d collisions

…ngthen key to 12 hex

…splay name

wylovelyi · 2026-10-08T01:35:56Z

@klboke 你好，这个 PR 按源 URL hash 对磁盘预览缓存做命名空间隔离避免文件冲突（#791）。准备好 review 了，谢谢！

klboke

Changes are needed before merging this head (ae73789107f8360c5077b5ff5743c3e4f1d9bc66).

Archive-contained previews now point at nonexistent source files. In FileHandlerService#getFileAttribute, the isCompressFile branch obtains the already-extracted relative path from kkCompressfilepath and sets skipDownLoad=true. The new unconditional physicalName = fileKey + "_" + originFileName then changes that existing path. DownloadUtils.downLoad does not download or rename archive entries; it returns fileDir + fileAttribute.getName(). I reproduced this using the actual handler with an existing archive.zip_/docs/report.pdf: the extracted file exists, but getOriginFilePath() points to e1b8d850b7abea186f9f58a6_archive.zip_/docs/report.pdf, which does not exist. PDF/Office/text/conversion paths inside the archive consequently fail. Namespace the downloaded outer archive and its generated outputs while preserving the actual extracted entry path, and add a handler/preview regression using an existing extracted file.
The new regression suite fails. mvn -q -pl server -Dtest=KkFileUtilsUrlCacheKeyTests test produces Tests run: 4, Failures: 1: keyIsSafeForFileName expects 12 characters but receives 24. urlCacheKey currently formats 12 digest bytes as two hex characters each. Make the intended hash length, implementation, documentation, and assertions agree.

The current CI packaging jobs skip executing Java tests, so a green packaging build cannot clear these failures.

…ition kekingcn#796 的缓存隔离把 FileAttribute.name 改成了物理唯一名（<hash>_原名），该物理名被 pdfjs 等前端当作下载保存名，导致用户拿到的文件名带 hash 前缀（UX 回归）。在 /getCorsFile 流式响应上加 Content-Disposition 用源 URL 的原始文件名，预览渲染不受影响（pdfjs 通过 XHR 取字节，Content-Disposition 仅作用于下载动作）。 getName() 仍保持物理唯一名，kekingcn#791 的同名文件碰撞隔离不受影响。

wylovelyi · 2026-10-08T06:26:19Z

补充一个下载文件名 UX 修复（commit be0e5a1a）：

#796 的缓存隔离把 FileAttribute.name 改成了物理唯一名（<hash>_原名），该名被 pdfjs 等前端当作下载保存名，导致用户下载到的文件名带 hash 前缀（UX 回归，非功能 bug）。

修复：在 /getCorsFile 流式响应上加 Content-Disposition: attachment; filename="<源URL原始文件名>"。

预览渲染不受影响：pdfjs 通过 XHR 取字节做渲染，Content-Disposition 仅作用于“下载”动作。
getName() 仍保持物理唯一名，#791 的同名文件碰撞隔离不受影响（若回退 getName() 为原名会重新引入碰撞）。

注：本地文件（url.startsWith(baseUrl)）经静态资源供下载时仍可能显示物理名，本提交先覆盖最常见的远程文件（经 getCorsFile）场景。

klboke

Re-reviewed the updated head be0e5a1addd03238786b3bd4c96fb279db2011c9. The new commit adds download-name handling, but both blockers from the previous review remain reproducible, and the new header handling also needs correction. Details are attached to the affected lines.

Validation on this exact head:

mvn -q -pl server clean -Dtest=KkFileUtilsUrlCacheKeyTests test: 4 tests, 1 failure (expected: <12> but was: <24>).
Actual FileHandlerService#getFileAttribute with an existing extracted PDF: the original extracted file exists; the newly computed hash-prefixed source path does not.
Actual OnlinePreviewController#getCorsFile against a local HTTP fixture: all four requests return 200 and file bytes, but query-bearing URLs produce incorrect Content-Disposition filenames. The bundled PDF.js header parser rejects the report.pdf?token=demo and partB filenames.

The latest three-platform builds and 27 E2E checks are green. Packaging skips Java tests, and the archive smoke tests inspect the directory page without opening an extracted entry, so those checks do not cover the confirmed failures above.

klboke · 2026-10-08T08:18:26Z

        boolean isHtmlView = suffix.equalsIgnoreCase("xls") || suffix.equalsIgnoreCase("xlsx") || suffix.equalsIgnoreCase("csv") || suffix.equalsIgnoreCase("xlsm") || suffix.equalsIgnoreCase("xlt") || suffix.equalsIgnoreCase("xltm") || suffix.equalsIgnoreCase("et") || suffix.equalsIgnoreCase("ett") || suffix.equalsIgnoreCase("xlam");
+        // #791: 用完整源 URL 的短哈希为物理落地名加前缀，避免不同路径下同名文件互相覆盖（内容污染）
+        String fileKey = KkFileUtils.urlCacheKey(url);
+        String physicalName = fileKey + "_" + originFileName; // 物理唯一名；getName 同步设为 physicalName，使下载/转换/引用三处路径一致


[P1] Preserve the actual extracted path for archive entries.

This blocker from the previous review is unchanged. For an archive-contained file, originFileName has already been set to the existing extracted relative path from kkCompressfilepath, and skipDownLoad=true. Adding another URL hash here changes getName() and getOriginFilePath() to a path that was never extracted. On this head, an existing archive.zip_/docs/report.pdf becomes e1b8d850b7abea186f9f58a6_archive.zip_/docs/report.pdf (does not exist). DownloadUtils.downLoad returns this path without downloading or renaming archive entries, so nested previews fail.

Namespace the outer archive and its outputs while preserving the actual extracted entry path. Add a regression that opens an already-extracted file through the real handler/preview flow; the current archive directory smoke test cannot detect this.

klboke · 2026-10-08T08:18:26Z

+            MessageDigest md = MessageDigest.getInstance("MD5");
+            byte[] digest = md.digest(url.getBytes(StandardCharsets.UTF_8));
+            StringBuilder sb = new StringBuilder(24);
+            for (int i = 0; i < 12 && i < digest.length; i++) {


[P2] Make the hash length agree with the documented contract and regression test.

This still emits 24 hexadecimal characters: 12 digest bytes times two characters per byte. Both the Javadoc and KkFileUtilsUrlCacheKeyTests.keyIsSafeForFileName require 12 characters. Running the new suite on this exact head still yields 4 tests with 1 failure (expected: <12> but was: <24>). Choose the intended length and align the implementation, documentation, and assertions before merging.

klboke · 2026-10-08T08:18:26Z

+        String originalFileName = urlPath;
+        int lastSlash = originalFileName.lastIndexOf('/');
+        if (lastSlash >= 0 && lastSlash + 1 < originalFileName.length()) {
+            originalFileName = originalFileName.substring(lastSlash + 1);
+        }
+        response.setHeader("Content-Disposition", "attachment; filename=\"" + originalFileName + "\"");


[P2] Derive the download name from the URL path, not the complete URL string.

The new code includes query parameters in the filename and also treats slashes inside query values as path separators. Calling the actual controller against an HTTP fixture produces:

/report.pdf?token=demo -> filename="report.pdf?token=demo";

/report.pdf?token=partA/partB -> filename="partB";

/download?id=42&fullfilename=report.pdf -> filename="download?id=42&fullfilename=report.pdf".

The bundled PDF.js extractFilenameFromHeader rejects the first two values because they do not end in .pdf, so this does not restore the intended download name for those ordinary URL forms. Parse the URL path separately, honor the supported fullfilename override, and serialize a properly escaped/encoded Content-Disposition filename. Please cover query parameters, a slash inside a query value, and the filename override with controller-level tests.

wylovelyi added 7 commits September 22, 2026 09:36

fix(kekingcn#791): add urlCacheKey helper to KkFileUtils

f71c0ba

fix(kekingcn#791): namespace on-disk cache by source URL hash to avoi…

22cbe1c

…d collisions

test(kekingcn#791): add regression test for urlCacheKey uniqueness

da1bfc5

fix(kekingcn#791): align getName() with physical cache name (P0) + le…

94bcf9c

…ngthen key to 12 hex

fix(kekingcn#791): align getName() with physical cache name (P0) + le…

7939801

…ngthen key to 12 hex

fix(kekingcn#791): align getName() with physical cache name (P0) + le…

f8234b0

…ngthen key to 12 hex

fix(kekingcn#791): update stale comment to reflect physicalName as di…

ae73789

…splay name

klboke requested changes Oct 8, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(#791): namespace on-disk preview cache by source URL hash to avoid file collisions - #796

fix(#791): namespace on-disk preview cache by source URL hash to avoid file collisions#796
wylovelyi wants to merge 8 commits into
kekingcn:masterfrom
wylovelyi:fix/issue-791-cache-key

wylovelyi commented Sep 22, 2026 •

edited

Loading

Uh oh!

wylovelyi commented Oct 8, 2026

Uh oh!

klboke left a comment

Uh oh!

wylovelyi commented Oct 8, 2026

Uh oh!

klboke left a comment

Uh oh!

klboke Oct 8, 2026

Uh oh!

klboke Oct 8, 2026

Uh oh!

klboke Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

wylovelyi commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

关联 Issue

审查后修正（post-review fix）

问题根因

修复方案

回归测试

兼容性说明

复现 / 验证步骤（本地）

Uh oh!

wylovelyi commented Oct 8, 2026

Uh oh!

klboke left a comment

Choose a reason for hiding this comment

Uh oh!

wylovelyi commented Oct 8, 2026

Uh oh!

klboke left a comment

Choose a reason for hiding this comment

Uh oh!

klboke Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

klboke Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

klboke Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wylovelyi commented Sep 22, 2026 •

edited

Loading