Sophisticate proxy parsing and generating, fix misinterpretation if SOCKS proxies

This is a quick and dirty fix; it seems a major redesign is appropriate.
We should also switch to some maintained version of proxy-vole from github.
This one looks halfway active:
https://github.com/MarkusBernhardt/proxy-vole
or maybe one of its forks...

11 files changed, 173 insertions, 56 deletions

diff --git a/pom.xml b/pom.xml
index b77cdb4..3f39806 100644
--- a/pom.xml
+++ b/pom.xml
@@ -3,7 +3,7 @@
 
   <groupId>org.openslx</groupId>
   <artifactId>proxy_vole</artifactId>
-  <version>0.0.3-SNAPSHOT</version>
+  <version>0.0.4-SNAPSHOT</version>
   <packaging>jar</packaging>
 
   <name>Proxy Vole</name>
diff --git a/src/main/java/com/btr/proxy/search/browser/firefox/FirefoxProxySearchStrategy.java b/src/main/java/com/btr/proxy/search/browser/firefox/FirefoxProxySearchStrategy.java
index de576e4..228bc8c 100644
--- a/src/main/java/com/btr/proxy/search/browser/firefox/FirefoxProxySearchStrategy.java
+++ b/src/main/java/com/btr/proxy/search/browser/firefox/FirefoxProxySearchStrategy.java
@@ -260,8 +260,15 @@ public class FirefoxProxySearchStrategy implements ProxySearchStrategy {
 		String proxyHost = settings.getProperty("network.proxy."+protocol, null);
 		int proxyPort = Integer.parseInt(settings.getProperty("network.proxy."+protocol+"_port", "0"));
 		if (proxyHost != null) {
+			Proxy.Type type;
+			if (protocol.startsWith("socks")) {
+				type = Proxy.Type.SOCKS;
+				protocol = "socket";
+			} else {
+				type = Proxy.Type.HTTP;
+			}
 			Logger.log(getClass(), LogLevel.TRACE, "Firefox "+protocol+" proxy is {0}:{1}", proxyHost, proxyPort);
-			ps.setSelector(protocol, ProxyUtil.parseProxySettings(proxyHost, Proxy.Type.HTTP, proxyPort));
+			ps.setSelector(protocol, ProxyUtil.parseProxySettings(proxyHost, type, proxyPort));
 		}
 	}
 	
