[PR] [Studio] refactor(server): 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. [rocketmq-dashboard]

["zhaohai666 (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [Studio] refactor(server): 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. [rocketmq-dashboard]

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins merged PR #2398:
URL: https://github.com/apache/rocketmq-dashboard/pull/2398


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org