HTTPCLIENT-2350 - Enhance HttpClientConnectionOperator with Optional DNS Resolution Control

[arturobernalg]

This PR introduces an enhancement to DefaultHttpClientConnectionOperator, adding the ability to disable DNS resolution for specific connections. This is particularly useful when connecting to .onion addresses through a SOCKS proxy, where public DNS lookups are not possible or desirable.

[ok2c]

@arturobernalg Damn. I also started working on this one. Conceptually I am not in favor of adding a new config nob every time someone asks for it. We did that often in the days of HC 3 and did us little good. I would prefer this issue solved with a custom Function<HttpHost, InetSocketAddress[]> address resolver or by making DnsResolver able to return unresolved addresses.

[thc202]

It would be a lot more flexible if this could be configured through the context (that's what we did to this forked class).

[thc202]

Or not, it's not determining what the address is.

[thc202]

I would prefer this issue solved with a custom Function<HttpHost, InetSocketAddress[]> address resolver or by making DnsResolver able to return unresolved addresses.

I'd much be in favour of something like that.

[arturobernalg]

@arturobernalg Damn. I also started working on this one. Conceptually I am not in favor of adding a new config nob every time someone asks for it. We did that often in the days of HC 3 and did us little good. I would prefer this issue solved with a custom Function<HttpHost, InetSocketAddress[]> address resolver or by making DnsResolver able to return unresolved addresses.

Fair enough, @ok2c . I agree with your point about avoiding adding new configuration knobs unnecessarily. I've updated the implementation to use a Function. Please take another pass

[ok2c]

@arturobernalg My idea was to add this method to DnsResolver

    /**
     * Returns a list of {@link SocketAddress} for the given host with the given port.
     *
     * @see SocketAddress
     *
     * @since 5.5
     */
    default List<SocketAddress> resolve(String host, int port) throws UnknownHostException {
        InetAddress[] inetAddresses = resolve(host);
        if (inetAddresses == null) {
            return Collections.singletonList(InetSocketAddress.createUnresolved(host, port));
        }
        return Arrays.stream(inetAddresses)
                .map(e -> new InetSocketAddress(e, port))
                .collect(Collectors.toList());
    }

[arturobernalg]

@arturobernalg My idea was to add this method to DnsResolver

    /**
     * Returns a list of {@link SocketAddress} for the given host with the given port.
     *
     * @see SocketAddress
     *
     * @since 5.5
     */
    default List<SocketAddress> resolve(String host, int port) throws UnknownHostException {
        InetAddress[] inetAddresses = resolve(host);
        if (inetAddresses == null) {
            return Collections.singletonList(InetSocketAddress.createUnresolved(host, port));
        }
        return Arrays.stream(inetAddresses)
                .map(e -> new InetSocketAddress(e, port))
                .collect(Collectors.toList());
    }

What about now, @ok2c ? I believe I've made the correct changes this time. Please review.

[ok2c]

@arturobernalg Looks good to me, but similar change needs to be doe to MultihomeIOSessionRequester as well.

[ok2c]

@arturobernalg Can we avoid formatting changes in the same commit with functional ones? If you want to change formatting please do it through the style check or at least in a separate commit.

[arturobernalg]

Pelase @ok2c do another pass.

[ok2c]

@arturobernalg Almost there. Let's change this method to return List<InetSocketAddress> in order to avoid ugly downcasts in several places.

[arturobernalg]

done

[ok2c]

@arturobernalg I suppose this cast is no longer needed

[arturobernalg]

@ok2c I don't know what happened. I just removed.