qiyu-lu opened a new issue, #4554:
URL: https://github.com/apache/rocketmq-dashboard/issues/4554

   ### Before Creating the Bug Report
   
   - [x] I have searched the open issues of this repository and believe that 
this is not a duplicate. (It continues #4181, which the stale bot closed, and 
the review of #4182.)
   - [x] This is a defect in RocketMQ Studio.
   - [x] I have stated the exact version below.
   
   ### Studio Version
   
   branch: `rocketmq-studio`
   git commit id: reproduced on `0a596661`; `BrokerTopologyGuards` is unchanged 
on the current head `d50ffecc` (still an exact `Set.contains` on the registered 
address string)
   deployed as: built from source (dev profile, H2)
   
   ### Runtime Environment
   
   Ubuntu (Linux 5.15), JDK Temurin 21, Chrome.
   
   ### Connected RocketMQ Cluster
   
   RocketMQ 5.5.1, direct to NameServer/Broker. The broker registers with a 
hostname (`brokerIP1=<hostname>`), the same style as the bundled 
`deploy/rocketmq/conf/broker-0.conf` (`brokerIP1=broker-0`).
   
   ### Describe the Bug
   
   When a broker registers with a hostname, querying a message by its physical 
offset msgId returns nothing. The offset msgId embeds the broker's IP, while 
`BrokerTopologyGuards.knownBrokerEndpoints` holds the registered 
`hostname:port` strings, so the guard treats the address as outside the 
topology. Unique-key msgId lookup is not affected.
   
   ### Steps to Reproduce
   
   1. Register a broker as `repro-hostname-broker:10911` (resolves to 
`172.26.0.4`).
   2. Send a message; querying by topic finds it.
   3. On the message page choose "by Message ID" and enter the `offsetMsgId` 
from the send result.
   
   ### What Did You Expect to See?
   
   The message is returned (the official SDK reads the same id successfully).
   
   ### What Did You See Instead?
   
   0 results; the guard logs "Rejecting decoded broker address ... not a known 
broker endpoint". Screenshots are in #4181.
   
   ### Additional Context — design proposal for a revised fix
   
   Thanks @lizhimins for the detailed review on #4182 — all five points are 
fair, and items 1–3 showed the first version put the fix in the wrong place. 
CONTRIBUTING now asks to agree on non-trivial designs in an issue first, so 
here is the proposal before I write code again.
   
   1. **No DNS on the request path.** Resolution moves out of the guard into an 
instance-scoped, short-TTL (30s, configurable) immutable topology snapshot, 
built single-flight per instance from one `examineBrokerClusterInfo()` call. 
Hostname endpoints are resolved through an injected `BrokerHostResolver` backed 
by a small bounded executor with a per-host timeout and an overall budget; IP 
literals are never looked up. The guard becomes a pure in-memory comparison, 
which also removes today's uncached cluster-info RPC per guard call.
   2. **Scoped, attributable candidates.** The snapshot keeps `clusterName` / 
`brokerName` / `brokerId` / registered address per endpoint. Candidates are 
limited to the instance's clusters using the existing 
`configuredClusterName(instanceId)` convention (default-cluster instance → that 
cluster only; registered instance → its NameServer). A match is logged with 
broker and cluster; an `ip:port` that maps to more than one broker in scope is 
rejected. Since an offset msgId carries no `brokerName`, split-horizon DNS / 
NAT can still cause a false *reject* — I would make that diagnosable (warn with 
endpoint + reason) rather than claim to solve it.
   3. **Trust boundary.** On a hostname match, remoting connects to the 
**registered address**, not the decoded IP, so a DNS answer never authorizes a 
new connect target — the msgId only selects among registered brokers. 
Hostname-matched offset ids therefore bypass `adminExt.viewMessage(topic, 
msgId)` (which decodes and connects on its own) and use the direct 
`viewMessage(brokerAddr, topic, offset)` path at all three call sites (message 
query, trace timestamp, DLQ resend). In addition, DNS-derived addresses are 
filtered with `UrlHostGuard.areAllowed(..., false)` (made public, no new rule). 
Exact literal matches against the registry keep today's behaviour.
   4. **Robustness.** Per-endpoint exception handling, deterministic iteration 
order, resolution failures logged at `warn`.
   5. **Reuse + tests.** One shared host/port helper based on 
`resolveGrpcTarget` (IPv6 brackets, port range); an injected resolver instead 
of `mockStatic(InetAddress)`; a `BrokerTopologyGuardsTest` that first locks in 
the existing exact-match behaviour and reuses `clusterInfoWithBrokerAddresses`; 
a fresh branch from current `rocketmq-studio`.
   
   **Two questions**
   
   - Is connecting to the registered address (and bypassing 
`adminExt.viewMessage` for hostname-matched offset ids) acceptable, or would 
you rather keep the decoded address plus the `UrlHostGuard` filter?
   - For the helper: migrate all five existing host/port splitters in this PR, 
or only `resolveGrpcTarget` plus the new code, with the rest as a follow-up? 
The former pushes the PR past ~10 files.
   
   ### Are You Willing to Submit a Pull Request?
   
   - [x] Yes, I am willing to submit a pull request.
   


-- 
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]

Reply via email to