Skip to content

feat: add five example modules, fix the async example, and fix a runtime-only -parameters bug - #43

Merged
houko merged 5 commits into
mainfrom
feat/more-examples
Sep 9, 2026
Merged

feat: add five example modules, fix the async example, and fix a runtime-only -parameters bug#43
houko merged 5 commits into
mainfrom
feat/more-examples

Conversation

@houko

@houko houko commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Two things: five new example modules for scenarios the collection was missing, and a rewrite of the async module so it actually demonstrates what it claims to. Also fixes a runtime-only bug the new tests uncovered.

The -parameters bug

Worth reading first, because it affects existing code rather than anything added here.

Spring Framework 6.1 removed LocalVariableTableParameterNameDiscoverer, which used to recover parameter names from debug symbols. Without the -parameters compiler flag, a @PathVariable/@RequestParam that does not spell out its name now fails at request time with:

IllegalArgumentException: Name for argument of type [int] not specified, and parameter
name information not available via reflection. Ensure that the compiler uses the '-parameters' flag.

There are 40 such declarations in this repository — BaseController, AuthOperate, and several controllers under website. Every one of those endpoints returns 500 on Spring Boot 4. Nothing fails at compile time, which is exactly why #41 passed its build and its smoke test without catching this: the smoke test exercised order, whose path variables happen to be explicitly named.

spring-boot-starter-parent sets this flag by default. This project imports the spring-boot-dependencies BOM instead of inheriting from the parent, so it has to configure it itself. Added to maven-compiler-plugin, with a comment explaining why it must not be removed.

This surfaced only because the new validation module has a test that hits an unnamed path variable. It is the clearest argument I can offer for the tests in this PR.

New modules

None of them require an external service, so all five ship with tests that run in CI. The tests double as executable documentation for each technique.

Module Demonstrates
validation @Valid on request bodies, @Validated on path parameters, and a @RestControllerAdvice that keeps the unified Result shape on the error path
fileupload Single and batch upload plus download. Stores under a random name rather than the client-supplied one, and verifies the resolved path stays inside the storage root
restclient RestClient (the Spring 6.1 replacement for RestTemplate) against an external API, mapping 4xx to a domain exception. Tested with MockRestServiceServer, so no network
cache Spring's cache abstraction over Caffeine: @Cacheable, @CachePut, @CacheEvict
actuator Health endpoint, a custom HealthIndicator, and business metrics registered through Micrometer

Two details worth calling out:

  • The fileupload path-traversal guard is tested, not just asserted in a comment.
  • The cache tests assert on how many times the method body ran, rather than poking at cache internals. That is the effect a caller actually cares about.

Rewriting the async example

async/TestController extended BaseController, which forced it to implement nine CRUD methods that have nothing to do with asynchrony. All nine were return null;. Ninety lines of noise around roughly ten lines of actual subject matter.

While rewriting it I found the module did not work at all:

  • @EnableAsync was missing. Without it @Async is inert, so every task ran synchronously on the request thread. The example never demonstrated the thing it exists to demonstrate.
  • AsyncResult is deprecated in Spring 6; replaced with CompletableFuture.completedFuture.
  • The controller polled with while (true) { if (all done) break; Thread.sleep(1000); }. Besides burning a thread, the one-second granularity made the reported elapsed time wrong by up to a second. Replaced with CompletableFuture::join.
  • System.out.println to a logger, static Random to ThreadLocalRandom.

New tests assert that the three tasks genuinely run concurrently and that the caller is not blocked — which is what would have caught the missing @EnableAsync.

Also removed @RestController from the abstract BaseController. An abstract base class should not be a @RestController, and its @PathVariable/@RequestBody annotations do nothing without a @RequestMapping. Added a class comment saying not to extend it from non-CRUD controllers.

I left the five website controllers extending BaseController alone. Those genuinely are CRUD controllers, so the base class fits them; their unimplemented methods are a separate question from async, where the inheritance was pure noise.

Also fixed copy-pasted javadoc in four entry-point classes that each described a different module ("RabbitMq启动器" on the security, mongodb and async mains, "后台管理启动器" on javase and socket).

Verification

All 21 modules build; 24 tests, 23 passing and 1 skipped (the RabbitMQ test, which needs a broker):

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.async.AsyncTaskTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.validation.RegisterControllerTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.fileupload.FileControllerTest
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.restclient.GithubServiceTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.cache.BookServiceCacheTest
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 -- info.xiaomo.actuator.ActuatorEndpointTest
[INFO] BUILD SUCCESS

Notes

  • .properties files are read as ISO-8859-1, so the Chinese values in actuator's config use \uXXXX escapes, matching the convention already used elsewhere in this repository. A test asserts the value comes back correctly, since this fails silently otherwise.
  • .gitignore gains upload (the fileupload default storage directory) and .DS_Store, and its missing trailing newline is fixed.

CodeQL caught a real weakness in the new code

Worth recording, since this is the first time CodeQL has ever run on this repository.

It flagged two high-severity Uncontrolled data used in path expression alerts in StorageService, and it was right. extensionOf took everything after the last dot of the client-supplied filename, so an upload named a.b/../../evil yields the "extension" .b/../../evil — a whole path segment spliced into the storage path. The original code did guard against this by comparing the parent directory after normalize(), so it was not exploitable, but that is checking after the tainted value has already been built into a path. Fragile, and hard to argue about.

Fixed in two rounds:

  1. Keep the tainted value out of the path in the first place: strip every directory component from the original filename (handling Windows backslashes), then require the remainder to match \.[A-Za-z0-9]{1,10} or discard it entirely. Downloads must match the exact shape store produces.
  2. CodeQL does not treat a regex check as a sanitizer, so the alerts persisted. Switched the resolution itself to the canonical normalize() then startsWith(root) form, centralised in one resolveWithinRoot method. The allowlist stays as defence in depth.

