[ISSUE #10591] Fix IPv6 NPE in NetworkUtil.string2SocketAddress by stripping brackets

[itxaiohanglover]

Which Issue(s) This PR Fixes

Fixes #10591

Brief Description

When a gRPC client connects to the Proxy via IPv6, the remote address is formatted as [240e:...:2]:44880 (with brackets). The NetworkUtil.string2SocketAddress method splits on the last : to separate host and port, but the host part retains the surrounding [] brackets.

When new InetSocketAddress("[240e:...:2]", port) is constructed with a bracketed host, Java's InetAddress.getByName() fails to resolve it, causing getAddress() to return null. This null propagates to CommitLog.asyncPutMessage (line 994) where bornSocketAddress.getAddress() triggers a NullPointerException.

Root cause: string2SocketAddress does not strip IPv6 brackets before constructing InetSocketAddress.

Fix: Strip surrounding [] from the host part before calling new InetSocketAddress(host, port).

How Did You Test This Change?

Added NetworkUtilTest with 5 test cases:

• testString2SocketAddressIPv4 — existing IPv4 behavior unchanged
• testString2SocketAddressIPv6WithBrackets — reproduces the issue: [240e:341:6246:c700:c4ad:e645:459e:2]:44880 now resolves correctly with non-null getAddress()
• testString2SocketAddressIPv6Loopback — [::1]:8080 resolves correctly
• testSocketAddress2StringIPv4 — round-trip conversion unchanged
• testNormalizeAndDenormalizeHostAddress — bracket normalize/denormalize helpers

mvn -pl common -Dtest=NetworkUtilTest test
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

[yx9o]

Thanks for the fix. Could you also check SimpleChannel#parseSocketAddress? It currently uses address.split(":"), so [IPv6]:port returns null there. That seems closer to the bornSocketAddress is null NPE in #10591.

[itxaiohanglover]

Thanks for pointing that out! You're right — SimpleChannel#parseSocketAddress used address.split(":"), which produces more than 2 segments for IPv6 addresses like [240e:...:2]:44880, so it returns null and bornSocketAddress ends up null.

Fixed in the latest push — switched to lastIndexOf(":") to split host:port and strip brackets from IPv6 host, consistent with the NetworkUtil.string2SocketAddress fix.

[yx9o]

Thanks for the update. It would be better to add a unit test for the SimpleChannel change.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.17%. Comparing base (8242c1e) to head (316c1e0).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../org/apache/rocketmq/common/utils/NetworkUtil.java	50.00%	0 Missing and 1 partial ⚠️
.../rocketmq/proxy/service/channel/SimpleChannel.java	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10592      +/-   ##
=============================================
- Coverage      48.26%   48.17%   -0.09%     
+ Complexity     13433    13412      -21     
=============================================
  Files           1378     1378              
  Lines         100817   100823       +6     
  Branches       13040    13042       +2     
=============================================
- Hits           48660    48573      -87     
- Misses         46211    46293      +82     
- Partials        5946     5957      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes NPE when parsing IPv6 addresses in NetworkUtil.string2SocketAddress() and SimpleChannel.parseSocketAddress(). The root cause was using split(":") which breaks for IPv6 addresses containing multiple colons. The fix uses lastIndexOf(":") to correctly split host and port, and strips square brackets from IPv6 addresses.

Findings

• [Info] Good fix — switching from split(":") to lastIndexOf(":") correctly handles full IPv6 addresses like [240e:341:6246:c700:c4ad:e645:459e:2]:44880. The bracket stripping logic is also correct.

• [Info] The SimpleChannelTest addresses the unit test request from @yx9o's earlier review. Tests cover IPv4, IPv6 with brackets, IPv6 without brackets, and no-port edge cases.

• [Warning] NetworkUtil.java:198 and SimpleChannel.java:98 — Integer.parseInt(port) will throw NumberFormatException on malformed input (e.g., "[::1]:abc"). Both methods currently let this propagate uncaught. Consider wrapping in a try-catch and returning null (or logging a warning) for robustness, since these are utility methods that may receive untrusted input.

• [Info] NetworkUtilTest.java — The testString2SocketAddressIPv6WithoutBrackets test uses "240e:341:6246:c700:c4ad:e645:459e:2:44880" which is ambiguous (the last colon could be part of the IPv6 address or the port separator). The lastIndexOf(":") approach will treat 44880 as port and the rest as host, which is correct for this format. However, this is inherently fragile — consider documenting that IPv6 addresses without brackets are not reliably parseable.

Suggestions

1. Add input validation or try-catch around Integer.parseInt(port) in both methods.
2. Consider adding a Javadoc note on string2SocketAddress() specifying the expected format: host:port for IPv4, [host]:port for IPv6.

---

Automated review by github-manager-bot