zjncs opened a new pull request, #11097:
URL: https://github.com/apache/rocketmq/pull/11097

   ### Motivation
   
   `QUERY_ASSIGNMENT` and `SET_MESSAGE_REQUEST_MODE` requests whose body is 
missing `topic`, `consumerGroup` or `messageModel` (e.g. sent by an old/foreign 
client or a hand-crafted request — the JSON body simply leaves the field unset) 
currently fail deep inside `QueryAssignmentProcessor` with a 
`NullPointerException` instead of a proper error response:
   
   - `getMessageRequestMode(null, ...)` / `findSubscriptionGroupConfig(null)` / 
`getConsumerGroupInfo(null)` → `ConcurrentHashMap.get(null)` throws NPE 
(`Object.hashCode() ... because "key" is null`);
   - `doLoadBalance` does `switch (messageModel)` → NPE on the null enum 
(`MessageModel.ordinal() ... is null`) — the `messageModel` field has no 
default in `QueryAssignmentRequestBody`;
   - `setMessageRequestMode` calls `topic.startsWith(...)` on a null topic → 
NPE.
   
   The client only receives an opaque `SYSTEM_ERROR` carrying the NPE stack. 
Guarding the required body fields up front and answering `INVALID_PARAMETER` 
matches how other processors (e.g. `AdminBrokerProcessor`, 
`PollingInfoProcessor`) respond to malformed requests.
   
   ### Modifications
   
   - `queryAssignment`: reject with `INVALID_PARAMETER` when 
`topic`/`consumerGroup` is empty or `messageModel` is null, before any table 
lookup or `doLoadBalance`.
   - `setMessageRequestMode`: reject with `INVALID_PARAMETER` when 
`topic`/`consumerGroup` is empty, before the retry-topic check and the config 
lookups.
   
   ### Verification
   
   Fail-before (each of the 5 new tests errors on unpatched code with the NPE 
described above), e.g.:
   
   ```
   testQueryAssignmentWithNullTopic » NullPointer  Cannot invoke 
"Object.hashCode()" because "key" is null
   testQueryAssignmentWithNullMessageModel » NullPointer  Cannot invoke 
"MessageModel.ordinal()" because "messageModel" is null
   testSetMessageRequestModeWithNullTopic » NullPointer  Cannot invoke 
"String.startsWith(String)" because "topic" is null
   ```
   
   Pass-after — full `QueryAssignmentProcessorTest` (5 existing + 5 new tests) 
is green:
   
   ```
   mvn -pl broker test -Dtest='QueryAssignmentProcessorTest'
   Tests run: 10, Failures: 0, Errors: 0, Skipped: 0
   ```


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