unbridled-41 opened a new pull request, #10994:
URL: https://github.com/apache/rocketmq/pull/10994

   <!-- Please make sure the target branch is right. In most case, the target 
branch should be `develop`. -->
   
   ### Which Issue(s) This PR Fixes
   
   - Fixes #10990
   
   ### Brief Description
   
   `PopLongPollingService#notifyMessageArrivingFromRetry` dereferences the 
dispatch request's properties map without a null check:
   
   ```java
   String originGroup = properties.get(MessageConst.PROPERTY_ORIGIN_GROUP);   
// NPE when properties == null
   ```
   
   A `DispatchRequest` legitimately carries a null properties map 
(`MessageDecoder.string2messageProperties` returns null for a message stored 
without properties — e.g. written by a non-Java client on a `%RETRY%`-prefixed 
topic). The store's own code acknowledges nullability: 
`DefaultMessageStore#notifyMessageArrive4MultiQueue` guards `prop == null` and 
`PullRequestHoldService#notifyMessageArriving` guards `properties != null` — 
only the pop retry branch misses it.
   
   The blast radius is broker-wide: the listener is invoked from 
`DefaultMessageStore.ReputMessageService#doReput` **before** `reputFromOffset` 
is advanced, and `doReput` only catches `RocksDBException`. The NPE propagates 
to `ServiceThread.run`, which logs and loops — then re-reads the same commitlog 
record and throws again forever, so all dispatch stops (consume queues stop 
advancing, consumers see a full outage) until manual intervention, and the 
poison message survives restarts.
   
   This PR returns early when `properties == null`: a retry-topic message 
without properties cannot be mapped back to an origin group, so there is no 
long-polling request to wake up. This matches the guards used by the sibling 
listeners.
   
   ### How Did You Test This Change?
   
   Added 
`PopLongPollingServiceTest#testNotifyMessageArrivingFromRetryWithoutProperties`,
 which invokes `notifyMessageArrivingWithRetryTopic` on a `%RETRY%` topic with 
a null properties map. It throws NPE on the unfixed code and passes (no 
wake-up) with this change.
   
   `mvn -pl broker test -Dtest=PopLongPollingServiceTest` passes (13/13).


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