zhaohai666 opened a new pull request, #2398:
URL: https://github.com/apache/rocketmq-dashboard/pull/2398
# PR: Backend Code Optimizations
- **Branch:** `feature/studio-backend-optimizations` (based on
`rocketmq-studio`)
- **Goal:** Bound a broker RPC amplification, make the data-source SSRF
guard fail-closed,
and a couple of low-risk cleanups. Behavior-preserving except the SSRF
edge case below.
## Changes
### 1. Bound the topic-consumer N+1 RPC fan-out (B1)
`RocketMQMetadataProvider.getTopicConsumers` called
`getTopicConsumersPage(instanceId, name, 1, Integer.MAX_VALUE)`, which made
the page
implementation issue one `examineConsumeStats` **and** one
`examineConsumerConnectionInfo`
RPC **per consumer group** — an unbounded N+1 against the broker for a
single HTTP request
(and a real DoS / broker-pressure surface on topics with many groups).
Capped the non-paginated scan with `TOPIC_CONSUMER_SCAN_LIMIT = 1000` so the
worst case is
bounded. Topics with more consumer groups should use the already-existing
paginated
`GET /api/topics/{name}/consumers/page` endpoint. `getTopicConsumers`
discards `total`,
so the cap does not change the contract for its caller.
### 2. Fail-closed data-source SSRF guard (B2)
`SettingsService.isAllowedDataSourceHost` caught `UnknownHostException` and
returned `true`
(fail-open), which handed unresolvable hosts to the connection layer and
diverged from
`UrlHostGuard.check(url, false)` — the policy used by the save/create path.
Changed the
catch to `return false` (fail-closed), matching `UrlHostGuard`. The
loopback/private
rejection policy is unchanged, so all existing tests pass (including the two
that call
`areAllowedDataSourceAddresses` directly).
### 3. Don't swallow per-group RPC failures silently (B12)
The two `catch (Exception ignored)` blocks in `getTopicConsumersPage` now
log the skipped
group at `debug` level, preserving the degrade-without-numbers behavior
while keeping the
cause available for troubleshooting.
### 4. Minor cleanup (B11)
`LlmConfigService.getConfig` used a redundant double negation
`!!StringUtils.hasText(token)`
→ `StringUtils.hasText(token)`.
## Tests
- `mvn -o checkstyle:check`: 0 violations.
- `mvn -o test -Dtest=SettingsServiceTest`: 39 passed (covers B2 + the direct
`areAllowedDataSourceAddresses` policy tests).
- `mvn -o test -Dtest=RocketMQMetadataProviderTest`: 19 passed (covers B1 +
B12).
- Full `mvn -o test` compilation of main + test sources succeeded.
## Deferred (documented, not in this PR)
From `docs/code-review-2026-08-19.md`:
- **B3 / B4** `findConsumerConnections` and DLQ list still issue one RPC per
group/topic.
Genuine fixes need concurrent/batch fetching and should be validated under
load before
broad rollout — kept separate to avoid behavior risk.
- **B5** `MqAdminExtFactory` cache has no TTL/eviction — separate hardening
(Caffeine-style
eviction + active `release` on credential/instance deletion).
- **B6** `TencentClientFactory` `@PreDestroy` — **verified not applicable**:
the Tencent SDK
`AbstractClient` (3.1.1500) exposes no `close()`/`shutdown()`, so there is
nothing to
release at shutdown (unlike the Aliyun `AsyncClient`, which does).
- **B7 / B8** per-request `RestClient` rebuild and per-message `Producer`
start/stop — pooling
requires careful instance/credential isolation; deferred.
- **B9** duplicate time-window validation and **B10** magic numbers →
`@ConfigurationProperties`
are larger refactors, deferred.
--
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]
