Jest should use default proxy settings

[davidconnard]

The default implementations of HttpClient and HttpAsynClient do not bother to check the system proxy settings - ie. http.proxyHost and https.proxyHost.

Jest may be used in environments where a proxy connection is required.

An approach that worked for me in my environment (where http.proxyHost / https.proxyHost are all fully set and required to be used) is below. However, I'm not certain that that'd be the approach you want to use (eg. vs. setting some manual proxy configurations, and I'm not certain that the implementation details of ProxySelectorAsyncRoutePlanner are totally correct)... so, I haven't raised a pull request for this change

Change HttpClient as follows, around line 72 of JestClientFactory.java:

    // add proxy-aware route planner
    ProxySelectorRoutePlanner routePlanner = new ProxySelectorRoutePlanner(
            httpclient.getConnectionManager().getSchemeRegistry(),
            ProxySelector.getDefault());
    httpclient.setRoutePlanner(routePlanner);

    client.setHttpClient(httpclient);

(where client is changed to be a DefaultHttpClient object, not a HttpClient object).

However, this solution does not seem to work for the HttpAsyncClient. I was able to make the HttpAsyncClient work via the following code around line 80 of JestClientFactory:

        // add proxy-aware async route planner
        ProxySelectorAsyncRoutePlanner aSyncRoutePlanner = new ProxySelectorAsyncRoutePlanner(
                asyncClient.getConnectionManager().getSchemeRegistry(),
                ProxySelector.getDefault());
        asyncClient.setRoutePlanner(aSyncRoutePlanner);

where ProxySelectorAsyncRoutePlanner is a class copied from:

https://github.com/markkolich/kolich-httpclient4-closure/blob/master/src/main/java/com/kolich/http/async/routing/ProxySelectorAsyncRoutePlanner.java

(this represents a wrapper library to HttpClient / HttpAsyncClient which allegedly improves on a few things... proxy support by default being one of them).

Perhaps you just want to depend on that Kolich HttpClient library? Perhaps you want to adopt the above implementation? Perhaps there is a different solution.

[kramer]

Here is a very simple way of implementing manual proxy configuration. It seems to work for both sync and async client modes, though it still does not respect the environment set proxy variables (maybe we can add it to constructor via System.getProperty() calls...). What are your thoughts?

https://gist.github.com/kramer/38f5dc7a8a313bef16b4

[davidconnard]

Agreed, that that is a simple functional solution. It would be useful to have it pick up the default proxy settings, but it's not the end of the world if explicit configuration is required... as long as it's clear on the config methods that explicit pass-through of default proxy settings is required.

Note that this solution doesn't provide the ability to specify complex proxy settings - eg. https.proxyHost vs. http.proxyHost (eg. you may need different proxy settings for https), nor would it support http.nonProxyHosts (eg. you could conceivably want to talk to one local node directly and one remote node behind a proxy). None of these proxy complexities are required in our environment, so I don't particularly care, but using the ProxySelector.getDefault() concept would hide all this complexity from your code, and lets the usual system properties & proxy handling take care of it all.

Offhand, I'm surprised that the HttpClient & HttpAsyncClient do not already work in this way. But there you go.

Not immediately sure why you think that patch would work for async modes, as you haven't set the property on the async client? Your async test actually seems to call the synchronous execute method, so it doesn't seem to be testing the async client in any way. I haven't had a chance to run your patched code with our proxy environment... the alternate code that I suggested is about to be put to into use however.

[kramer]

Ah ok, digging deeper into RoutePlanner, I now see what you mean 😸 Using ProxySelectorRoutePlanner indeed seems to be the most compatible/painless solution for the sync client, though I'm not sure about what to do for the async client. I'll look around too see any other alternative solutions, in the mean time feel free to provide any additional recommendations and comments.

I wonder if @ferhatsb has anything to add regarding this issue...?

[davidconnard]

I wouldn't rush to any one solution, when it's not immediately clear what the "correct" implementation for an async client (with default settings) should be. Maybe let this issue sit for a while and see who else has anything to add?

For what it's worth, the code solution I outlined has just gone into our live environment, and it seems to have performed fine under heavy load. It still doesn't seem quite "right" however, having to add the ProxySelectorAsyncRoutePlanner class. Logically, this class ought to be part of the HttpAsyncClient library.

Perhaps the simplest solution could be to just depend on the Kolich HttpClient library, and leave it as someone else's problem...! :) Or perhaps one of us should contribute this class (and indeed, these overall changes, to make ProxySelector.getDefault() the default behaviour) back to the HttpClient codebase.

[ferhatsb]

I have started to think about to use Jest for Android as well. Actually we want to add marketing stuff for mobile devices. Whatever while deciding a way, we should start to think to make jest as light as possible with all required/necessary features.

[kramer]

@davidconnard it's been a while sorry, but I think we finally have the technology to do this 😆
Any comments on the commit 4b8300b ?

[davidconnard]

Thanks, looks like it should do the job... :) Note that I'm not in a position to test this at the moment (due to not yet having moved up to Elastic 1.0).

[kramer]

Closing as internal tests show this is resolved now, please re-open otherwise.