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

   ## Motivation
   
   `RemotingSerializable.decode(byte[], Class)` returns **null** when the 
request carries no body. Several broker processors dereference the decoded 
object immediately, so a request without a body (a bare RPC probe from a 
monitoring tool, or a misbehaving client) produces a `NullPointerException` 
inside the broker instead of a clean error response — the NPE propagates to the 
remoting layer's catch-all, which answers a generic "the process request error" 
and logs a full stack trace per request.
   
   Affected sites (all verified to NPE on a null body):
   
   | Processor method | First dereference |
   |---|---|
   | `ClientManageProcessor.heartBeat` | 
`heartbeatData.getHeartbeatFingerprint()` |
   | `QueryAssignmentProcessor.queryAssignment` | `requestBody.getTopic()` |
   | `QueryAssignmentProcessor.setMessageRequestMode` | 
`requestBody.getTopic()` |
   | `AdminBrokerProcessor.updateAndCreateTopicList` | 
`requestBody.getTopicConfigList()` |
   | `AdminBrokerProcessor.updateAndCreateSubscriptionGroupConfigList` | 
`subscriptionGroupList.getGroupConfigList()` |
   | `AdminBrokerProcessor.lockBatchMQ` | `requestBody.getConsumerGroup()` |
   | `AdminBrokerProcessor.unlockBatchMQ` | `requestBody.isOnlyThisBroker()` |
   | `AdminBrokerProcessor.notifyBrokerRoleChanged` | 
`syncStateSetInfo.getSyncStateSet()` |
   | `AdminBrokerProcessor.createUser` / `updateUser` | 
`userInfo.setUsername(...)` |
   
   The codebase already establishes the convention of null-checking after 
decode — `LiteSubscriptionCtlProcessor.processRequest`, `AckMessageProcessor` 
(batch ack), `updateAndCreateSubscriptionGroupConfig`, and `checkClientConfig` 
all guard — this PR brings the remaining sites in line.
   
   ## Modification
   
   After each affected decode, return `SYSTEM_ERROR` with a remark naming the 
missing body when the decode yields null. No behavior change for requests that 
carry a valid body.
   
   ## Test Evidence
   
   Added tests: `testHeartbeatWithNullBody` (ClientManageProcessorTest), 
`testQueryAssignmentWithNullBody` + `testSetMessageRequestModeWithNullBody` 
(QueryAssignmentProcessorTest), `testUpdateAndCreateTopicListWithNullBody` + 
`testLockBatchMQWithNullBody` (AdminBrokerProcessorTest).
   
   Fail-before (unpatched develop, new tests only):
   
   ```
   Tests run: 5, Failures: 0, Errors: 3, Skipped: 2
   java.lang.NullPointerException: Cannot invoke 
"...QueryAssignmentRequestBody.getTopic()" because "requestBody" is null
   java.lang.NullPointerException: Cannot invoke 
"...CreateTopicListRequestBody.getTopicConfigList()" because "requestBody" is 
null
   java.lang.NullPointerException: Cannot invoke 
"...HeartbeatData.getHeartbeatFingerprint()" because "heartbeatData" is null
   ```
   
   Pass-after (with fix):
   
   ```
   docker exec rmq-build mvn -pl broker test 
-Dtest='AdminBrokerProcessorTest,QueryAssignmentProcessorTest,ClientManageProcessorTest'
 -Dsurefire.failIfNoSpecifiedTests=true
   Tests run: 7,  Failures: 0, Errors: 0, Skipped: 0 - 
QueryAssignmentProcessorTest
   Tests run: 95, Failures: 0, Errors: 0, Skipped: 0 - AdminBrokerProcessorTest
   Tests run: 7,  Failures: 0, Errors: 0, Skipped: 0 - ClientManageProcessorTest
   ```
   
   No associated issue (self-discovered during a broker-processor self-audit).


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