fix: modify resource upload and download #265
Conversation
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java (1)
195-199: Duplicate check ignores tenant/app boundary—risk of cross‑tenant false conflictsUniqueness should be within appId. Add the app_id criterion.
queryWrapper.eq("name", resource.getName()); queryWrapper.eq("category", resource.getCategory()); +queryWrapper.eq("app_id", loginUserContext.getAppId()); // 接入租户系统需添加租户id查询 Resource resourceResult = this.baseMapper.selectOne(queryWrapper);Operational: enforce a DB unique index on (app_id, name, category) to make this guarantee robust.
🧹 Nitpick comments (6)
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java (2)
182-188: Prefer safe URL construction (no manual string concat, no double-encoding pitfalls)Build query strings with UriComponentsBuilder and let it handle encoding.
-String encodedName = URLEncoder.encode(imageName, StandardCharsets.UTF_8); -String resourceUrl = tinyEngineUrl + "?name=" + encodedName + "&isResource=" + true; -String thumbnailUrl = tinyEngineUrl + "?name=" + encodedName + "&isResource=" + false; -if (resource.getCategory() == null) { - resourceUrl = tinyEngineUrl + "?name=" + encodedName + "&isResource=" + true + "&isChat=" + true; - thumbnailUrl = tinyEngineUrl + "?name=" + encodedName + "&isResource=" + false + "&isChat=" + true; - resource.setCategory("image"); -} +boolean defaultIsChat = resource.getCategory() == null; +if (defaultIsChat) { + resource.setCategory("image"); +} +String resourceUrl = org.springframework.web.util.UriComponentsBuilder + .fromHttpUrl(tinyEngineUrl) + .queryParam("name", imageName) + .queryParam("isResource", true) + .queryParam("isChat", defaultIsChat) + .encode(StandardCharsets.UTF_8) + .toUriString(); +String thumbnailUrl = org.springframework.web.util.UriComponentsBuilder + .fromHttpUrl(tinyEngineUrl) + .queryParam("name", imageName) + .queryParam("isResource", false) + .queryParam("isChat", defaultIsChat) + .encode(StandardCharsets.UTF_8) + .toUriString();Add imports outside the hunk:
import org.springframework.web.util.UriComponentsBuilder;Follow‑up: please confirm the requirement that “missing category” implies isChat=true URLs by default. If that’s not a strict rule, drop the isChat queryParam entirely when false using queryParamIfPresent.
189-193: Minor: use isNotBlank to skip whitespace-only payloadsThis avoids storing empty thumbnails for malformed base64.
-if (!StringUtils.isEmpty(resourceData)) { +if (org.apache.commons.lang3.StringUtils.isNotBlank(resourceData)) {base/src/main/java/com/tinyengine/it/controller/ResourceController.java (4)
298-306: Swagger doc doesn’t list the new isChat parameterExpose it so clients see the behavior change.
@Operation(summary = "获取资源", description = "获取资源", parameters = { @Parameter(name = "name", description = "名称"), - @Parameter(name = "isResource", description = "isResource"), + @Parameter(name = "isResource", description = "是否返回原图（true）或缩略图（false）"), + @Parameter(name = "isChat", description = "聊天场景下载：true=attachment, false/缺省=inline"), }, responses = {
309-311: Make default explicit and resilient for isChatAdd defaultValue to avoid ambiguous parsing and keep backward compatibility in docs.
-public void getResource(@RequestParam String name, @RequestParam boolean isResource, - @RequestParam(required = false) boolean isChat, HttpServletResponse response) throws Exception { +public void getResource(@RequestParam String name, @RequestParam boolean isResource, + @RequestParam(required = false, defaultValue = "false") boolean isChat, + HttpServletResponse response) throws Exception {
315-317: Handle missing/empty data to avoid 500 on decodeNull base64 leads to IllegalArgumentException on decode.
String base64Data = isResource ? resource.getResourceData() : resource.getThumbnailData(); +if (base64Data == null || base64Data.isBlank()) { + throw new ServiceException(ExceptionEnum.CM009.getResultCode(), "Resource content is empty"); +} String cleanBase64 = ImageThumbnailGenerator.extractCleanBase64(base64Data);
327-331: Set Content-Disposition once; remove stray space; add nosniffCleaner header handling and a small security hardening.
-// 只使用 filename* 格式，避免中文字符直接出现在header中 -response.setHeader("Content-Disposition", "inline; filename*=UTF-8''" + encodedFileName); -if(isChat){ - response.setHeader("Content-Disposition", "attachment ; filename*=UTF-8''" + encodedFileName); -} +// 只使用 filename* 格式，避免中文字符直接出现在header中 +String disposition = isChat + ? "attachment; filename*=UTF-8''" + encodedFileName + : "inline; filename*=UTF-8''" + encodedFileName; +response.setHeader("Content-Disposition", disposition); +response.setHeader("X-Content-Type-Options", "nosniff");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java(2 hunks)base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java(1 hunks)
🔇 Additional comments (1)
base/src/main/java/com/tinyengine/it/controller/ResourceController.java (1)
190-207: Upload path looks solid—sanitization and type validation are in placecleanPath + “..” check and ImageIO validation reduce traversal and polyglot tricks. Nice.
base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java
Show resolved
Hide resolved
2 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Refactor