Check for null target in avoidProxy to prevent exceptions; Close #560. #561
Conversation
| @@ -44,5 +44,11 @@ public void testBasics() { | |||
| proxyServer = new ProxyServer("foo", 1234); | |||
| proxyServer.addNonProxyHost("*.somewhere.org"); | |||
| assertFalse(ProxyUtils.avoidProxy(proxyServer, req)); | |||
|
|
|||
| // shouldn't crash | |||
| req = new RequestBuilder("GET").setUrl("http:///badhost/foo").build(); | |||
There was a problem hiding this comment.
Why shouldn't this crash?
There was a problem hiding this comment.
The error is happening pretty deep in the guts of the code at the point where you already have the url and are trying to execute it (see partial stacktrace below). Ultimately, I'd be fine with dropping this change and test as long as the changes to AsyncHttpProviderUtils make it in, because then callers can check bad urls ahead of time. Would you honor the pull request if I dropped ProxyUtils*?
java.lang.NullPointerException
at com.ning.http.util.ProxyUtils.avoidProxy(ProxyUtils.java:101)
at com.ning.http.util.ProxyUtils.avoidProxy(ProxyUtils.java:86)
at com.ning.http.util.ProxyUtils.getProxyServer(ProxyUtils.java:72)
at
com.ning.http.client.providers.netty.NettyAsyncHttpProvider.doConnect(NettyAsyncHttpProvider.java:955)
at
com.ning.http.client.providers.netty.NettyAsyncHttpProvider.execute(NettyAsyncHttpProvider.java:937)
at
com.ning.http.client.AsyncHttpClient.executeRequest(AsyncHttpClient.java:520)
There was a problem hiding this comment.
From my pov, this should crash as soon as possible, meaning in setUrl where the illegal url is passed.
No description provided.