yyqdbngt opened a new pull request, #4705:
URL: https://github.com/apache/rocketmq-dashboard/pull/4705
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Brief Description
`ProxyAddressService.addProxyAddr` validated a proxy address but stored the
caller's string
verbatim, and `normalizeProxyAddr` returned `normalized` (the trimmed input)
after the port check.
The host was never lower-cased and the port was never canonicalised, so
logically identical
addresses were registered as **distinct** entries in the backing
`LinkedHashSet`:
```
addProxyAddr("LocalHost:8081"); // stored as LocalHost:8081
addProxyAddr("localhost:8081"); // stored as localhost:8081 <- separate
entry
addProxyAddr("[2001:DB8::1]:080") // stored as [2001:DB8::1]:080
```
Host names are case-insensitive and `080` *is* port 80, so the registry
listed the same proxy two
or three times, `removeProxyAddr` only removed the exact spelling the user
happened to re-type, and
lookups by canonical address missed. The sibling parser for the other
address registry in the same
package, `NamesrvAddrParser.normalizeSegment`, already lower-cases the host
and re-parses the port
with `Integer.parseInt`; the two registries disagreed on what a duplicate is.
`normalizeProxyAddr` now canonicalises the address it validated: the host
(or the bracketed IPv6
literal) is lower-cased with `Locale.ROOT` and the port is re-parsed, so
`LocalHost:8081` becomes
`localhost:8081` and `[2001:DB8::1]:080` becomes `[2001:db8::1]:80`.
Validation behaviour is
unchanged — a malformed IPv6 literal or an out-of-range port is still
rejected with the same
messages, and the IPv6 literal is still validated *before* being re-emitted.
### How Did You Test This Change?
One new test method in `ProxyAddressServiceTest`,
`addProxyAddrShouldNormalizeHostCaseAndPortLikeTheNameServerAddressParser`:
it registers
`LocalHost:8081`, `localhost:8081` and `[2001:DB8::1]:080`, asserts the home
page list is exactly
`["127.0.0.1:8081", "localhost:8081", "[2001:db8::1]:80"]` (so the duplicate
host is collapsed and
the non-canonical spellings are *not* present), then removes the canonical
`[2001:db8::1]:80` and
asserts the entry really went away.
Before the fix (red):
```
$ cd server && mvn -B -ntp test -Dtest=ProxyAddressServiceTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
ProxyAddressServiceTest.addProxyAddrShouldNormalizeHostCaseAndPortLikeTheNameServerAddressParser:399
Expecting actual:
["127.0.0.1:8081", "LocalHost:8081", "localhost:8081",
"[2001:DB8::1]:080"]
to contain exactly (and in same order):
["127.0.0.1:8081", "localhost:8081", "[2001:db8::1]:80"]
but some elements were not found:
["[2001:db8::1]:80"]
and others were not expected:
["LocalHost:8081", "[2001:DB8::1]:080"]
```
After the fix (green) — the whole class, plus the neighbouring proxy classes:
```
$ cd server && mvn -B -ntp test -Dtest=ProxyAddressServiceTest
[INFO] Tests run: 21, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
$ cd server && mvn -B -ntp test
-Dtest='ProxyControllerTest,ProxyCompatControllerTest,ApacheRocketMqProxyMetricsCollectorTest'
[INFO] Tests run: 17, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
$ cd server && mvn -B -ntp test
-Dtest='ProxyConfigToolHandlerTest,ProxyListToolHandlerTest'
[INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
`mvn test` runs checkstyle in the `validate` phase: `You have 0 Checkstyle
violations.`
Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp
test` already reports
`Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes
are the MySQL 8 backed
Spring integration tests (`AuthServiceBootstrapIntegrationTest`,
`AuthServiceConcurrencyIntegrationTest`,
`AuthServiceSessionOverviewIntegrationTest`,
`HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the
external-CLI ones
(`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are
touched by this change.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no UI text in this change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header (no new files)
- [x] Documentation touched where behaviour changed (no documented contract
change: the address list still echoes the registered proxies, only now in
canonical form)
--
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]