Three more tests cover the specific vector: a traversal fragment in the original filename must not reach the extension, a filename not matching the stored shape is rejected, and a legitimate extension is still preserved.

CodeQL — No new alerts in code changed by this pull request.

async/TestController 继承 BaseController, 被迫实现 9 个与异步毫无关系的 CRUD 方法, 全部 return null。结果是 90 行空实现淹没了 10 行真正想演示的东西。这个模块的存在意义就是展示 @async, 现在让它只展示 @async。

- 删除 TestController(整类都是空实现), 新增 AsyncController 只保留异步演示
- 不再继承 BaseController
- AsyncResult 在 spring 6 已废弃, 换成 CompletableFuture.completedFuture
- 去掉 while(true) + Thread.sleep(1000) 的自旋等待, 改用 CompletableFuture::join。原写法不仅浪费 CPU, 1 秒的轮询粒度还让打印出来的"总耗时"最多偏差一秒
- System.out.println 换成 logger; static Random 换成 ThreadLocalRandom
- 补上 @EnableAsync —— 原先根本没加, 也就是说这个"异步示例"里的 @async 一直没有生效
- 新增测试, 断言三个任务确实并发执行且调用方不被阻塞

同时修正若干复制粘贴的 javadoc: security / mongodb / javase / socket 四个启动类的 Description 都写着别的模块的名字。

BaseController 上的 @RestController 也一并去掉 —— 抽象基类不应该是 @RestController, 其抽象方法上的 @PathVariable / @RequestBody 在没有 @RequestMapping 的情况下也不起任何作用。
每个模块都不依赖任何外部服务, 因此都带了可以直接跑的测试 —— 这些测试同时也是各自技术点的可执行文档。

- validation: @Valid 校验请求体 / @validated 校验路径参数, 配合 @RestControllerAdvice 做全局异常处理, 保证出错时也返回统一的 Result 结构
- fileupload: 单文件与多文件上传、下载。存储时用随机文件名而非客户端传来的原始文件名, 并校验落点在存储根目录之下, 防路径穿越
- restclient: 用 spring 6.1 引入的 RestClient(RestTemplate 的替代)调用外部 HTTP 服务, 针对 4xx 转成语义明确的异常。测试用 MockRestServiceServer 拦截请求, 不依赖网络
- cache: spring cache 抽象 + caffeine 本地缓存, 演示 @Cacheable / @cACHEpUT / @CacheEvict。测试通过"方法体被执行了几次"来断言缓存行为
- actuator: 健康检查、自定义 HealthIndicator、以及用 micrometer 注册的业务指标

同时修复一个只在运行时才会暴露的问题: 给 maven-compiler-plugin 加上 -parameters。

spring framework 6.1 移除了从调试符号推断参数名的 LocalVariableTableParameterNameDiscoverer, 因此不带该标志时, 未显式命名的 @PathVariable / @RequestParam 会在请求到达时抛 IllegalArgumentException, 而编译期没有任何提示。项目中共有 40 处这样的写法(BaseController、AuthOperate 以及 website 下的多个控制器), 也就是说升级到 spring boot 4 之后这些端点全部会 500。

spring-boot-starter-parent 默认会加这个标志, 但本项目是导入 spring-boot-dependencies BOM 而非继承 parent, 所以需要自己配置。这个问题是新模块的测试跑起来之后才暴露出来的。

.gitignore 补上 upload(fileupload 模块的默认存储目录)与 .DS_Store, 并修掉文件末尾缺失的换行。
Readme 模块表补上 validation / fileupload / restclient / cache / actuator 五个模块, 技术栈补上 caffeine 与 micrometer, 快速开始补上跑测试的命令, 并标出哪些模块自带可运行的测试。

changeLog.md 记录本次新增, 并单独说明 -parameters 这个只在运行时暴露的坑。
CodeQL 在 PR #43 上报了两处 high 级别的 "Uncontrolled data used in path expression", 是对的。

问题出在 extensionOf: 它取"最后一个点之后的全部内容"作为扩展名, 而原始文件名来自客户端。形如 a.b/../../evil 的名字会得到扩展名 .b/../../evil, 整段路径被拼进落盘路径。原先靠 normalize() 之后比对父目录来兜底, 虽然拦得住, 但属于先把污染数据拼进路径再回头检查, 既脆弱也难以论证。

改成让污染数据根本进不了路径表达式:
- 取扩展名时先剥掉所有目录成分(兼容 windows 反斜线), 再要求匹配白名单 \.[A-Za-z0-9]{1,10}, 不匹配就整个丢弃
- 读取时要求文件名完全匹配 store 生成的形状(UUID + 可选安全扩展名), 含分隔符或 .. 的输入直接拒绝

补三个测试: 原始文件名里的穿越片段不得混入扩展名、不符合存储命名格式的文件名被拒绝、合法扩展名仍被保留。
上一版用正则白名单把污染数据挡在路径之外, 但 CodeQL 的污点分析不把正则校验当作净化器, 告警仍在。

改用 normalize() 之后 startsWith(root) 这一标准写法, 集中到 resolveWithinRoot 一处。normalize 折叠掉 .., startsWith 保证结果落在根目录内, 无论输入是什么。正则白名单保留作为纵深防御。
@houko
houko merged commit bb5f6a2 into main Sep 9, 2026
4 checks passed
@houko
houko deleted the feat/more-examples branch September 9, 2026 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants