On Wed, 20 Nov 2024 12:42:13 GMT, Volkan Yazıcı <[email protected]> wrote:
>> src/java.base/share/classes/java/net/Socket.java line 709:
>>
>>> 707: }
>>> 708:
>>> 709: try {
>>
>> At (old) L680, it checks that endpoint is an InetSocketAddress. We should
>> also check that it resolved, as in:
>>
>> if (epoint.isUnresolved()) {
>> close();
>> throw new UnknownHostException(epoint.getHostName() + " is
>> unresolved");
>> }
>>
>> The benefit is the the constructor does the validation before is delegates
>> to the SocketImpl. We could change the SocketImpl to throw IAE for this
>> case, it doesn't matter because it will never happen if Socket rejects it.
>
> Implemented in 424fc8ccd3a495a49f859f0cfc563cf463b3efeb, along with a test.
> Are you sure we want to have a `close()` there instead of `closeQuietly()`?
I can't think of any cases where close could throw here but it would be better
to catch any exception or error from close so that it doesn't override the UHE.
A supporting method to catch and add as a suppressed exception is okay but
maybe think more about the name as "quietly" it misleading, and "parentError"
is a bit strange as it's not an Error.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22160#discussion_r1850347109
