Skip to content

Commit 3eb5461

Browse files
vyjaikiran
authored andcommitted
8343791: Socket.connect API should document whether the socket will be closed when hostname resolution fails or another error occurs
Reviewed-by: dfuchs, alanb
1 parent 4ac2e47 commit 3eb5461

File tree

5 files changed

+226
-42
lines changed

5 files changed

+226
-42
lines changed

src/java.base/share/classes/java/net/Socket.java

+37-19
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ private Socket(SocketAddress address, SocketAddress localAddr, boolean stream)
454454
throws IOException
455455
{
456456
Objects.requireNonNull(address);
457+
assert address instanceof InetSocketAddress;
457458

458459
// create the SocketImpl and the underlying socket
459460
SocketImpl impl = createImpl();
@@ -463,16 +464,13 @@ private Socket(SocketAddress address, SocketAddress localAddr, boolean stream)
463464
this.state = SOCKET_CREATED;
464465

465466
try {
466-
if (localAddr != null)
467+
if (localAddr != null) {
467468
bind(localAddr);
468-
connect(address);
469-
} catch (IOException | IllegalArgumentException e) {
470-
try {
471-
close();
472-
} catch (IOException ce) {
473-
e.addSuppressed(ce);
474469
}
475-
throw e;
470+
connect(address);
471+
} catch (Throwable throwable) {
472+
closeSuppressingExceptions(throwable);
473+
throw throwable;
476474
}
477475
}
478476

@@ -571,6 +569,10 @@ void setConnected() {
571569
/**
572570
* Connects this socket to the server.
573571
*
572+
* <p> If the endpoint is an unresolved {@link InetSocketAddress}, or the
573+
* connection cannot be established, then the socket is closed, and an
574+
* {@link IOException} is thrown.
575+
*
574576
* <p> This method is {@linkplain Thread#interrupt() interruptible} in the
575577
* following circumstances:
576578
* <ol>
@@ -589,6 +591,8 @@ void setConnected() {
589591
* @param endpoint the {@code SocketAddress}
590592
* @throws IOException if an error occurs during the connection, the socket
591593
* is already connected or the socket is closed
594+
* @throws UnknownHostException if the endpoint is an unresolved
595+
* {@link InetSocketAddress}
592596
* @throws java.nio.channels.IllegalBlockingModeException
593597
* if this socket has an associated channel,
594598
* and the channel is in non-blocking mode
@@ -605,6 +609,11 @@ public void connect(SocketAddress endpoint) throws IOException {
605609
* A timeout of zero is interpreted as an infinite timeout. The connection
606610
* will then block until established or an error occurs.
607611
*
612+
* <p> If the endpoint is an unresolved {@link InetSocketAddress}, the
613+
* connection cannot be established, or the timeout expires before the
614+
* connection is established, then the socket is closed, and an
615+
* {@link IOException} is thrown.
616+
*
608617
* <p> This method is {@linkplain Thread#interrupt() interruptible} in the
609618
* following circumstances:
610619
* <ol>
@@ -625,6 +634,8 @@ public void connect(SocketAddress endpoint) throws IOException {
625634
* @throws IOException if an error occurs during the connection, the socket
626635
* is already connected or the socket is closed
627636
* @throws SocketTimeoutException if timeout expires before connecting
637+
* @throws UnknownHostException if the endpoint is an unresolved
638+
* {@link InetSocketAddress}
628639
* @throws java.nio.channels.IllegalBlockingModeException
629640
* if this socket has an associated channel,
630641
* and the channel is in non-blocking mode
@@ -644,26 +655,25 @@ public void connect(SocketAddress endpoint, int timeout) throws IOException {
644655
if (isClosed(s))
645656
throw new SocketException("Socket is closed");
646657
if (isConnected(s))
647-
throw new SocketException("already connected");
658+
throw new SocketException("Already connected");
648659

649660
if (!(endpoint instanceof InetSocketAddress epoint))
650661
throw new IllegalArgumentException("Unsupported address type");
651662

663+
if (epoint.isUnresolved()) {
664+
var uhe = new UnknownHostException(epoint.getHostName());
665+
closeSuppressingExceptions(uhe);
666+
throw uhe;
667+
}
668+
652669
InetAddress addr = epoint.getAddress();
653-
int port = epoint.getPort();
654670
checkAddress(addr, "connect");
655671

656672
try {
657673
getImpl().connect(epoint, timeout);
658-
} catch (SocketTimeoutException e) {
659-
throw e;
660-
} catch (InterruptedIOException e) {
661-
Thread thread = Thread.currentThread();
662-
if (thread.isVirtual() && thread.isInterrupted()) {
663-
close();
664-
throw new SocketException("Closed by interrupt");
665-
}
666-
throw e;
674+
} catch (IOException error) {
675+
closeSuppressingExceptions(error);
676+
throw error;
667677
}
668678

669679
// connect will bind the socket if not previously bound
@@ -1589,6 +1599,14 @@ public boolean getReuseAddress() throws SocketException {
15891599
return ((Boolean) (getImpl().getOption(SocketOptions.SO_REUSEADDR))).booleanValue();
15901600
}
15911601

1602+
private void closeSuppressingExceptions(Throwable parentException) {
1603+
try {
1604+
close();
1605+
} catch (IOException exception) {
1606+
parentException.addSuppressed(exception);
1607+
}
1608+
}
1609+
15921610
/**
15931611
* Closes this socket.
15941612
* <p>

src/java.base/share/classes/sun/nio/ch/Net.java

+4-21
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.net.StandardSocketOptions;
4141
import java.net.UnknownHostException;
4242
import java.nio.channels.AlreadyBoundException;
43+
import java.nio.channels.AlreadyConnectedException;
4344
import java.nio.channels.ClosedChannelException;
4445
import java.nio.channels.NotYetBoundException;
4546
import java.nio.channels.NotYetConnectedException;
@@ -166,6 +167,8 @@ else if (x instanceof NotYetConnectedException)
166167
nx = newSocketException("Socket is not connected");
167168
else if (x instanceof AlreadyBoundException)
168169
nx = newSocketException("Already bound");
170+
else if (x instanceof AlreadyConnectedException)
171+
nx = newSocketException("Already connected");
169172
else if (x instanceof NotYetBoundException)
170173
nx = newSocketException("Socket is not bound yet");
171174
else if (x instanceof UnsupportedAddressTypeException)
@@ -190,32 +193,12 @@ private static SocketException newSocketException(String msg) {
190193
return new SocketException(msg);
191194
}
192195

193-
static void translateException(Exception x,
194-
boolean unknownHostForUnresolved)
195-
throws IOException
196-
{
196+
static void translateException(Exception x) throws IOException {
197197
if (x instanceof IOException ioe)
198198
throw ioe;
199-
// Throw UnknownHostException from here since it cannot
200-
// be thrown as a SocketException
201-
if (unknownHostForUnresolved &&
202-
(x instanceof UnresolvedAddressException))
203-
{
204-
throw new UnknownHostException();
205-
}
206199
translateToSocketException(x);
207200
}
208201

209-
static void translateException(Exception x)
210-
throws IOException
211-
{
212-
translateException(x, false);
213-
}
214-
215-
private static InetSocketAddress getLoopbackAddress(int port) {
216-
return new InetSocketAddress(InetAddress.getLoopbackAddress(), port);
217-
}
218-
219202
private static final InetAddress ANY_LOCAL_INET4ADDRESS;
220203
private static final InetAddress ANY_LOCAL_INET6ADDRESS;
221204
private static final InetAddress INET4_LOOPBACK_ADDRESS;

src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -599,8 +599,11 @@ protected void connect(SocketAddress remote, int millis) throws IOException {
599599
}
600600
} catch (IOException ioe) {
601601
close();
602-
if (ioe instanceof InterruptedIOException) {
602+
if (ioe instanceof SocketTimeoutException) {
603603
throw ioe;
604+
} else if (ioe instanceof InterruptedIOException) {
605+
assert Thread.currentThread().isVirtual();
606+
throw new SocketException("Closed by interrupt");
604607
} else {
605608
throw SocketExceptions.of(ioe, isa);
606609
}

src/java.base/share/classes/sun/nio/ch/SocketAdaptor.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.net.SocketException;
3636
import java.net.SocketOption;
3737
import java.net.StandardSocketOptions;
38+
import java.net.UnknownHostException;
3839
import java.nio.channels.SocketChannel;
3940
import java.util.Set;
4041

@@ -85,6 +86,14 @@ public void connect(SocketAddress remote) throws IOException {
8586
public void connect(SocketAddress remote, int timeout) throws IOException {
8687
if (remote == null)
8788
throw new IllegalArgumentException("connect: The address can't be null");
89+
if (remote instanceof InetSocketAddress isa && isa.isUnresolved()) {
90+
if (!sc.isOpen())
91+
throw new SocketException("Socket is closed");
92+
if (sc.isConnected())
93+
throw new SocketException("Already connected");
94+
close();
95+
throw new UnknownHostException(remote.toString());
96+
}
8897
if (timeout < 0)
8998
throw new IllegalArgumentException("connect: timeout can't be negative");
9099
try {
@@ -95,7 +104,7 @@ public void connect(SocketAddress remote, int timeout) throws IOException {
95104
sc.blockingConnect(remote, Long.MAX_VALUE);
96105
}
97106
} catch (Exception e) {
98-
Net.translateException(e, true);
107+
Net.translateException(e);
99108
}
100109
}
101110

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
import jdk.test.lib.Utils;
25+
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.params.ParameterizedTest;
27+
import org.junit.jupiter.params.provider.MethodSource;
28+
29+
import java.io.IOException;
30+
import java.net.InetAddress;
31+
import java.net.InetSocketAddress;
32+
import java.net.ServerSocket;
33+
import java.net.Socket;
34+
import java.net.SocketException;
35+
import java.net.UnknownHostException;
36+
import java.nio.channels.SocketChannel;
37+
import java.util.List;
38+
39+
import static org.junit.jupiter.api.Assertions.assertEquals;
40+
import static org.junit.jupiter.api.Assertions.assertFalse;
41+
import static org.junit.jupiter.api.Assertions.assertThrows;
42+
import static org.junit.jupiter.api.Assertions.assertTrue;
43+
44+
/*
45+
* @test
46+
* @bug 8343791
47+
* @summary verifies that `connect()` failures throw the expected exception and leave socket in the expected state
48+
* @library /test/lib
49+
* @run junit ConnectFailTest
50+
*/
51+
class ConnectFailTest {
52+
53+
private static final int DEAD_SERVER_PORT = 0xDEAD;
54+
55+
private static final InetSocketAddress REFUSING_SOCKET_ADDRESS = Utils.refusingEndpoint();
56+
57+
private static final InetSocketAddress UNRESOLVED_ADDRESS =
58+
InetSocketAddress.createUnresolved("no.such.host", DEAD_SERVER_PORT);
59+
60+
@Test
61+
void testUnresolvedAddress() {
62+
assertTrue(UNRESOLVED_ADDRESS.isUnresolved());
63+
}
64+
65+
/**
66+
* Verifies that an unbound socket is closed when {@code connect()} fails.
67+
*/
68+
@ParameterizedTest
69+
@MethodSource("sockets")
70+
void testUnboundSocket(Socket socket) throws IOException {
71+
try (socket) {
72+
assertFalse(socket.isBound());
73+
assertFalse(socket.isConnected());
74+
assertThrows(IOException.class, () -> socket.connect(REFUSING_SOCKET_ADDRESS));
75+
assertTrue(socket.isClosed());
76+
}
77+
}
78+
79+
/**
80+
* Verifies that a bound socket is closed when {@code connect()} fails.
81+
*/
82+
@ParameterizedTest
83+
@MethodSource("sockets")
84+
void testBoundSocket(Socket socket) throws IOException {
85+
try (socket) {
86+
socket.bind(new InetSocketAddress(0));
87+
assertTrue(socket.isBound());
88+
assertFalse(socket.isConnected());
89+
assertThrows(IOException.class, () -> socket.connect(REFUSING_SOCKET_ADDRESS));
90+
assertTrue(socket.isClosed());
91+
}
92+
}
93+
94+
/**
95+
* Verifies that a connected socket is not closed when {@code connect()} fails.
96+
*/
97+
@ParameterizedTest
98+
@MethodSource("sockets")
99+
void testConnectedSocket(Socket socket) throws Throwable {
100+
try (socket; ServerSocket serverSocket = createEphemeralServerSocket()) {
101+
socket.connect(serverSocket.getLocalSocketAddress());
102+
try (Socket _ = serverSocket.accept()) {
103+
assertTrue(socket.isBound());
104+
assertTrue(socket.isConnected());
105+
SocketException exception = assertThrows(
106+
SocketException.class,
107+
() -> socket.connect(REFUSING_SOCKET_ADDRESS));
108+
assertEquals("Already connected", exception.getMessage());
109+
assertFalse(socket.isClosed());
110+
}
111+
}
112+
}
113+
114+
/**
115+
* Verifies that an unbound socket is closed when {@code connect()} is invoked using an unresolved address.
116+
*/
117+
@ParameterizedTest
118+
@MethodSource("sockets")
119+
void testUnboundSocketWithUnresolvedAddress(Socket socket) throws IOException {
120+
try (socket) {
121+
assertFalse(socket.isBound());
122+
assertFalse(socket.isConnected());
123+
assertThrows(UnknownHostException.class, () -> socket.connect(UNRESOLVED_ADDRESS));
124+
assertTrue(socket.isClosed());
125+
}
126+
}
127+
128+
/**
129+
* Verifies that a bound socket is closed when {@code connect()} is invoked using an unresolved address.
130+
*/
131+
@ParameterizedTest
132+
@MethodSource("sockets")
133+
void testBoundSocketWithUnresolvedAddress(Socket socket) throws IOException {
134+
try (socket) {
135+
socket.bind(new InetSocketAddress(0));
136+
assertTrue(socket.isBound());
137+
assertFalse(socket.isConnected());
138+
assertThrows(UnknownHostException.class, () -> socket.connect(UNRESOLVED_ADDRESS));
139+
assertTrue(socket.isClosed());
140+
}
141+
}
142+
143+
/**
144+
* Verifies that a connected socket is not closed when {@code connect()} is invoked using an unresolved address.
145+
*/
146+
@ParameterizedTest
147+
@MethodSource("sockets")
148+
void testConnectedSocketWithUnresolvedAddress(Socket socket) throws Throwable {
149+
try (socket; ServerSocket serverSocket = createEphemeralServerSocket()) {
150+
socket.connect(serverSocket.getLocalSocketAddress());
151+
try (Socket _ = serverSocket.accept()) {
152+
assertTrue(socket.isBound());
153+
assertTrue(socket.isConnected());
154+
assertThrows(IOException.class, () -> socket.connect(UNRESOLVED_ADDRESS));
155+
assertFalse(socket.isClosed());
156+
}
157+
}
158+
}
159+
160+
static List<Socket> sockets() throws Exception {
161+
Socket socket = new Socket();
162+
@SuppressWarnings("resource")
163+
Socket channelSocket = SocketChannel.open().socket();
164+
return List.of(socket, channelSocket);
165+
}
166+
167+
private static ServerSocket createEphemeralServerSocket() throws IOException {
168+
return new ServerSocket(0, 0, InetAddress.getLoopbackAddress());
169+
}
170+
171+
}

0 commit comments

Comments
 (0)