apache / apache/rocketmq

[Bug] Delay messages are delivered with internal properties leaked into propertiesString (ScheduleMessageService#messageTimeUp)

Open
#11,037 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
22.6k
Forks
12k
Avg merge
3d 1h
Merged PRs (30d)
27

Description

### Before Creating the Bug Report

- [X] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions).
- [X] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate.
- [X] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.

### Runtime platform environment
Linux, JDK 21, develop (ff8f6f74c)

### RocketMQ version
5.x develop

### Describe the Bug

ScheduleMessageService#messageTimeUp (broker/src/main/java/org/apache/rocketmq/broker/schedule/ScheduleMessageService.java) encodes the delivered message's `propertiesString` **before** the internal properties are cleared:

```java
msgInner.setPropertiesString(MessageDecoder.messageProperties2String(msgExt.getProperties())); // line 344 — encode
...
MessageAccessor.clearProperty(msgInner, MessageConst.PROPERTY_DELAY_TIME_LEVEL); // line 353 — clear after
MessageAccessor.clearProperty(msgInner, MessageConst.PROPERTY_TIMER_DELIVER_MS);
MessageAccessor.clearProperty(msgInner, MessageConst.PROPERTY_TIMER_DELAY_SEC);
```

The `propertiesString` is what actually gets persisted in the commitlog and decoded on the consumer side, so every delay-level message delivered by `ScheduleMessageService` still carries `DELAY_TIME_LEVEL` (and `TIMER_DELIVER_MS`/`TIMER_DELAY_SEC` when the producer used timer properties) on the wire, while the broker-side property map no longer has them. The property map and the wire data disagree; SQL92 property filtering and user code see stale internal properties.

This is the exact defect class fixed for the timer-wheel path in #10972 / commit e533b663f (`TimerMessageStore#convertMessage` now encodes **after** clearing). The same pattern in `ScheduleMessageService#messageTimeUp` predates it (old unmerged PR #4190 tried to address it in 2022) and was not covered by that fix.

### Steps to Reproduce

1. Send a message with `setDelayTimeLevel(n)`.
2. When the delay expires, `messageTimeUp` re-writes it into the real topic.
3. Consume the message: `message.getProperties()` / `getProperty("DELAY_TIME_LEVEL")` (decoded from `propertiesString`) still returns the internal delay properties.

### Expected Behavior

The delivered message's `propertiesString` must be encoded after the internal properties are cleared, so the property map and the wire data are consistent and no internal properties leak to consumers.

### Corresponding PR

- Fix PR: #11038 (linked with `Closes #11037` in the PR description; contains the regression test that fails before the fix and passes after it).

Contributor guide

Open the contributing guide

Research direction

Start in broker/src/main/java/org/apache/rocketmq/broker/schedule/ScheduleMessageService.java at messageTimeUp, then inspect the regression test included in PR #11038. Verify the delivered propertiesString stays consistent with the cleared property map and that the delay-message reproduction no longer exposes internal properties.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.