Metastarx opened a new pull request, #11274:
URL: https://github.com/apache/rocketmq/pull/11274
### Which Issue(s) This PR Fixes
(本地扫描，无链接)
- Fixes #issue_id
### Brief Description
缺陷说明：GroupConsumeInfo.compareTo (ConsumerProgressSubCommand.java:347-353)
returns (int) (o.diffTotal - diffTotal) at line 352 while diffTotal is a long
(field declared at line 283 and assigned from consumeStats.computeTotalDiff()
at line 243). The same truncating pattern appears twice more:
TagCountBean.compareTo (PrintMessageByQueueCommand.java:253) returns (int)
(o.getCount().get() - this.count.get()) on long counters, and
QueryMsgByUniqueKeySubCommand.queryById (QueryMsgByUniqueKeySubCommand.java:76)
sorts with list.sort((o1, o2) -> (int) (o1.getStoreTimestamp() -
o2.getStoreTimestamp())) on long timestamps. When the real difference falls in
(Integer.MAX_VALUE, 2^32) the int cast wraps: a positive difference becomes
negative and vice versa, so the comparison reports the wrong ordering in one
direction. That both reverses the printed order and breaks the comparator
contract (sign(compare(a,b)) != -sign(compare(b,a))), which the JDK TimSort
used by Collections.sort/List
.sort can detect and fail with 'Comparison method violates its general
contract!'. These are reachable in practice: a group's total consume lag can
exceed 2^31 messages, a user-specified -s/-e timestamp window in
queryMsgByUniqueKey can span more than about 24.8 days, and a tag count printed
by printMsgByQueue can exceed 2^31.
复现步骤：Unit level. For the lag case, build two GroupConsumeInfo through
setCount(1) with diffTotal=0 and diffTotal=(long) Integer.MAX_VALUE + 1: both
left.compareTo(right) and right.compareTo(left) return a negative int, so the
comparator is not antisymmetric; sorting a list containing those entries (plus
0 and Long.MAX_VALUE) prints groups in the wrong order. For the timestamp case,
sort two MessageExt whose getStoreTimestamp() values differ by more than
Integer.MAX_VALUE milliseconds and check that the older one is still first; for
the tag case, compare TagCountBean with counts 0 and (long) Integer.MAX_VALUE +
1.
修复方向：Replace each subtraction with an overflow-safe comparison:
Long.compare(o.diffTotal, diffTotal) at ConsumerProgressSubCommand.java:352,
Long.compare(o.getCount().get(), this.count.get()) at
PrintMessageByQueueCommand.java:253, and a shared static Comparator<MessageExt>
STORE_TIMESTAMP_COMPARATOR =
Comparator.comparingLong(MessageExt::getStoreTimestamp) used at
QueryMsgByUniqueKeySubCommand.java:76, with a short comment explaining why a
plain subtraction is unsafe. Regression tests, following the existing JUnit4
style in tools/src/test (org.junit.Assert, plain model objects, no mocking
required for the comparators): extend ConsumerProgressSubCommandTest with
compareTo antisymmetry over values straddling Integer.MAX_VALUE and a full
Collections.sort ordering assertion; add a new PrintMessageByQueueCommandTest
that orders TagCountBean values beyond the int range; extend
QueryMsgByUniqueKeySubCommandTest to sort messages with store timestamps more
than Integer.MAX_VALUE
ms apart and assert oldest-first ordering. About 300 changed lines across
three main files and three test files.
### How Did You Test This Change?
PASSED: wsl -e bash scripts/gate-rocketmq.sh
<details><summary>补充说明</summary>
## 背景
缺陷说明：GroupConsumeInfo.compareTo (ConsumerProgressSubCommand.java:347-353)
returns (int) (o.diffTotal - diffTotal) at line 352 while diffTotal is a long
(field declared at line 283 and assigned from consumeStats.computeTotalDiff()
at line 243). The same truncating pattern appears twice more:
TagCountBean.compareTo (PrintMessageByQueueCommand.java:253) returns (int)
(o.getCount().get() - this.count.get()) on long counters, and
QueryMsgByUniqueKeySubCommand.queryById (QueryMsgByUniqueKeySubCommand.java:76)
sorts with list.sort((o1, o2) -> (int) (o1.getStoreTimestamp() -
o2.getStoreTimestamp())) on long timestamps. When the real difference falls in
(Integer.MAX_VALUE, 2^32) the int cast wraps: a positive difference becomes
negative and vice versa, so the comparison reports the wrong ordering in one
direction. That both reverses the printed order and breaks the comparator
contract (sign(compare(a,b)) != -sign(compare(b,a))), which the JDK TimSort
used by Collections.sort/List
.sort can detect and fail with 'Comparison method violates its general
contract!'. These are reachable in practice: a group's total consume lag can
exceed 2^31 messages, a user-specified -s/-e timestamp window in
queryMsgByUniqueKey can span more than about 24.8 days, and a tag count printed
by printMsgByQueue can exceed 2^31.
复现步骤：Unit level. For the lag case, build two GroupConsumeInfo through
setCount(1) with diffTotal=0 and diffTotal=(long) Integer.MAX_VALUE + 1: both
left.compareTo(right) and right.compareTo(left) return a negative int, so the
comparator is not antisymmetric; sorting a list containing those entries (plus
0 and Long.MAX_VALUE) prints groups in the wrong order. For the timestamp case,
sort two MessageExt whose getStoreTimestamp() values differ by more than
Integer.MAX_VALUE milliseconds and check that the older one is still first; for
the tag case, compare TagCountBean with counts 0 and (long) Integer.MAX_VALUE +
1.
修复方向：Replace each subtraction with an overflow-safe comparison:
Long.compare(o.diffTotal, diffTotal) at ConsumerProgressSubCommand.java:352,
Long.compare(o.getCount().get(), this.count.get()) at
PrintMessageByQueueCommand.java:253, and a shared static Comparator<MessageExt>
STORE_TIMESTAMP_COMPARATOR =
Comparator.comparingLong(MessageExt::getStoreTimestamp) used at
QueryMsgByUniqueKeySubCommand.java:76, with a short comment explaining why a
plain subtraction is unsafe. Regression tests, following the existing JUnit4
style in tools/src/test (org.junit.Assert, plain model objects, no mocking
required for the comparators): extend ConsumerProgressSubCommandTest with
compareTo antisymmetry over values straddling Integer.MAX_VALUE and a full
Collections.sort ordering assertion; add a new PrintMessageByQueueCommandTest
that orders TagCountBean values beyond the int range; extend
QueryMsgByUniqueKeySubCommandTest to sort messages with store timestamps more
than Integer.MAX_VALUE
ms apart and assert oldest-first ordering. About 300 changed lines across
three main files and three test files.
## 变更内容
```diff
.../consumer/ConsumerProgressSubCommand.java | 6 +-
.../message/PrintMessageByQueueCommand.java | 6 +-
.../message/QueryMsgByUniqueKeySubCommand.java | 12 +++-
.../consumer/ConsumerProgressSubCommandTest.java | 78
++++++++++++++++++++++
.../message/QueryMsgByUniqueKeySubCommandTest.java | 29 ++++++++
5 files changed, 128 insertions(+), 3 deletions(-)
tools/src/test/java/org/apache/rocketmq/tools/command/message/PrintMessageByQueueCommandTest.java
| 88 +++++++++
```
## 验证
PASSED: wsl -e bash scripts/gate-rocketmq.sh
## 检查清单
- [ ] 未运行本地测试（以仓库 CI 为准）
- [x] 变更最小且不包含无关文件
- [x] 已遵守仓库贡献规范
</details>
--
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]
