zjncs opened a new pull request, #11074:
URL: https://github.com/apache/rocketmq/pull/11074
## Motivation
`ConsumerRunningInfo.analyzeSubscription`, `isPushType` and
`analyzeProcessQueue` run on data reported by **each consumer over the wire**
(`DefaultMQAdminExtImpl.getConsumerRunningInfo` just forwards the broker
response), so the well-known properties cannot be assumed to exist. Two latent
crashes exist today when a client does not report them:
1. `analyzeSubscription` falls back to
`String.valueOf(properties.get(PROP_CONSUMER_START_TIMESTAMP))` when
`getProperty` returns null. For a genuinely **absent** property this produces
the literal string `"null"`, and `Long.parseLong("null")` throws:
```
java.lang.NumberFormatException: For input string: "null"
at java.base/java.lang.Long.parseLong(Long.java:XXX)
at
...ConsumerRunningInfo.analyzeSubscription(ConsumerRunningInfo.java:64)
```
This aborts `mqadmin consumerStatus` / `consumerSubCommand` / the consumer
monitor midway through the report.
2. `isPushType` (and the identical block inlined in `analyzeProcessQueue`)
cast the raw `properties.get(PROP_CONSUME_TYPE)` value to `ConsumeType` and
call `.name()` on it, throwing NPE when the property is absent:
```
java.lang.NullPointerException: Cannot invoke "ConsumeType.name()" because
the return value of "java.util.Properties.get(Object)" is null
```
`isPushType` is also called from
`QueryMsgByIdSubCommand`/`QueryMsgByUniqueKeySubCommand` on the same wire data.
The existing `if (property == null)` fallbacks were introduced to handle
**non-String** values after deserialization, but they fail to handle the
**missing** value. No Java client omits these properties today, but the wire
protocol is implemented by other clients/versions, and one misbehaving client
should not take down the whole admin command.
## Modification
- `analyzeSubscription`: read the start timestamp once via
`properties.get(...)`; when it is unknown, keep the startup grace
(`startForAWhile = false`) instead of crashing — the 2-minute window exists to
tolerate freshly started consumers, and an unknown start time deserves the same
grace rather than a false "different subscription" alarm.
- `isPushType`: return `false` when `PROP_CONSUME_TYPE` is unknown instead
of NPE-ing; a single code path now handles both String values and in-memory
`ConsumeType` values (the case covered by the existing test).
- `analyzeProcessQueue`: reuse `isPushType` and drop the duplicated block.
## Verification
`mvn -pl remoting test -Dtest=ConsumerRunningInfoTest`
fail-before (fix stashed, new tests kept):
```
Tests run: 3, Failures: 0, Errors: 1, Skipped: 1
testAnalyzeSubscriptionWithMissingStartTimestamp <<< ERROR!
java.lang.NumberFormatException: For input string: "null"
testAnalyzeMethodsTolerateMissingProperties <<< ERROR!
java.lang.NullPointerException: Cannot invoke "ConsumeType.name()" because
the return value of "java.util.Properties.get(Object)" is null
```
pass-after:
```
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0 - in ConsumerRunningInfoTest
BUILD SUCCESS
```
No associated issue; found by code inspection.
--
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]
