fix(remoting): tolerate missing properties in ConsumerRunningInfo analyze methods

[zjncs]

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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

LGTM — solid NPE fix. The original code had a latent bug: getProperty() returns null for non-String values stored in the Properties map, and the fallback get() could also return null, leading to Long.parseLong(null) → NPE.

The fix correctly handles null at each access point. Nice refactor to extract isPushType() and reuse it in analyzeProcessQueue(), eliminating the duplicated property-reading logic.

Test coverage for the null-property scenarios is well done.

---

Automated review by github-manager-bot