RFR: 8346017: Socket.connect specified to throw UHE for unresolved address is problematic for SOCKS V5 proxy [v4]

Daniel Fuchs dfuchs at openjdk.org
Fri Dec 13 16:47:38 UTC 2024


On Fri, 13 Dec 2024 15:21:54 GMT, Volkan Yazıcı <duke at openjdk.org> wrote:

>> [JDK-8343791](https://bugs.openjdk.org/browse/JDK-8343791) modified `Socket::connect` to throw `UHE` on unresolved addresses – merged in #22160. As reported in [JDK-8346017](https://bugs.openjdk.org/browse/JDK-8346017), this renders `connect()` unusable when there is a (SOCKS) proxy in play, where calling `connect()` using unresolved addresses is a valid operation. This PR
>> 
>> 1. Reverts the earlier `Socket::connect` change
>> 2. Updates tests accordingly
>> 3. Enhances tests with proxied socket inputs
>> 
>> These changes require CSR and RN tickets. I will implement them once we agree on a solution here.
>
> Volkan Yazıcı has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Rename `proxiedSocket` to `noProxySocket`

Sorry for my comments on outdated code - my review had been pending and I'd forgotten to commit it. I've made the comments again on the new code.

test/jdk/java/net/Socket/ConnectSocksProxyTest.java line 82:

> 80:     @BeforeAll
> 81:     static void initProxyServer() throws IOException {
> 82:         PROXY_SERVER = new SocksServer(0);

We should use the constructor that takes an InetAddress and pass the loopback address here.

test/jdk/java/net/Socket/ConnectSocksProxyTest.java line 164:

> 162:     void testBoundSocketWithUnresolvedAddress() throws IOException {
> 163:         try (Socket socket = createProxiedSocket()) {
> 164:             socket.bind(new InetSocketAddress(0));

if the server is bound to the loopback, we should use the loopback here too

test/jdk/java/net/Socket/ConnectSocksProxyTest.java line 177:

> 175:         try (ServerSocket serverSocket = createEphemeralServerSocket()) {
> 176:             InetSocketAddress unresolvedAddress =
> 177:                     InetSocketAddress.createUnresolved("localhost", serverSocket.getLocalPort());

Suggestion:

                    InetSocketAddress.createUnresolved(getLoopbackAddress().getHostAddress(), serverSocket.getLocalPort());

-------------

PR Comment: https://git.openjdk.org/jdk/pull/22719#issuecomment-2541843845
PR Review Comment: https://git.openjdk.org/jdk/pull/22719#discussion_r1884191886
PR Review Comment: https://git.openjdk.org/jdk/pull/22719#discussion_r1884195241
PR Review Comment: https://git.openjdk.org/jdk/pull/22719#discussion_r1884200430


More information about the net-dev mailing list