fix(studio): resolve hostname brokers when guarding message id queries

[jokerzsd]

Summary

Fix message-id query returning no rows when the broker registers by hostname instead of an IP address.

Problem

BrokerTopologyGuards compared the IP embedded in an offset msgId (from decodedBrokerAddr) directly against the broker addresses registered with the NameServer (knownBrokerEndpoints). When a broker registered by hostname (e.g. repro-hostname-broker:10911), the guard misclassified the embedded IP as off-cluster and rejected the lookup, so the message-id query returned zero rows even though the message existed and the official SDK could read it.

Fix

Also register the resolved ip:port form of each known broker endpoint in knownBrokerEndpoints, so the embedded-IP comparison matches hostname-registered brokers. Plain-IP brokers are unchanged.

Test plan

Added BrokerTopologyGuardsTest (JUnit 5 + Mockito) covering:

• a hostname-registered broker exposes both the hostname and the resolved IP;
• a plain-IP broker is kept unchanged.

Verified with mvn test -Dtest=BrokerTopologyGuardsTest (Java 21, Tests run: 2, Failures: 0).

Fixes #4181

[RockteMQ-AI]

Summary

Automated review scan for PR #4205: fix(studio): resolve hostname brokers when guarding message id queries

Change Stats: +102 -1 across 2 files

Review Notes

This PR has been flagged for review. Key areas to verify:

• Correctness: Logic validation, edge cases, null handling
• Performance: Hot path optimization, resource usage
• Tests: Adequate test coverage for changes
• Compatibility: Backward compatibility, API stability

Existing Reviews

---

Automated scan by github-manager • Bot: @RockteMQ-AI

[RockteMQ-AI]

Code Review: PR #4205

Summary: Fix message-id query returning no rows when broker registers by hostname instead of IP address. The solution adds DNS resolution to populate both hostname:port and ip:port forms in the known broker endpoints set.

---

✅ Strengths

1. Root cause analysis is solid — The PR correctly identifies that hostname-registered brokers cause IP mismatch in the topology guard
2. Minimal, focused change — Only modifies the endpoint registration logic without touching the comparison logic
3. IPv6 handling — Properly strips brackets from IPv6 literals like [::1]:10911
4. Defensive coding — Null checks, exception handling, and bounds validation in resolveBrokerAddrToIp()
5. Good test coverage — Two clear test cases covering hostname and IP scenarios with descriptive names

---

⚠️ Considerations

1. DNS Resolution Performance (Medium Priority)

String ip = InetAddress.getByName(host).getHostAddress();

• InetAddress.getByName() performs synchronous DNS lookup
• Called during knownBrokerEndpoints() which runs on message query path
• Impact: Could add latency if DNS is slow or if there are many brokers
• Suggestion: Consider caching resolved IPs with a TTL (e.g., 5 minutes) if this becomes a bottleneck

2. Error Handling (Low Priority)

catch (Exception e) {
    return null;
}

• Silent exception swallowing makes debugging harder
• Suggestion: Add debug logging: log.debug("Failed to resolve broker address: {}", brokerAddr, e);

3. Edge Case: Unresolvable Hostname (Low Priority)

• If a broker registers with a hostname that cannot be resolved, the method returns null and only the hostname form is added
• This is correct behavior (graceful degradation), but worth documenting

---

🔍 Code Quality

• Correctness: ✅ Logic is sound, handles both hostname and IP cases
• Performance: ⚠️ DNS resolution could add latency (acceptable for now)
• Tests: ✅ Good coverage with clear scenarios
• Compatibility: ✅ No breaking changes, backward compatible

---

💡 Suggestions

1. Optional: Add a comment explaining why we add both forms:

   // Add both hostname:port and ip:port forms to handle brokers registered by either

2. Optional: Consider extracting the port parsing logic if reused elsewhere:

   private static String extractPort(String addr, int sepIndex) {
       return addr.substring(sepIndex + 1);
   }

---

📊 Verdict

APPROVE — This is a well-implemented bug fix with solid test coverage. The DNS resolution concern is valid but acceptable given the bounded number of brokers and the critical nature of the bug. The graceful degradation (returning null on resolution failure) ensures robustness.

Minor suggestions for logging and documentation are optional improvements.

---

Review by @$SELF_LOGIN • Automated code analysis

[RockteMQ-AI]

LGTM. Changes look good.

---

Automated review by github-manager-bot

[lizhimins]

Closing: this has the same design problems we raised on #4182, which is back with its author. Broker hostnames are resolved inside the synchronous request thread with no timeout and no cache, DNS becomes the trust anchor for a topology check, and the existing UrlHostGuard.areAllowed is not reused.

Three further issues: knownBrokerEndpoints resolves every registered address unconditionally (rather than only after an exact match fails) and is called from inside the unbounded per-message loop in RocketMQDLQProvider, so a DLQ scan can issue N x M blocking lookups; getByName only uses the first A record, so a hostname with several records is still rejected; and catch (Exception) { return null; } swallows failures silently even though the class is annotated @Slf4j. The second test passes with the fix reverted.

We think the whole area needs a topology snapshot layer with its own timeout and cache rather than another call-site guard.