zjncs opened a new pull request, #11094:
URL: https://github.com/apache/rocketmq/pull/11094
### Motivation
`SubscriptionGroupManager.subGroupTable` and
`TopicConfigManager.subTopicConfigTable` (the split-metadata path behind
`GET_ALL_SUBSCRIPTIONGROUP_CONFIG` / `GET_ALL_TOPIC_CONFIG`, gated by
`enableSplitMetadata`) slice the sorted key list using three client-supplied
values. Three inputs crash the broker request thread:
1. **`maxGroupNum`/`maxTopicNum` = 0 or negative** — `endIndex` collapses to
`beginIndex` (or below), so `keyList.get(endIndex - 1)` throws
`ArrayIndexOutOfBoundsException` (e.g. `Index -1 out of bounds`), or the
`ImmutableSortedMap.subMap` call throws for an inverted range when `beginIndex
> 0`.
2. **negative `groupSeq`/`topicSeq` with a matching data version** —
`keyList.get(beginIndex)` indexes negatively (e.g. `Index -3 out of bounds`).
3. **malformed `dataVersion` JSON** — `DataVersion.fromJson` throws
fastjson's `JSONException` (`illegal fieldName ...`) instead of degrading.
All of these come straight from the request header, so a misbehaving (or
malicious) client can fail every metadata fetch on a broker running with split
metadata enabled.
### Changes
- Clamp the sequence to `>= 0`.
- Skip the slice (return an empty page) when the max limit is `<= 0`.
- Parse the data version through a helper that treats an unparseable value
as an *unknown* version — which the existing fallback already handles by
restarting from index 0 — instead of letting the exception escape.
Behaviour for well-formed requests is unchanged.
### Verification
Six new tests (`SubscriptionGroupManagerTest`, `TopicConfigManagerTest`),
each red on develop and green with the patch:
- `...WithNonPositiveMaxGroupNum` / `...WithNonPositiveMaxTopicNum`: was
`ArrayIndexOutOfBoundsException: Index -1`
- `...WithNegativeGroupSeqClampsToHead` /
`...WithNegativeTopicSeqClampsToHead`: was `ArrayIndexOutOfBoundsException:
Index -3`
- `...WithMalformedDataVersionFallsBackToFullPage`: was
`com.alibaba.fastjson2.JSONException`
```
$ mvn -pl broker test
-Dtest='SubscriptionGroupManagerTest,TopicConfigManagerTest'
(before) Errors: ArrayIndexOutOfBoundsException x4, JSONException x2
(after) SubscriptionGroupManagerTest: Tests run: 10, Failures: 0, Errors: 0
TopicConfigManagerTest: Tests run: 17, Failures: 0, Errors: 0
```
Note: this branch also touches `SubscriptionGroupManager.java` (the
`subGroupTable` method); #11093 touches a different method
(`updateForbiddenValue`) in the same file, so one of the two may need a trivial
rebase.
--
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]
