On Tue, 19 Sep 2023 23:54:31 GMT, Liam Miller-Cushon <cus...@openjdk.org> wrote:
> > I think Alex has commented that this can be made more obvious with a check
> > for AF_UNSPEC.
>
> Thanks, yes, I have applied that suggestion.
>
> > > For 748, it's handling a situation where IPv4 isn't specifically
> > > preferred. The implementation comment notes 'if preferredAddressFamily is
> > > AF_INET6 or not set'.
> >
> >
> > What does "specifically preferred" mean? By default it will be set to
> > AF_INET. If it is set to AF_INET, you can't tell if
> > `java.net.preferIPv6Addresses=false` was specified (which implies AF_INET),
> > or if nothing was specified and it just defaulted to AF_INET. The comment
> > and the code read as if preferredAddressFamily will be something other than
> > AF_INET unless `java.net.preferIPv6Addresses=false` is used. Maybe the code
> > is correct and the comment needs some cleaning up. TBH I don't understand
> > what this code is suppose to do.
>
> I agree 'preferredAddressFamily is AF_INET6 or not set' is unclear, since
> `preferredAddressFamily` was only ever `AF_INET` or `AF_INET6`. I wondered if
> the use of `if (preferredAddressFamily != AF_INET) {` instead of `if
> (preferredAddressFamily == AF_INET6) {` and the comment about 'not set' was
> referring to the possibility of `AF_UNSPEC`. The history for this logic is in
>
> * https://bugs.openjdk.org/browse/JDK-8250630
>
> *
> https://mail.openjdk.org/pipermail/serviceability-dev/2020-August/thread.html#32673
>
>
> The bug mentions it fixed an issue on Alpine Linux, and I don't have
> convenient access to a test environment for that.
>
> I could update L750 to `if (preferredAddressFamily == AF_INET6) {` to prevent
> this logic from re-ordering address when
> `java.net.preferIPv6Addresses=system` is set.
>
> What do you think?
I took some time to understand what this code for.
I think the condition is not correct. it should be
if (!allowOnlyIPv4) {
allowOnlyIPv4 (reflects "java.net.preferIPv4Stack" sys.prop) means that only
IPv4 connections are allowed (even if IPv6 available)
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15796#issuecomment-1726708238
