On Tue, 19 Nov 2024 10:06:02 GMT, Alan Bateman <[email protected]> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Apply review suggestions by Alan & Daniel
>
> src/java.base/share/classes/java/net/Socket.java line 706:
>
>> 704: try {
>> 705: getImpl().connect(epoint, timeout);
>> 706: } catch (IOException error) {
>
> At L680 we reject an endpoint that is not an InetSocketAddresss. I think we
> should follow this with a check to ensure that the address is resolved, that
> way Socket does all the validation before delegation to the SocketImpl. It
> means if getImpl().connect(..) throws any exception or error then you can
> close the Socket, meaning you can catch Throwable.
The spec is still not right: we throw SocketException if the socket is already
connected, but we do not close the socket in that case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1848762389
