HTTPCLIENT-2292: fix regression leading to invalid proxy

[mmoayyed]

This seems to have caused a regression in 5.2.2.

java.lang.IllegalArgumentException: Invalid Proxy
    at java.base/java.net.Socket.<init>(Socket.java:216)
    at org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory.createSocket(SSLConnectionSocketFactory.java:208)
    at org.apache.hc.client5.http.impl.io.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:158)
    at org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager.connect(PoolingHttpClientConnectionManager.java:447)
    at org.apache.hc.client5.http.impl.classic.InternalExecRuntime.connectEndpoint(InternalExecRuntime.java:162)
    at org.apache.hc.client5.http.impl.classic.InternalExecRuntime.connectEndpoint(InternalExecRuntime.java:172) 

When the proxy is undefined or set to null, DefaultHttpClientConnectionOperator does the following:

Timeout soTimeout = socketConfig.getSoTimeout();
SocketAddress socksProxyAddress = socketConfig.getSocksProxyAddress();
Proxy proxy = socksProxyAddress != null ? new Proxy(Type.SOCKS, socksProxyAddress) : null; 

The proxy object above is then null, which means the following call:

Socket sock = sf.createSocket(proxy, context); 

...will eventually pass down to SSLConnectionSocketFactory which does this:

public Socket createSocket(Proxy proxy, HttpContext context) throws IOException {
    return new Socket(proxy); //this proxy cannot be null
} 

I should mention that in the above case,

No SOCKS proxy is ever configured via the PoolingHttpClientConnectionManagerBuilder. setDefaultSocketConfig() method
No SocketConfig.setSocksProxyAddress(socksaddr) exists and is always null.

[ok2c]

Fixed by 6d60624

[mmoayyed]

Thank you!

[garydgregory]

FWIW, I ran into the same regression. Staying on 5.2.1 for now...
https://issues.apache.org/jira/browse/HTTPCLIENT-2292

[Sineaggi]

Unfortunately this is still causing issues for projects such as docker-java (see docker-java/docker-java#2290). Currently this bug manifests as a backwards-imcompatible change to projects inheriting from PlainConnectionSocketFactory and SSLConnectionSocketFactory.

I've made an example fix #535 to pull the proxy null check higher out