unbridled-41 opened a new pull request, #4737:
URL: https://github.com/apache/rocketmq-dashboard/pull/4737
Fixes #4735.
### Problem
`rmq.message.redelivery` published the message explorer's **display
projection** instead of the stored payload: the body cut at 64 KiB, a binary
body replaced by the Base64 text of at most 48 KiB, and the user-property map
capped at 64 entries / 1024 code points per value. A redelivery therefore
stored a silently rewritten copy while reporting success.
Chain: `MessageRedeliveryToolHandler.execute`
(`ops/ai/tool/handler/message/MessageRedeliveryToolHandler.java:63,76`) →
`MetadataService.redeliverMessage(String,String,String,String,String)`
(`instance/topic/MetadataService.java:293-298`) → `findMessageForRedelivery`
(`:342-350`) → `MessageService.queryMessages` →
`RocketMQMessageProvider.toRecordVO`
(`provider/apache/RocketMQMessageProvider.java:811-832`, display caps applied
at `:813,:824` via `displayBody` `:836-861`) →
`MetadataService.redeliverMessage(..., MessageRecordVO, ...)` (`:300-315`)
publishes `original.getBody()`.
The tool's plan text promises "Publishes a new message containing the source
payload and properties" (`MessageRedeliveryToolHandler.java:43-45`) and
`findMessageForRedelivery` is documented as loading "the exact source message"
(`MetadataService.java:341`), so the display caps are leaking into a write
path. The sibling DLQ resend path already uses the raw bytes
(`provider/apache/RocketMQDLQProvider.java:583`).
### Root cause
`findMessageForRedelivery` reuses the message-explorer query, whose
`MessageRecordVO` is a *rendering* projection: `body`, `bodyEncoding`,
`bodyTruncated`, `properties` and `propertiesTruncated` exist so the UI can
show a bounded body, not so a writer can republish one. The redelivery path
read `getBody()`/`getProperties()` and ignored the truncation flags that say
the value is not the stored one.
### Fix
`MetadataService.redeliverMessage(..., MessageRecordVO, ...)` now refuses a
lossy source before building the send request: `isBodyTruncated()`,
`bodyEncoding=BASE64`, and `isPropertiesTruncated()` each raise
`BusinessException(409, …)` naming the reason. Publishing the exact bytes is
not expressible through `SendMessageDTO` (its body is a `String` that the admin
client turns into UTF-8 bytes), so the honest outcome is a refusal instead of a
corrupt copy. The display contract of `MessageRecordVO` is untouched — the caps
stay where they belong, in the explorer.
### Priority
**71** = impact 28 + reach 8 + reproducibility 20 + maintenance value 15.
- Impact 28/40: silent data corruption of a published message (truncated
body, Base64 text as payload, dropped properties) that reports success; the
loss is only discoverable downstream.
- Reach 8/20: one tool (`rmq.message.redelivery`), and only for sources
whose body exceeds 64 KiB, are not valid UTF-8, or carry more than 64 user
properties.
- Reproducibility 20/20: deterministic unit test on the service — no cluster
needed.
- Maintenance value 15/20: removes a display-versus-write conflation; the
refusal message names exactly which projection limit was hit.
`FIX_CONFIDENCE`: **88** — the tool's own plan text and the DLQ sibling fix
the intended behavior; the change is one guard plus three tests, and it cannot
regress the exact-source path (a VO with the flags unset behaves as before).
### Tests
Red (source at base commit `7ce9a682`, three new tests added to
`MetadataServiceTest`):
```
mvn -B -ntp -o test -Dtest=MetadataServiceTest
[ERROR] Tests run: 55, Failures: 3, Errors: 0, Skipped: 0
redeliverMessageShouldRefuseATruncatedSourceBodyTest
redeliverMessageShouldRefuseABase64SourceBodyTest
redeliverMessageShouldRefuseATruncatedSourcePropertySetTest
```
Each failed because the redelivery proceeded and the provider captured the
truncated/encoded payload; the tests now assert a 409 whose message names the
reason and that `sendMessage` was never called.
Green (this branch):
```
mvn -B -ntp -o test
-Dtest='MetadataServiceTest,MessageServiceTest,MessageRedeliveryToolHandlerTest'
[INFO] Tests run: 80, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
Full module suite on this branch (`fix/redelivery-exact-source` @ f1d1bd57):
```
mvn -B -ntp -o test
[ERROR] Tests run: 3054, Failures: 0, Errors: 17, Skipped: 0
```
The 17 errors are **pre-existing and environmental**: they are Spring
`ApplicationContext` failures caused by an unreachable MySQL in this sandbox
(`CannotGetJdbcConnectionException: Communications link failure`), and they are
byte-identical to the pristine `rocketmq-studio` baseline run measured in the
same session — same 9 test classes, same 17 methods (`StudioApplicationTest`,
`HealthProbeIntegrationTest`, `AuthServiceSessionOverviewIntegrationTest`,
`QueryHistoryServiceIntegrationTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, …). Test count moved 3051 → 3054, exactly
the three new tests, with no new failures.
### Risk
Low. The guard only fires on a source VO that is explicitly flagged as
lossy, which is exactly the case that previously produced a corrupt copy; a
fully projected message is unchanged. The behavior change worth stating in the
release notes: redelivering a message whose body is larger than 64 KiB (or
whose body/properties were cut for display) now answers `409` instead of
publishing a partial copy — an operator who needs that payload must use the DLQ
resend path, which already works from the raw stored bytes.
### Base branch
Targets `rocketmq-studio`, the development trunk. Note the repository's
default branch is still `master`; GitHub interprets the closing keyword only
for PRs that target the default branch, so the `Fixes` link above does not
auto-close #4735 when this merges — the issue needs to be closed by hand.
--
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]
