Repository navigation
fix(#791): namespace on-disk preview cache by source URL hash to avoid file collisions #796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f71c0ba
22cbe1c
da1bfc5
94bcf9c
7939801
f8234b0
ae73789
be0e5a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,8 @@ | |
|
|
||
| import java.io.File; | ||
| import java.net.URL; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.security.MessageDigest; | ||
| import java.util.*; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
@@ -260,4 +262,30 @@ public static boolean isNumeric(String str){ | |
| Matcher isNum = pattern.matcher(str); | ||
| return isNum.matches(); | ||
| } | ||
|
|
||
| /** | ||
| * 根据文件源 URL 生成稳定的短哈希,用于隔离不同路径下同名文件的物理缓存, | ||
| * 避免转换产物被互相覆盖(kkFileView #791)。 | ||
| * 取 MD5 前 12 位十六进制,结果不含路径分隔符,可直接拼入文件名。 | ||
| * | ||
| * @param url 文件源 URL(与 getFileAttribute 中规范化后的 url 保持一致即可) | ||
| * @return 12 位十六进制短哈希;url 为 null 时返回 "null" | ||
| */ | ||
| public static String urlCacheKey(String url) { | ||
| if (url == null) { | ||
| return "null"; | ||
| } | ||
| try { | ||
| 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++) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 |
||
| sb.append(String.format("%02x", digest[i] & 0xff)); | ||
| } | ||
| return sb.toString(); | ||
| } catch (Exception e) { | ||
| // MD5 为 JDK 必带算法,正常情况下不会进入此分支 | ||
| throw new IllegalStateException("MD5 algorithm not available", e); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -192,6 +192,17 @@ public void getCorsFile(@RequestParam String urlPath, | |
| FileAttribute fileAttribute = fileHandlerService.getFileAttribute(urlPath, req); | ||
| logger.info("读取跨域文件url:{}", urlPath); | ||
|
|
||
| // #791 修复副作用:getFileAttribute 为了隔离同名文件碰撞,把 FileAttribute.name 设成了 | ||
| // 物理唯一名(形如 <hash>_<原名>)。该物理名会被 pdfjs 等前端当作下载保存名,导致用户 | ||
| // 拿到的文件名带 hash 前缀(UX 回归)。这里用源 URL 的原始文件名作为下载保存名。 | ||
| // Content-Disposition 仅作用于“下载”动作,不影响 pdfjs 通过 XHR 取字节做预览渲染。 | ||
| 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 + "\""); | ||
|
Comment on lines
+199
to
+204
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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:
The bundled PDF.js |
||
|
|
||
| if (!isFtpUrl(url)) { | ||
| // HTTP/HTTPS 处理(修复:不关闭共享的 CloseableHttpClient) | ||
| CloseableHttpClient httpClient = HttpRequestUtils.createConfiguredHttpClient(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package cn.keking.utils; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertNotEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| /** | ||
| * kkFileView #791 回归测试:不同路径下同名文件必须映射到不同的缓存 key, | ||
| * 防止转换产物被互相覆盖(内容污染)。 | ||
| */ | ||
| public class KkFileUtilsUrlCacheKeyTests { | ||
|
|
||
| @Test | ||
| void sameUrlProducesStableKey() { | ||
| String url = "http://example.com/pathA/test.docx"; | ||
| assertEquals(KkFileUtils.urlCacheKey(url), KkFileUtils.urlCacheKey(url)); | ||
| } | ||
|
|
||
| @Test | ||
| void differentPathSameBasenameProducesDifferentKey() { | ||
| String a = "http://example.com/pathA/test.docx"; | ||
| String b = "http://example.com/pathB/test.docx"; | ||
| assertNotEquals(KkFileUtils.urlCacheKey(a), KkFileUtils.urlCacheKey(b)); | ||
| } | ||
|
|
||
| @Test | ||
| void keyIsSafeForFileName() { | ||
| String key = KkFileUtils.urlCacheKey("http://example.com/pathA/test.docx"); | ||
| assertEquals(12, key.length()); | ||
| assertFalse(key.contains("/")); | ||
| assertFalse(key.contains("\\")); | ||
| assertFalse(key.contains("..")); | ||
| assertTrue(key.matches("[0-9a-f]{12}")); | ||
| } | ||
|
|
||
| /** | ||
| * P0 防护:模拟 FileHandlerService.getFileAttribute 的命名契约 | ||
| * physicalName = urlCacheKey(url) + "_" + originFileName。 | ||
| * 不同路径下同名文件必须得到不同的物理名,否则下载/转换/引用三处路径失配(预览 404)。 | ||
| */ | ||
| @Test | ||
| void physicalNameDiffersAcrossUrlsWithSameBasename() { | ||
| String originName = "test.docx"; | ||
| String a = "http://example.com/pathA/" + originName; | ||
| String b = "http://example.com/pathB/" + originName; | ||
| String physicalA = KkFileUtils.urlCacheKey(a) + "_" + originName; | ||
| String physicalB = KkFileUtils.urlCacheKey(b) + "_" + originName; | ||
| assertNotEquals(physicalA, physicalB, "同名不同 URL 必须映射到不同物理名,否则缓存互相覆盖"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Preserve the actual extracted path for archive entries.
This blocker from the previous review is unchanged. For an archive-contained file,
originFileNamehas already been set to the existing extracted relative path fromkkCompressfilepath, andskipDownLoad=true. Adding another URL hash here changesgetName()andgetOriginFilePath()to a path that was never extracted. On this head, an existingarchive.zip_/docs/report.pdfbecomese1b8d850b7abea186f9f58a6_archive.zip_/docs/report.pdf(does not exist).DownloadUtils.downLoadreturns 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.