messere1 opened a new pull request, #7037:
URL: https://github.com/apache/rocketmq-dashboard/pull/7037
### Summary
`rmq.message.query` crashes with `IllegalArgumentException` instead of a 400
when the
`uniqueKey` path is combined with a `limit` below 1.
The tool catalog declares the parameter as `limit: { type: integer, minimum:
1 }`
(`server/src/main/resources/tool-catalog/tools/message.yaml`), but
`MessageQueryInput.resultLimit()` only enforced the upper cap
(`Math.min(limit, MAX_LIMIT)`), passing any negative value through untouched.
`MessageQueryToolHandler` funnels the msgId and key paths through
`MessageService.queryMessagesPage`, whose `pageSize` validation rejects such
a value
with a 400 — but the uniqueKey path hands the raw limit to
`MessageQueryOutput.fromUniqueKey`, which slices the provider result with
`messages.subList(0, Math.min(limit, messages.size()))`.
Reproduction on unpatched `rocketmq-studio` (unit harness, mocked provider
returning
one record):
- `limit = -1` -> `java.lang.IllegalArgumentException: fromIndex(0) >
toIndex(-1)`
at `MessageQueryOutput.fromUniqueKey(MessageQueryOutput.java:45)`
- `limit = -100` -> same, `toIndex(-100)`
- `limit = 0` -> silently returns an empty result, diverging from the
msgId/key
paths which answer 400 for the same input
So the same tool accepts, rejects or crashes depending on which identifier
the caller
supplied, and the crashing case surfaces as a raw 500-class error to the
AI/MCP caller.
### Fix
Enforce the schema floor where the contract is read: `resultLimit()` now
throws
`BusinessException(400, "limit must be at least 1")` for any explicit `limit
< 1`,
before the identifier dispatch and before any provider call. Default (20) and
upper-cap (100) semantics are unchanged, and all three dispatch paths now
return the
same 400 for the same out-of-contract input.
### How Did You Test This Change?
- New `limitBelowTheSchemaMinimumIsRejectedOnTheUniqueKeyPathTest`
(parameterized,
0 / -1 / -100) and `msgIdPathRejectsTheSameBelowMinimumLimitTest` in
`MessageQueryToolHandlerTest`.
- **Red on unpatched tip**: the uniqueKey cases fail with the production
crash
(`IllegalArgumentException: fromIndex(0) > toIndex(-1)` /
`toIndex(-100)` from
`MessageQueryOutput.fromUniqueKey`); the `limit = 0` case fails because
no
exception is thrown at all; the msgId case fails the same 400
expectation.
`MessageQueryToolHandlerTest`: 11 tests, 4 failures.
- **Green with this patch**: `MessageQueryToolHandlerTest` 11/11; the new
tests
additionally verify `verifyNoInteractions(messageService)` — the
rejection now
happens before the provider call.
- Regression: the ten message/trace/redelivery tool handler +
`MessageServiceTest` +
`QueryHistoryServiceTest` classes — 103/103 green (`mvn test -Dtest=...`,
JDK 21).
- Full `mvn test` on this branch: 3739 tests, 0 failures, 22 errors. The
unpatched base
(`rocketmq-studio` @ `c34535db`) scores 3735 / 0 failures / 22 errors in
the same
sandbox — the branch adds exactly the 4 new tests, all green, with the
same 22
pre-existing errors (MySQL-dependent Spring context tests failing with
`CommunicationsException: Connection refused`; this sandbox has no MySQL).
- `mvn checkstyle:check` clean; no API shape change — the only new output is
the 400.
Fixes: none - no issue filed for this defect
Assisted-by: ZCode
Signed-off-by: messere1 <[email protected]>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
