[JDK-8257080] Java does not try all DNS results when opening a socket

[Benjamin Marwell]

On 16.12.20 13:49, Alan Bateman wrote:
> On 16/12/2020 09:04, Benjamin Marwell wrote:
>> Hi Alan,
>>
>> > I can't tell if your proposal is focused on the Socket constructors
>> > that take a hostname or more generally changing all APIs that
>> > connect to a SocketAddress to support connecting to an
>> > isUnresolved InetSocketAddress
>>
>> Way before we even get to sockets. I am talking about InetAddress.
>> It’s resolving logic is resolve()[0]. That’s it. But an InetAddress 
>> does not carry any
>> port information and thus should not resolve in my opinion.
>>
>> ---
>>
>> I think I would start looking in java/net/AbstractPlainSocketImpl.java.
>> These are the relevant lines: 
>> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java#L149-L164 
>> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java*L149-L164__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeSYGhb8tw$> 
>>
>>
>> For the new implementation these lines seem pretty relevant:
>> https://github.com/openjdk/jdk/blob/51d5164ca2b4801c14466e8d1420ecf27cb7615f/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java#L560-L562 
>> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/51d5164ca2b4801c14466e8d1420ecf27cb7615f/src/java.base/share/classes/sun/nio/ch/NioSocketImpl.java*L560-L562__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeQy8PBhSg$> 
>>
>> That is where I would probably add the for loop.
>>
>> Here is python’s implementation as a reference:
>> https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/socket.py#L707-L719 
>> <https://urldefense.com/v3/__https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/socket.py*L707-L719__;Iw!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeTvZM9G2A$> 
>>
>>
>> By the way, any changes would solve a few other issues as well:
>> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8201428 
>> <https://urldefense.com/v3/__https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8201428__;!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeQkc7Ex1w$> 
>>
>> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780 
>> <https://urldefense.com/v3/__https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8192780__;!!GqivPVa7Brio!IQGo98vib-LqfZaVN1Y7MJ51_ZRRrljY_n6IM-btaxHD3suFHe77kiBgTeSpuxbIvA$> 
>>
>>
> 
> If I read your mail correctly, I think what you are proposing is to 
> change the existing APIs to allow an unresolved InetSocketAddress to be 
> specified, is that right? I suspect this approach will have a lot of 
> implications for methods that are called after the socket is created as 
> it would require re-creating the underlying socket when a connect 
> attempt fails (the socket may have been explicitly bound or socket 
> options were set). There are issues with the semantics of the timeout 
> parameter. It might not be possible to even do this with APIs such as 
> SocketChannel when the channel is configured non-blocking.
> 
> If the starting point is the Socket constructor that takes a host name 
> and port then it might be easier because that can do the lookup and 
> cycle through the addresses and/or overlap the connection attempts.
> 
> If you have cycles to do experiments and report back then it would be 
> useful. I assume the attractiveness of changing the existing APIs is so 
> that existing usages, e.g. HTTP protocol handler, could use it without 
> changing to a new APIs. Maybe changing existing vs. new API could be 
> part of your investigation? I think it would also be useful to know if a 
> simple iteration through the addresses is sufficient for more cases or 
> whether something close to Happy Eyeballs would be needed. There have 
> been a few discussions here over the years on the latter as it is hard 
> to get right and there may be a case of having support for that in the 
> APIs.
> 
> -Alan
> 
> 
> 

Huh… the APIs have already been changed which is why the issues above 
exist. There is no workaround anymore.

It is true that I did not take into account that other methods would 
rely on this. The biggest issue I see (easily) is the connect timeout.
It must either be distributed along all connection attempts or per 
connection attempt – both would change semantics.

I just looked up Happy Eyeballs, and this comes very close to
what I mean. I like that it implies DNS-RR.

Lets see if I (or anyone else) can come up with a sample implementation.

Thanks,
Ben