From 728b858c787567fa4eed6dd44730dfdb8b30be0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Jeli=C5=84ski?= Date: Tue, 24 Oct 2023 05:36:43 +0000 Subject: [PATCH] 8318130: SocksSocketImpl needlessly encodes hostname for IPv6 addresses Reviewed-by: dfuchs, jpai, aefimov, michaelm --- .../classes/java/net/SocksSocketImpl.java | 16 +- .../Socks/SocksSocketProxySelectorTest.java | 195 ++++++++++++++++++ 2 files changed, 203 insertions(+), 8 deletions(-) create mode 100644 test/jdk/java/net/Socks/SocksSocketProxySelectorTest.java diff --git a/src/java.base/share/classes/java/net/SocksSocketImpl.java b/src/java.base/share/classes/java/net/SocksSocketImpl.java index 9353452efeb..e6efddae65c 100644 --- a/src/java.base/share/classes/java/net/SocksSocketImpl.java +++ b/src/java.base/share/classes/java/net/SocksSocketImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -37,6 +37,8 @@ import sun.net.SocksProxy; import sun.net.spi.DefaultProxySelector; import sun.net.www.ParseUtil; +import static sun.net.util.IPAddressUtil.isIPv6LiteralAddress; + /** * SOCKS (V4 & V5) TCP socket implementation (RFC 1928). */ @@ -297,17 +299,15 @@ class SocksSocketImpl extends DelegatingSocketImpl implements SocksConsts { URI uri; // Use getHostString() to avoid reverse lookups String host = epoint.getHostString(); - // IPv6 literal? - if (epoint.getAddress() instanceof Inet6Address && - (!host.startsWith("[")) && (host.indexOf(':') >= 0)) { + if (isIPv6LiteralAddress(host)) { host = "[" + host + "]"; + } else { + host = ParseUtil.encodePath(host); } try { - uri = new URI("socket://" + ParseUtil.encodePath(host) + ":"+ epoint.getPort()); + uri = new URI("socket://" + host + ":"+ epoint.getPort()); } catch (URISyntaxException e) { - // This shouldn't happen - assert false : e; - uri = null; + throw new IOException("Failed to select a proxy", e); } Proxy p = null; IOException savedExc = null; diff --git a/test/jdk/java/net/Socks/SocksSocketProxySelectorTest.java b/test/jdk/java/net/Socks/SocksSocketProxySelectorTest.java new file mode 100644 index 00000000000..e43edb4f6f7 --- /dev/null +++ b/test/jdk/java/net/Socks/SocksSocketProxySelectorTest.java @@ -0,0 +1,195 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.NetworkInterface; +import java.net.Proxy; +import java.net.ProxySelector; +import java.net.Socket; +import java.net.SocketAddress; +import java.net.SocketException; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.List; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * @test + * @bug 8318130 + * @summary Tests that java.net.SocksSocketImpl produces correct arguments + * for proxy selector + * @run junit/othervm SocksSocketProxySelectorTest + */ +public class SocksSocketProxySelectorTest { + + public static final String SHORTEN_IPV6 = "((?<=\\[)0)?:(0:)+"; + + @BeforeAll + public static void beforeTest() { + ProxySelector.setDefault(new LoggingProxySelector()); + } + + // should match the host name + public static Stream ipLiterals() { + return Stream.of("127.0.0.1", + "[::1]", + "[fe80::1%1234567890]"); + } + + // should be wrapped in [ ] + public static Stream shortIpv6Literals() { + return Stream.of("::1", + "fe80::1%1234567890"); + } + + // with real interface names in scope + // should be wrapped in [ ], repeated 0's not trimmed + public static Stream linkLocalIpv6Literals() throws SocketException { + return NetworkInterface.networkInterfaces() + .flatMap(NetworkInterface::inetAddresses) + .filter(InetAddress::isLinkLocalAddress) + .map(InetAddress::getHostAddress); + } + + public static Stream hostNames() throws UnknownHostException { + return Stream.of( + InetAddress.getByAddress("localhost", new byte[] {127,0,0,1}), + InetAddress.getByAddress("bugs.openjdk.org", new byte[] {127,0,0,1}), + InetAddress.getByAddress("xn--kda4b0koi.com", new byte[] {127,0,0,1}) + ); + } + + /** + * Creates a socket connection, which internally triggers proxy selection for the target + * address. The test has been configured to use a {@link LoggingProxySelector ProxySelector} + * which throws an {@link IllegalArgumentException} with hostname in exception message. + * The test then verifies that the hostname matches the expected one. + * + * @throws Exception + */ + @ParameterizedTest + @MethodSource("ipLiterals") + public void testIpLiterals(String host) throws Exception { + try (Socket s1 = new Socket(host, 80)) { + fail("IOException was expected to be thrown, but wasn't"); + } catch (IOException ioe) { + // expected + // now verify the IOE was thrown for the correct expected reason + if (!(ioe.getCause() instanceof IllegalArgumentException iae)) { + // rethrow this so that the test output failure will capture the entire/real + // cause in its stacktrace + throw ioe; + } + assertNotNull(iae.getMessage(), "Host not found"); + assertEquals(host, + iae.getMessage().replaceFirst(SHORTEN_IPV6, "::"), + "Found unexpected host"); + } + } + + @ParameterizedTest + @MethodSource("shortIpv6Literals") + public void testShortIpv6Literals(String host) throws Exception { + try (Socket s1 = new Socket(host, 80)) { + fail("IOException was expected to be thrown, but wasn't"); + } catch (IOException ioe) { + // expected + // now verify the IOE was thrown for the correct expected reason + if (!(ioe.getCause() instanceof IllegalArgumentException iae)) { + // rethrow this so that the test output failure will capture the entire/real + // cause in its stacktrace + throw ioe; + } + assertNotNull(iae.getMessage(), "Host not found"); + assertEquals('[' + host + ']', + iae.getMessage().replaceFirst(SHORTEN_IPV6, "::"), + "Found unexpected host"); + } + } + + @ParameterizedTest + @MethodSource("linkLocalIpv6Literals") + public void testLinkLocalIpv6Literals(String host) throws Exception { + try (Socket s1 = new Socket(host, 80)) { + fail("IOException was expected to be thrown, but wasn't"); + } catch (IOException ioe) { + // expected + // now verify the IOE was thrown for the correct expected reason + if (!(ioe.getCause() instanceof IllegalArgumentException iae)) { + // rethrow this so that the test output failure will capture the entire/real + // cause in its stacktrace + throw ioe; + } + assertNotNull(iae.getMessage(), "Host not found"); + assertEquals('[' + host + ']', + iae.getMessage(), + "Found unexpected host"); + } + } + + @ParameterizedTest + @MethodSource("hostNames") + public void testHostNames(InetAddress host) throws Exception { + try (Socket s1 = new Socket(host, 80)) { + fail("IOException was expected to be thrown, but wasn't"); + } catch (IOException ioe) { + // expected + // now verify the IOE was thrown for the correct expected reason + if (!(ioe.getCause() instanceof IllegalArgumentException iae)) { + // rethrow this so that the test output failure will capture the entire/real + // cause in its stacktrace + throw ioe; + } + assertNotNull(iae.getMessage(), "Host not found"); + assertEquals(host.getHostName(), + iae.getMessage(), + "Found unexpected host"); + } + } + + /** + * A {@link ProxySelector} which throws an IllegalArgumentException + * with the given hostname in exception message + */ + private static final class LoggingProxySelector extends + ProxySelector { + + @Override + public List select(final URI uri) { + throw new IllegalArgumentException(uri.getHost()); + } + + @Override + public void connectFailed(URI uri, SocketAddress sa, IOException ioe) { + + } + } +}