Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ public static InetSocketAddress createSocketAddr(
URI uri = createURI(target, hasScheme, helpText, useCacheIfPresent);

String host = uri.getHost();
if (host != null && host.startsWith("[") && host.endsWith("]")) {
host = host.substring(1, host.length() - 1);
}
int port = uri.getPort();
if (port == -1) {
port = defaultPort;
Expand Down Expand Up @@ -763,10 +766,29 @@ public static String getHostname() {
* Compose a "host:port" string from the address.
*
* @param addr address.
* @return hort port string.
* @return host port string.
*/
public static String getHostPortString(InetSocketAddress addr) {
return addr.getHostName() + ":" + addr.getPort();
return getHostPortString(addr.getHostName(), addr.getPort());
}

/**
* Compose a "host:port" string, bracketing IPv6 literals.
*
* @param host host name or IP address.
* @param port port number.
* @return host port string.
*/
public static String getHostPortString(String host, int port) {
String normalizedHost = host;
if (normalizedHost != null && normalizedHost.startsWith("[")
&& normalizedHost.endsWith("]")) {
normalizedHost = normalizedHost.substring(1, normalizedHost.length() - 1);
}
if (normalizedHost != null && normalizedHost.contains(":")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new helper emits bracketed IPv6 authorities (e.g. "[::1]:123"), but the existing getPortFromHostPortString() (unchanged in this PR) still splits on ":" and requires exactly one colon — it cannot parse bracketed IPv6 strings.

Token round-trip via SecurityUtil.getTokenServiceAddr() → createSocketAddr() is fine, but the two helpers are now inconsistent. Any caller that formats with getHostPortString and parses with getPortFromHostPortString will break on IPv6.

Worth updating getPortFromHostPortString() to accept "[<ipv6>]:<port>" (and reject ambiguous unbracketed IPv6), with tests, either here or as an immediate follow-up on the same JIRA.

return "[" + normalizedHost + "]:" + port;
}
return normalizedHost + ":" + port;
Comment on lines +788 to +791
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ public static void setTokenService(Token<?> token, InetSocketAddress addr) {
* hadoop.security.token.service.use_ip
*/
public static Text buildTokenService(InetSocketAddress addr) {
String host = null;
String host;
if (useIpForTokenService) {
if (addr.isUnresolved()) { // host has no ip address
throw new IllegalArgumentException(
Expand All @@ -491,7 +491,7 @@ public static Text buildTokenService(InetSocketAddress addr) {
} else {
host = StringUtils.toLowerCase(addr.getHostName());
}
return new Text(host + ":" + addr.getPort());
return new Text(NetUtils.getHostPortString(host, addr.getPort()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When hadoop.security.token.service.use_ip=true, the service string will use whatever canonical form InetAddress.getHostAddress() returns (e.g. 0:0:0:0:0:0:0:1 vs ::1). That is pre-existing JDK behavior, not introduced here, but operators enabling IPv6 should know token service strings may use the expanded form. A brief release note may help.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,19 @@ public void testTrimCreateSocketAddress() {
assertEquals(defaultAddr.trim(), NetUtils.getHostPortString(addr));
}

@Test
public void testIPv6HostPortString() {
assertEquals("[::1]:123", NetUtils.getHostPortString("::1", 123));
assertEquals("[::1]:123", NetUtils.getHostPortString("[::1]", 123));

InetSocketAddress addr = NetUtils.createSocketAddrUnresolved("[::1]:123");
assertEquals("::1", addr.getHostString());
assertEquals(123, addr.getPort());

assertThrows(IllegalArgumentException.class,
() -> NetUtils.createSocketAddr("::1:123"));
}

@Test
public void testGetPortFromHostPortString() throws Exception {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ void runBadPortPermutes(String arg, boolean validIfPosPort) {
String serviceHost = useIp ? ip : StringUtils.toLowerCase(host);

Token<?> token = new Token<TokenIdentifier>();
Text service = new Text(serviceHost+":"+port);
Text service = new Text(NetUtils.getHostPortString(serviceHost, port));

assertEquals(service, SecurityUtil.buildTokenService(addr));
SecurityUtil.setTokenService(token, addr);
Expand Down Expand Up @@ -366,6 +366,14 @@ public void testSocketAddrWithIP() {
verifyServiceAddr(staticHost, "127.0.0.1");
}

@Test
public void testSocketAddrWithIPv6() throws Exception {
SecurityUtil.setTokenServiceUseIp(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: SecurityUtil.setTokenServiceUseIp(false) here is redundant — verifyAddress() already exercises both use_ip=true and use_ip=false via verifyTokenService().

String host = "::1";
InetSocketAddress addr = NetUtils.createSocketAddr("[::1]:123");
verifyAddress(addr, host, InetAddress.getByName(host).getHostAddress(), 123);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testSocketAddrWithIPv6 assumes the resolved hostname for ::1 remains "::1", but on JDK 17 (verified locally) createSocketAddr("[::1]:123") yields getHostName() == "localhost". That breaks verifyValues() when use_ip=false and hostname-mode token service expectations ("[localhost]:123" vs "[::1]:123").

The PR description says both use_ip=true and use_ip=false round-trips are verified; this test may not reliably cover hostname mode depending on /etc/hosts and JDK reverse-DNS behavior.

Consider deriving the expected hostname from addr.getHostName() after creation, using NetUtils.addStaticResolution for a stable name (consistent with other tests in this class), or splitting explicit coverage for use_ip=true vs hostname mode.

}

@Test
public void testSocketAddrWithNameToStaticName() {
String staticHost = "host1";
Expand Down
Loading