[scottf]

Follows #1641 and #1666, both merged. How the client takes the entries of connect_urls, and how it dials the servers it learns from them.

An entry is host:port. That is the form nats-server writes, with an IPv6 address in brackets, and it is now the only form the client uses. User info in an entry, user:pass@host:port, is dropped: the discovered server is sent the credentials the client uses for the server that reported it, as before. An entry that carries a scheme, nats://host:port or ws://host:port, is skipped as a whole, not reduced to its host and port. An entry that does not parse as host:port once its user info is dropped, or whose port is outside 1 to 65535, is skipped. The other entries of the INFO are still used. The rule is NatsUri.discoveredHostPort, applied in NatsConnection.handleInfo before the pool sees the list and again in NatsServerPool. nats-server never writes user info or a scheme into an entry, so nothing a cluster advertises changes; a proxy that rewrites entries into URLs loses them, and is told so.

A discovered server is dialed the way the server that reported it was dialed. It takes that server's scheme, so it gets the same transport and, the context being per connection, the same TLS: an entry learned over wss:// becomes a wss:// server, one learned over tls:// a tls:// server. Until now a discovered server took the scheme of the first configured server that was not plain nats://, which in a list that mixes WebSocket and TCP servers could be the wrong transport. Without an origin the default scheme applies as before. A configured server named by an entry does not change.

Skipped entries are reported. New ErrorListener.discoveredServerSkipped(Connection conn, String entry, String reason), a default method, called once per skipped entry each time an INFO carries it, with the entry as the server sent it. The logger and console listeners log it at WARNING with the entry quoted and control characters escaped. The runtime rejection from #1641 stays for a ws:// server a custom ServerPool hands out; a connect_urls entry can no longer bring one.

For custom ServerPool implementations: acceptDiscoveredUrls now receives host:port strings only, never an entry with a scheme or with user info. A pool that gave its discovered servers a scheme from the entry has to take it from the origin of acceptDiscoveredUrls(List, NatsUri) instead, as NatsServerPool does.

Docs: README paragraph under the SSLContext rules; docs/TLS_CONFIGURATION.md section 1 and new section 10.

Tests: OptionsTests.testNatsUriDiscoveredHostPort runs 20 used and 54 skipped forms, including entries written to confuse a URL parser (embedded control characters, mixed-case and percent-encoded schemes, path, query and fragment characters, bracket and numeric forms). ServerPoolTests: a discovered server takes the origin's scheme over one and two hops, the first report decides, configured servers do not change; the hygiene rule at the pool; user info in an entry is dropped and the origin's inherited. TLSConnectTests: entries with ws://, nats://, tls:// and wss:// schemes are skipped, reported once each per INFO, never pooled and never dialed, while the host:port entry next to them is pooled as tls://. ReconnectTests: an entry with user info does not log in as that user; the reconnect uses the configured url's credentials. ConnectionListenerTests: a scheme-bearing entry discovers nothing and is reported. Seven of these fail without the behavior change; the five untouched ServerPoolTests pass either way.

[synadia-claude-reviewer]

Claude finished @scottf's task in 3m 16s —— View job

---

Review complete

• Gather context (PR diff, description, CLAUDE.md)
• Review NatsUri.discoveredHostPort parsing logic
• Review NatsConnection.handleInfo / NatsServerPool changes
• Review ErrorListener/listener implementations
• Review test changes for coverage
• Post final review summary

I reviewed the implementation of NatsUri.discoveredHostPort, its use in NatsConnection.handleInfo and NatsServerPool.acceptDiscoveredUrls, the new ErrorListener.discoveredServerSkipped callback, and the docs/test changes.

Correctness

• discoveredHostPort correctly rejects any entry containing :// (scheme), strips user info up to the last @, validates the host (bracketed IPv6 or no embedded colon), validates the port digit-by-digit with early overflow rejection, and round-trips through URI to reject percent-encoded/path/query/fragment characters hiding inside the "host". The exhaustive OptionsTests.testNatsUriDiscoveredHostPort table (20 accepted / 54 rejected forms) backs this well, including the subtler Java URI quirks around numeric-looking hosts (999.1.1.1, 1.2.3, etc. vs. non-dotted numeric/hex hosts).
• NatsServerPool.acceptDiscoveredUrls now derives the dialing scheme from origin.getScheme() (falling back to the pool's default scheme when origin == null), and re-validates each entry with discoveredHostPort independently of NatsConnection, so a custom caller invoking the pool directly gets the same hygiene. The "first report wins" and "configured server unaffected" semantics are preserved via the existing findEquivalent/rejected set logic, which this PR doesn't disturb.
• One existing-but-worth-noting behavior, intentional and explicitly tested (testDiscoveredEntriesAreHostAndPortOnly): if every entry in a connect_urls list fails to parse, acceptDiscoveredUrls is called with an empty list, which prunes previously-gossiped servers just as a genuinely empty discovery list would. Pre-existing pruning behavior, not a regression from this change, but worth flagging since it means a single malformed INFO (e.g., from a misbehaving proxy) wipes all gossiped servers until the next good INFO repopulates them.
• Dropping the d.getUserInfo() == null guard in NatsServerPool (now unconditionally inheriting origin user info) is safe/correct since entries parsed via discoveredHostPort can never carry user info anymore.

Security

• Untrusted entries are consistently escaped before logging (quoteUntrusted(String) reusing the existing control-character-escaping/256-byte-cut logic from the TLS line handler), avoiding log/terminal injection via crafted connect_urls entries.
• Entries with a scheme are rejected as a whole rather than reduced to host:port, closing the path by which a malicious/misconfigured proxy could inject a ws:// (unencrypted) discovered server; this is exercised directly in TLSConnectTests.testConnectUrlsEntriesWithASchemeAreSkippedAndNotDialed.

Nits (not blocking)

• NatsConnection.handleInfo: building hostPorts always allocates a new ArrayList even when every entry is valid; this is a non-issue perf-wise (bounded by the INFO's connect_urls size).

Overall the change is well-scoped, the parsing logic matches the documented ADR-40 rules, and test coverage is thorough (unit-level NatsUri cases, pool-level scheme/credential-inheritance cases, and end-to-end TLS/reconnect/listener cases). No correctness or security issues found that need fixing.

[scottf]

On the ArrayList nit in handleInfo: reusing the input list would be more complicated than creating one. The new list holds the normalized form of each entry, host:port with leading zeros removed from the port, so the input strings cannot be passed through as they are even when nothing is skipped; reusing would mean detecting that no entry changed and no entry was skipped, then choosing which list to hand to the pool. The allocation is bounded by the size of connect_urls and happens once per INFO.