beautyarbutin opened a new issue, #11180:
URL: https://github.com/apache/rocketmq/issues/11180

   ### Before Creating the Bug Report
   
   - [x] I found a bug, not just asking a question, which should be created in 
GitHub Discussions.
   - [x] I searched the open issues and pull requests and found no duplicate.
   - [x] I confirmed that this bug belongs to Apache RocketMQ.
   
   ### Runtime platform environment
   
   All platforms; reproduced directly against `PopCheckPoint.indexOfAck`.
   
   ### RocketMQ version
   
   Branch: `develop`
   Commit: `42a6d576abda5761f56c74623a2ae49d0d8b91b5`
   
   ### JDK Version
   
   JDK 8+
   
   ### Describe the Bug
   
   For new-format POP checkpoints, `PopCheckPoint.indexOfAck` narrows the 
`long` difference between `ackOffset` and `startOffset` to `int` before looking 
it up in `queueOffsetDiff`:
   
   ```java
   return queueOffsetDiff.indexOf((int) (ackOffset - startOffset));
   ```
   
   An out-of-range ACK whose difference is congruent to a valid queue-offset 
difference modulo `2^32` is therefore accepted as that valid ACK. For example, 
if `startOffset` is `100` and `queueOffsetDiff` contains `3`, an ACK for `100 + 
(1L << 32) + 3` is incorrectly mapped to index `0`.
   
   The single-message ACK path does not validate the ACK against the queue's 
min/max offsets before constructing `AckMsg`. Both `PopBufferMergeService` and 
`PopReviveService` trust `indexOfAck`; a wrapped match can mark the checkpoint 
bit and treat a different popped message as acknowledged.
   
   ### Steps to Reproduce
   
   1. Create a `PopCheckPoint` with `startOffset = 100`.
   2. Add a new-format queue offset difference of `3` using `addDiff(3)`.
   3. Call `indexOfAck(100 + (1L << 32) + 3)`.
   4. Observe that the method returns `0` instead of `-1`.
   
   ### What Did You Expect to See?
   
   Only ACK offsets whose non-negative difference from `startOffset` fits the 
checkpoint's `int` offset-difference representation should be considered. An 
out-of-range difference must return `-1`.
   
   ### What Did You See Instead?
   
   The `long` difference wraps during the cast and matches a valid entry in 
`queueOffsetDiff`.
   
   ### Suggested Fix
   
   Calculate the difference as `long`, reject values outside `[0, 
Integer.MAX_VALUE]`, and only then narrow to `int`. Add focused regression 
coverage for both a normal ACK and a wrapped ACK.
   
   ### Are you willing to submit a PR?
   
   Yes.


-- 
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]

Reply via email to