diff --git a/src/main/java/com/btr/proxy/search/browser/ie/IEProxySearchStrategy.java b/src/main/java/com/btr/proxy/search/browser/ie/IEProxySearchStrategy.java
index 6641aff..dd062cd 100644
--- a/src/main/java/com/btr/proxy/search/browser/ie/IEProxySearchStrategy.java
+++ b/src/main/java/com/btr/proxy/search/browser/ie/IEProxySearchStrategy.java
@@ -165,7 +165,15 @@ public class IEProxySearchStrategy implements ProxySearchStrategy {
 	private void addFallbackSelector(Properties settings, ProtocolDispatchSelector ps) {
 		String proxy = settings.getProperty("default");
 		if (proxy != null) {
-			ps.setFallbackSelector(ProxyUtil.parseProxySettings(proxy));
+			Proxy defaultProxy = ProxyUtil.parseProxyString(proxy, Proxy.Type.HTTP, 0);
+			if (defaultProxy.type() == Proxy.Type.SOCKS) {
+				ps.setSelector("socket", new FixedProxySelector(defaultProxy));
+				return;
+			}
+			FixedProxySelector fp = new FixedProxySelector(defaultProxy);
+			ps.setSelector("http", fp);
+			ps.setSelector("https", fp);
+			ps.setSelector("ftp", fp);
 		}
 	}
 
@@ -180,8 +188,14 @@ public class IEProxySearchStrategy implements ProxySearchStrategy {
 	private void addSelectorForProtocol(Properties settings, String protocol, ProtocolDispatchSelector ps) {
 		String proxy = settings.getProperty(protocol);
 		if (proxy != null) {
-			Proxy.Type fb = protocol.startsWith("socks") ? Proxy.Type.SOCKS : Proxy.Type.HTTP;
-			ProxySelector protocolSelector = ProxyUtil.parseProxySettings(proxy, fb, 0);
+			Proxy.Type type;
+			if (protocol.startsWith("socks")) {
+				type = Proxy.Type.SOCKS;
+				protocol = "socket";
+			} else {
+				type = Proxy.Type.HTTP;
+			}
+			ProxySelector protocolSelector = ProxyUtil.parseProxySettings(proxy, type, 0);
 			ps.setSelector(protocol, protocolSelector);
 		}
 	}
diff --git a/src/main/java/com/btr/proxy/search/env/EnvProxySearchStrategy.java b/src/main/java/com/btr/proxy/search/env/EnvProxySearchStrategy.java
index 37bd0a1..f7b6483 100644
--- a/src/main/java/com/btr/proxy/search/env/EnvProxySearchStrategy.java
+++ b/src/main/java/com/btr/proxy/search/env/EnvProxySearchStrategy.java
@@ -4,11 +4,12 @@ import java.net.ProxySelector;
 import java.util.Properties;
 
 import com.btr.proxy.search.ProxySearchStrategy;
+import com.btr.proxy.selector.fixed.FixedProxySelector;
 import com.btr.proxy.selector.misc.ProtocolDispatchSelector;
 import com.btr.proxy.selector.whitelist.ProxyBypassListSelector;
 import com.btr.proxy.util.Logger;
-import com.btr.proxy.util.ProxyUtil;
 import com.btr.proxy.util.Logger.LogLevel;
+import com.btr.proxy.util.ProxyUtil;
 
 /*****************************************************************************
  * Reads some environment variables and extracts the proxy settings from them.
@@ -96,26 +97,30 @@ public class EnvProxySearchStrategy implements ProxySearchStrategy {
 	public ProxySelector getProxySelector() {
 		
 		Logger.log(getClass(), LogLevel.TRACE, "Inspecting environment variables.");
+		ProtocolDispatchSelector ps = new ProtocolDispatchSelector();
 
-		// Check if http_proxy var is set.
-		ProxySelector httpPS = ProxyUtil.parseProxySettings(this.httpProxy);
-		if (httpPS == null) {
-			return null;
+		FixedProxySelector httpPS = ProxyUtil.parseProxySettings(this.httpProxy);
+		if (httpPS != null) {
+			Logger.log(getClass(), LogLevel.TRACE, "Http Proxy is {0}", this.httpProxy);
+			ps.setSelector("http", httpPS);
 		}
-		
-		Logger.log(getClass(), LogLevel.TRACE, "Http Proxy is {0}", this.httpProxy);
-		ProtocolDispatchSelector ps = new ProtocolDispatchSelector();
-		ps.setSelector("http", httpPS);
 
-		ProxySelector httpsPS = ProxyUtil.parseProxySettings(this.httpsProxy);
-		Logger.log(getClass(), LogLevel.TRACE, "Https Proxy is {0}", httpsPS == null? this.httpsProxy: httpsPS);
-		ps.setSelector("https", httpsPS != null? httpsPS: httpPS);
+		FixedProxySelector httpsPS = ProxyUtil.parseProxySettings(this.httpsProxy);
+		if (httpsPS != null) {
+			Logger.log(getClass(), LogLevel.TRACE, "Https Proxy is {0}", this.httpsProxy);
+			ps.setSelector("https", httpsPS);
+		}
 
-		ProxySelector ftpPS = ProxyUtil.parseProxySettings(this.ftpProxy);
+		FixedProxySelector ftpPS = ProxyUtil.parseProxySettings(this.ftpProxy);
 		if (ftpPS != null) {
 			Logger.log(getClass(), LogLevel.TRACE, "Ftp Proxy is {0}", this.ftpProxy);
 			ps.setSelector("ftp", ftpPS);			
 		}
+		
+		ps.setFallbackSocksSelector(httpPS, httpsPS, ftpPS);
+		
+		if (ps.isEmpty())
+			return null;
 
 		// Wrap with white list support
 		ProxySelector result = ps;
diff --git a/src/main/java/com/btr/proxy/search/wpad/WpadProxySearchStrategy.java b/src/main/java/com/btr/proxy/search/wpad/WpadProxySearchStrategy.java
index e559d62..0d3e6dc 100644
--- a/src/main/java/com/btr/proxy/search/wpad/WpadProxySearchStrategy.java
+++ b/src/main/java/com/btr/proxy/search/wpad/WpadProxySearchStrategy.java
@@ -119,11 +119,8 @@ public class WpadProxySearchStrategy implements ProxySearchStrategy {
 				return ret;
 		}
 
-		BufferedReader br = null;
 		String[] addresses = null;
-		try {
-			FileReader fr = new FileReader("/etc/resolv.conf");
-			br = new BufferedReader(fr);
+		try (FileReader fr = new FileReader("/etc/resolv.conf"); BufferedReader br = new BufferedReader(fr)) {
 			String input;
 			while ((input = br.readLine()) != null) {
 				if (input.startsWith("search")) {
@@ -134,11 +131,6 @@ public class WpadProxySearchStrategy implements ProxySearchStrategy {
 			}
 		} catch (IOException e1) {
 			Logger.log(getClass(), LogLevel.DEBUG, "Could not read resolv.conf");
-		} finally {
-			try {
-				br.close();
-			} catch (Exception e) {
-			}
 		}
 
 		if (addresses == null) {
diff --git a/src/main/java/com/btr/proxy/selector/fixed/FixedProxySelector.java b/src/main/java/com/btr/proxy/selector/fixed/FixedProxySelector.java
index 2de95b5..79e0866 100644
--- a/src/main/java/com/btr/proxy/selector/fixed/FixedProxySelector.java
+++ b/src/main/java/com/btr/proxy/selector/fixed/FixedProxySelector.java
@@ -3,10 +3,12 @@ package com.btr.proxy.selector.fixed;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
+import java.net.Proxy.Type;
 import java.net.ProxySelector;
 import java.net.SocketAddress;
 import java.net.URI;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -19,7 +21,7 @@ import java.util.List;
 
 public class FixedProxySelector extends ProxySelector {
 	
-	private final List<Proxy> proxyList;
+	private List<Proxy> proxyList;
 	
 	
 	/*************************************************************************
@@ -27,12 +29,13 @@ public class FixedProxySelector extends ProxySelector {
 	 * @param proxy the proxy to use.
 	 ************************************************************************/
 	
-	public FixedProxySelector(Proxy proxy) {
+	public FixedProxySelector(Proxy... proxy) {
 		super();
-
-		List<Proxy> list = new ArrayList<Proxy>(1);
-		list.add(proxy);
-		this.proxyList = Collections.unmodifiableList(list);
+		if (proxy.length == 0)
+			throw new IllegalArgumentException("Empty list was passed");
+		this.proxyList = Collections.unmodifiableList(Arrays.asList(proxy));
+		if (this.proxyList.contains(null))
+			throw new NullPointerException("List conains NULL proxy");
 	}
 	
 	/*************************************************************************
@@ -53,7 +56,24 @@ public class FixedProxySelector extends ProxySelector {
 
 	@Override
 	public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
-		// Not used
+		final List<Proxy> list = this.proxyList;
+		if (list.size() < 2)
+			return;
+		if (!list.get(0).address().equals(sa))
+			return; // We only care if the failing one is the first one in list
+		List<Proxy> newList = new ArrayList<>(list.size());
+		Proxy broken = null;
+		for (Proxy p : list) {
+			if (p.address().equals(sa)) {
+				broken = p;
+			} else {
+				newList.add(p);
+			}
+		}
+		if (broken != null) {
+			newList.add(broken);
+		}
+		this.proxyList = Collections.unmodifiableList(newList);
 	}
 
 	/*************************************************************************
@@ -66,4 +86,8 @@ public class FixedProxySelector extends ProxySelector {
 		return this.proxyList;
 	}
 
+	public Type getType() {
+		return this.proxyList.get(0).type();
+	}
+
 }
diff --git a/src/main/java/com/btr/proxy/selector/misc/ProtocolDispatchSelector.java b/src/main/java/com/btr/proxy/selector/misc/ProtocolDispatchSelector.java
index 02ecc44..bb57452 100644
--- a/src/main/java/com/btr/proxy/selector/misc/ProtocolDispatchSelector.java
+++ b/src/main/java/com/btr/proxy/selector/misc/ProtocolDispatchSelector.java
@@ -10,6 +10,9 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import com.btr.proxy.selector.direct.NoProxySelector;
+import com.btr.proxy.selector.fixed.FixedProxySelector;
+import com.btr.proxy.util.Logger;
+import com.btr.proxy.util.Logger.LogLevel;
 
 /*****************************************************************************
  * This is a facade for a list of ProxySelecor objects. You can register 
@@ -46,6 +49,9 @@ public class ProtocolDispatchSelector extends ProxySelector {
 		if (selector == null) {
 			throw new NullPointerException("Selector must not be null.");
 		}
+		if (protocol.toLowerCase().startsWith("socks")) {
+			protocol = "socket";
+		}
 		this.selectors.put(protocol, selector);
 	}
 	
@@ -82,6 +88,17 @@ public class ProtocolDispatchSelector extends ProxySelector {
 		this.fallbackSelector = selector;
 	}
 	
+	private ProxySelector selectorForUri(URI uri) {
+		if (uri == null || uri.getScheme() == null)
+			return this.fallbackSelector;
+		String protocol = uri.getScheme();
+		ProxySelector selector = this.selectors.get(protocol);
+		if (selector == null) {
+			selector = this.fallbackSelector;
+		}
+		return selector;
+	}
+	
 	/*************************************************************************
 	 * connectFailed
 	 * @see java.net.ProxySelector#connectFailed(java.net.URI, java.net.SocketAddress, java.io.IOException)
@@ -89,11 +106,7 @@ public class ProtocolDispatchSelector extends ProxySelector {
 
 	@Override
 	public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
-		ProxySelector selector = this.fallbackSelector;
-		String protocol = uri.getScheme();
-		if (protocol != null && this.selectors.get(protocol) != null) {
-			selector = this.selectors.get(protocol);
-		}
+		ProxySelector selector = selectorForUri(uri);
 		selector.connectFailed(uri, sa, ioe);
 	}
 
@@ -104,19 +117,25 @@ public class ProtocolDispatchSelector extends ProxySelector {
 
 	@Override
 	public List<Proxy> select(URI uri) {
-		ProxySelector selector = null;
-		String protocol = uri.getScheme();
-		if (protocol != null && this.selectors.get(protocol) != null) {
-			selector = this.selectors.get(protocol);
-		}
-		if (selector == null && this.selectors.get("socks") != null) {
-			// Socks should always work
-			selector = this.selectors.get("socks");
-		}
-		if (selector == null) {
-			selector = this.fallbackSelector;
+		ProxySelector selector = selectorForUri(uri);
+		List<Proxy> ret = selector.select(uri);
+		Logger.log(getClass(), LogLevel.TRACE, "Selector {0} for {1} -> {2}", selector.getClass(), uri, ret);
+		return ret;
+	}
+
+	public void setFallbackSocksSelector(FixedProxySelector... pslist) {
+		if (!(this.fallbackSelector instanceof NoProxySelector))
+			return;
+		for (FixedProxySelector ps : pslist) {
+			if (ps != null && ps.getType() == Proxy.Type.SOCKS) {
+				this.fallbackSelector = ps;
+				return;
+			}
 		}
-		return selector.select(uri);
+	}
+
+	public boolean isEmpty() {
+		return this.selectors.isEmpty() && this.fallbackSelector instanceof NoProxySelector;
 	}
 
 }
diff --git a/src/main/java/com/btr/proxy/selector/misc/ProxyListFallbackSelector.java b/src/main/java/com/btr/proxy/selector/misc/ProxyListFallbackSelector.java
index 41859ec..e501952 100644
--- a/src/main/java/com/btr/proxy/selector/misc/ProxyListFallbackSelector.java
+++ b/src/main/java/com/btr/proxy/selector/misc/ProxyListFallbackSelector.java
@@ -11,6 +11,8 @@ import java.util.List;
 import java.util.Map.Entry;
 import java.util.concurrent.ConcurrentHashMap;
 
+import com.btr.proxy.util.ProxyUtil;
+
 /*****************************************************************************
  * Implements a fallback selector to warp it around an existing ProxySelector.
  * This will remove proxies from a list of proxies and implement an automatic
@@ -58,6 +60,7 @@ public class ProxyListFallbackSelector extends ProxySelector {
 	@Override
 	public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
 		this.failedDelayCache.put(sa, System.currentTimeMillis());
+		this.delegate.connectFailed(uri, sa, ioe);
 	}
 
 	/*************************************************************************
@@ -70,6 +73,9 @@ public class ProxyListFallbackSelector extends ProxySelector {
 		cleanupCache();
 		List<Proxy> proxyList = this.delegate.select(uri);
 		List<Proxy> result = filterUnresponsiveProxiesFromList(proxyList);
+		if (result.isEmpty()) {
+			return ProxyUtil.noProxyList();
+		}
 		return result;
 	}
 
diff --git a/src/main/java/com/btr/proxy/util/ProxyUtil.java b/src/main/java/com/btr/proxy/util/ProxyUtil.java
index 97f2af2..bfb3f70 100644
--- a/src/main/java/com/btr/proxy/util/ProxyUtil.java
+++ b/src/main/java/com/btr/proxy/util/ProxyUtil.java
@@ -1,7 +1,7 @@
 package com.btr.proxy.util;
 
+import java.net.InetSocketAddress;
 import java.net.Proxy;
-import java.net.ProxySelector;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -34,11 +34,18 @@ public class ProxyUtil {
 	 * @return a FixedProxySelector using this settings, null on parse error.
 	 ************************************************************************/
 
-	public static ProxySelector parseProxySettings(String proxyVar) {
+	public static FixedProxySelector parseProxySettings(String proxyVar) {
 		return parseProxySettings(proxyVar, Proxy.Type.HTTP, 0);
 	}
 	
-	public static ProxySelector parseProxySettings(String proxyVar, Proxy.Type fallback, int fallbackPort) {
+	public static FixedProxySelector parseProxySettings(String proxyVar, Proxy.Type fallback, int fallbackPort) {
+		Proxy proxy = parseProxyString(proxyVar, fallback, fallbackPort);
+		if (proxy == null)
+			return null;
+		return new FixedProxySelector(proxy);
+	}
+	
+	public static Proxy parseProxyString(String proxyVar, Proxy.Type fallback, int fallbackPort) {
 		if (proxyVar == null || proxyVar.trim().length() == 0) {
 			return null;
 		}
@@ -84,7 +91,7 @@ public class ProxyUtil {
 				port = DEFAULT_SOCKS_PROXY_PORT;
 			}
 		}
-		return new FixedProxySelector(type, host.trim(), port);
+		return new Proxy(type, InetSocketAddress.createUnresolved(host.trim(), port));
 	}
 
 	/*************************************************************************
diff --git a/src/test/java/com/btr/proxy/search/java/JavaProxySearchTest.java b/src/test/java/com/btr/proxy/search/java/JavaProxySearchTest.java
index cfc473a..069cda3 100644
--- a/src/test/java/com/btr/proxy/search/java/JavaProxySearchTest.java
+++ b/src/test/java/com/btr/proxy/search/java/JavaProxySearchTest.java
@@ -129,7 +129,7 @@ public class JavaProxySearchTest {
 	 ************************************************************************/
 	@Test
 	public void testSOCKS() {
-		List<Proxy> result = this.selector.select(TestUtil.SOCKS_TEST_URI);
+		List<Proxy> result = this.selector.select(TestUtil.SOCKET_TEST_URI);
 		assertEquals(TestUtil.SOCKS_TEST_PROXY, result.get(0));
 	}
 	
diff --git a/src/test/java/com/btr/proxy/util/ProxyUtilTest.java b/src/test/java/com/btr/proxy/util/ProxyUtilTest.java
index 5634600..ffcf03e 100644
--- a/src/test/java/com/btr/proxy/util/ProxyUtilTest.java
+++ b/src/test/java/com/btr/proxy/util/ProxyUtilTest.java
@@ -73,6 +73,49 @@ public class ProxyUtilTest {
 		assertEquals("HTTP @ 192.123.123.1:8080", psList.get(0).toString());
 	}
 
+	/*************************************************************************
+	 * Test parsing method.
+	 ************************************************************************/
+	
+	@Test
+	public void testParseProxySettings6() {
+		ProxySelector rs = ProxyUtil.parseProxySettings("192.123.123.1:8080",Proxy.Type.SOCKS, 1234);
+		List<Proxy> psList = rs.select(TestUtil.HTTP_TEST_URI);
+		assertEquals("SOCKS @ 192.123.123.1:8080", psList.get(0).toString());
+	}
+
+	/*************************************************************************
+	 * Test parsing method.
+	 ************************************************************************/
+	
+	@Test
+	public void testParseProxySettings7() {
+		ProxySelector rs = ProxyUtil.parseProxySettings("192.123.123.1", Proxy.Type.SOCKS, 1234);
+		List<Proxy> psList = rs.select(TestUtil.HTTP_TEST_URI);
+		assertEquals("SOCKS @ 192.123.123.1:1234", psList.get(0).toString());
+	}
+
+	/*************************************************************************
+	 * Test parsing method.
+	 ************************************************************************/
+	
+	@Test
+	public void testParseProxySettings8() {
+		ProxySelector rs = ProxyUtil.parseProxySettings("socks://192.123.123.1", Proxy.Type.HTTP, 1234);
+		List<Proxy> psList = rs.select(TestUtil.HTTP_TEST_URI);
+		assertEquals("SOCKS @ 192.123.123.1:1234", psList.get(0).toString());
+	}
+	
+	/*************************************************************************
+	 * Test parsing method.
+	 ************************************************************************/
+	
+	@Test
+	public void testParseProxySettings9() {
+		ProxySelector rs = ProxyUtil.parseProxySettings("socks://http_proxy.unit-test.invalid/");
+		List<Proxy> psList = rs.select(TestUtil.HTTP_TEST_URI);
+		assertEquals("SOCKS @ http_proxy.unit-test.invalid:1080", psList.get(0).toString());
+	}
 
 }