Move some proxy logic to BrowserAwareProxySelector

The logic for mangling URIs to be compatible with the browser seems
more appropriate to keep in the ProxySelector rather than the catch-all
PluginAppletViewer.

5 files changed, 94 insertions, 69 deletions

diff --git a/ChangeLog b/ChangeLog
index 0a364c0..ba7d0d3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2013-10-09  Omair Majid  <[email protected]>
+
+	* plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
+	(requestPluginProxyInfo): Accept a String instead of URI.
+	(convertUriSchemeForProxyQuery): Move to ...
+	* plugin/icedteanp/java/sun/applet/PluginProxySelector.java
+	(convertUriSchemeForProxyQuery): Here.
+	(getFromBrowser): Call convertUriSchemeForProxyQuery.
+	* tests/netx/unit/sun/applet/PluginAppletViewerTest.java
+	(testConvertUriSchemeForProxyQuery),
+	(assertQueryForBrowserProxyUsesHttpFallback),
+	(assertQueryForBrowserProxyContainsNoDoubleSlashes),
+	(assertQueryForBrowserProxyDoesNotChangeQuery): Move to ...
+	* tests/netx/unit/sun/applet/PluginProxySelectorTest.java: Here.
+
 2013-10-07  Andrew Azores  <[email protected]>
 
 	DeploymentConfiguration properties reproducer fix
diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
index bd4c0d5..b1abf19 100644
--- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
+++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
@@ -83,10 +83,8 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.InvocationTargetException;
-import java.net.MalformedURLException;
 import java.net.SocketPermission;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.security.AccessController;
@@ -1238,22 +1236,18 @@ public class PluginAppletViewer extends XEmbeddedFrame
         return request.getObject();
     }
 
-    public static Object requestPluginProxyInfo(URI uri) {
-
-        String requestURI = null;
+    /**
+     * Obtain information about the proxy from the browser.
+     *
+     * @param uri a String in url-encoded form
+     * @return a {@link URI} that indicates a proxy.
+     */
+    public static Object requestPluginProxyInfo(String uri) {
         Long reference = getRequestIdentifier();
 
-        try {
-            requestURI = convertUriSchemeForProxyQuery(uri);
-        } catch (Exception e) {
-            PluginDebug.debug("Cannot construct URL from ", uri.toString(), " ... falling back to DIRECT proxy");
-            OutputController.getLogger().log(OutputController.Level.ERROR_ALL,e);
-            return null;
-        }
-
         PluginCallRequest request = requestFactory.getPluginCallRequest("proxyinfo",
                 "plugin PluginProxyInfo reference " + reference + " " +
-                        requestURI, reference);
+                        uri, reference);
 
         PluginMessageConsumer.registerPriorityWait(reference);
         streamhandler.postCallRequest(request);
@@ -1274,21 +1268,6 @@ public class PluginAppletViewer extends XEmbeddedFrame
         return request.getObject();
     }
 
-    public static String convertUriSchemeForProxyQuery(URI uri) throws URISyntaxException, UnsupportedEncodingException {
-        // there is no easy way to get SOCKS proxy info. So, we tell mozilla that we want proxy for
-        // an HTTP uri in case of non http/ftp protocols. If we get back a SOCKS proxy, we can
-        // use that, if we get back an http proxy, we fallback to DIRECT connect
-
-        String scheme = uri.getScheme();
-        if (!scheme.startsWith("http") && !scheme.equals("ftp")) {
-            scheme = "http";
-        }
-
-        URI result = new URI(scheme, uri.getUserInfo(), uri.getHost(), uri.getPort(),
-                uri.getPath(), uri.getQuery(), uri.getFragment());
-        return UrlUtil.encode(result.toString(), "UTF-8");
-    }
-
     public static void JavaScriptFinalize(long internal) {
         Long reference = getRequestIdentifier();
 
diff --git a/plugin/icedteanp/java/sun/applet/PluginProxySelector.java b/plugin/icedteanp/java/sun/applet/PluginProxySelector.java
index c863fb9..cb4f58d 100644
--- a/plugin/icedteanp/java/sun/applet/PluginProxySelector.java
+++ b/plugin/icedteanp/java/sun/applet/PluginProxySelector.java
@@ -37,12 +37,16 @@ exception statement from your version. */
 
 package sun.applet;
 
+import java.io.UnsupportedEncodingException;
 import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 
+import com.sun.jndi.toolkit.url.UrlUtil;
+
 import net.sourceforge.jnlp.config.DeploymentConfiguration;
 import net.sourceforge.jnlp.runtime.JNLPProxySelector;
 import net.sourceforge.jnlp.util.logging.OutputController;
@@ -84,8 +88,19 @@ public class PluginProxySelector extends JNLPProxySelector {
         }
 
         // Nothing usable in cache. Fetch info from browser
+
+        String requestURI;
+        try {
+            requestURI = convertUriSchemeForProxyQuery(uri);
+        } catch (Exception e) {
+            PluginDebug.debug("Cannot construct URL from ", uri.toString(), " ... falling back to DIRECT proxy");
+            OutputController.getLogger().log(OutputController.Level.ERROR_ALL,e);
+            proxyList.add(Proxy.NO_PROXY);
+            return proxyList;
+        }
+
         Proxy proxy = Proxy.NO_PROXY;
-        Object o = getProxyFromRemoteCallToBrowser(uri);
+        Object o = getProxyFromRemoteCallToBrowser(requestURI);
 
         // If the browser returned anything, try to parse it. If anything in the try block fails, the fallback is direct connection
         try {
@@ -119,7 +134,7 @@ public class PluginProxySelector extends JNLPProxySelector {
     }
 
     /** For tests to override */
-    protected Object getProxyFromRemoteCallToBrowser(URI uri) {
+    protected Object getProxyFromRemoteCallToBrowser(String uri) {
         return PluginAppletViewer.requestPluginProxyInfo(uri);
     }
 
@@ -139,4 +154,18 @@ public class PluginProxySelector extends JNLPProxySelector {
         return null;
     }
 
+    public static String convertUriSchemeForProxyQuery(URI uri) throws URISyntaxException, UnsupportedEncodingException {
+        // there is no easy way to get SOCKS proxy info. So, we tell mozilla that we want proxy for
+        // an HTTP uri in case of non http/ftp protocols. If we get back a SOCKS proxy, we can
+        // use that, if we get back an http proxy, we fallback to DIRECT connect
+
+        String scheme = uri.getScheme();
+        if (!scheme.startsWith("http") && !scheme.equals("ftp")) {
+            scheme = "http";
+        }
+
+        URI result = new URI(scheme, uri.getUserInfo(), uri.getHost(), uri.getPort(),
+                uri.getPath(), uri.getQuery(), uri.getFragment());
+        return UrlUtil.encode(result.toString(), "UTF-8");
+    }
 }
diff --git a/tests/netx/unit/sun/applet/PluginAppletViewerTest.java b/tests/netx/unit/sun/applet/PluginAppletViewerTest.java
index 69e5cb1..1bf7e35 100644
--- a/tests/netx/unit/sun/applet/PluginAppletViewerTest.java
+++ b/tests/netx/unit/sun/applet/PluginAppletViewerTest.java
@@ -159,43 +159,6 @@ public class PluginAppletViewerTest {
         assertEquals(expectedReturn, call.join());
     }
 
-    @Test
-    public void testConvertUriSchemeForProxyQuery() throws Exception {
-        URI[] testUris = {
-                new URI("http", "foo.com", "/bar", null),
-                new URI("https", "foo.com", "/bar", null),
-                new URI("ftp", "foo.com", "/app/res/pub/channel.jar?i=1234", null),
-                new URI("socket", "foo.co.uk", "/bar/pub/ale.jar", null),
-        };
-
-        for (URI uri : testUris) {
-            URI result = new URI(PluginAppletViewer.convertUriSchemeForProxyQuery(uri));
-            assertQueryForBrowserProxyUsesHttpFallback(uri, result);
-            String hierarchicalPath = result.getAuthority() + result.getPath();
-            assertQueryForBrowserProxyContainsNoDoubleSlashes(hierarchicalPath);
-            assertQueryForBrowserProxyDoesNotChangeQuery(uri, result);
-        }
-    }
-
-    // Test that only HTTP is used as fallback scheme if a protocol other than HTTP(S) or FTP is specified
-    public void assertQueryForBrowserProxyUsesHttpFallback(URI expected, URI result) {
-        if (expected.getScheme().equals("ftp") || expected.getScheme().startsWith("http")) {
-            Assert.assertEquals(expected.getScheme(), result.getScheme());
-        } else {
-            Assert.assertEquals(result.getScheme(), "http");
-        }
-    }
-
-    // Test that absolute resource paths do not result in double-slashes within the URI
-    public void assertQueryForBrowserProxyContainsNoDoubleSlashes(String uri) {
-        Assert.assertFalse(uri.contains("//"));
-    }
-
-    // Test that the query string of the URI is not changed
-    public void assertQueryForBrowserProxyDoesNotChangeQuery(URI expected, URI result) {
-        Assert.assertEquals(expected.getQuery(), result.getQuery());
-    }
-
    /**************************************************************************
     *                          Test utilities                                *
     **************************************************************************/
diff --git a/tests/netx/unit/sun/applet/PluginProxySelectorTest.java b/tests/netx/unit/sun/applet/PluginProxySelectorTest.java
index 322a798..a7321f6 100644
--- a/tests/netx/unit/sun/applet/PluginProxySelectorTest.java
+++ b/tests/netx/unit/sun/applet/PluginProxySelectorTest.java
@@ -38,6 +38,7 @@ exception statement from your version. */
 package sun.applet;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 
 import java.net.InetSocketAddress;
@@ -65,7 +66,7 @@ public class PluginProxySelectorTest {
         }
 
         @Override
-        protected Object getProxyFromRemoteCallToBrowser(URI uri) {
+        protected Object getProxyFromRemoteCallToBrowser(String uri) {
             remoteCallCount++;
             return browserResponse;
         }
@@ -192,4 +193,42 @@ public class PluginProxySelectorTest {
         assertEquals(2, proxySelector.getRemoteCallCount());
     }
 
+
+    @Test
+    public void testConvertUriSchemeForProxyQuery() throws Exception {
+        URI[] testUris = {
+                new URI("http", "foo.com", "/bar", null),
+                new URI("https", "foo.com", "/bar", null),
+                new URI("ftp", "foo.com", "/app/res/pub/channel.jar?i=1234", null),
+                new URI("socket", "foo.co.uk", "/bar/pub/ale.jar", null),
+        };
+
+        for (URI uri : testUris) {
+            URI result = new URI(PluginProxySelector.convertUriSchemeForProxyQuery(uri));
+            assertQueryForBrowserProxyUsesHttpFallback(uri, result);
+            String hierarchicalPath = result.getAuthority() + result.getPath();
+            assertQueryForBrowserProxyContainsNoDoubleSlashes(hierarchicalPath);
+            assertQueryForBrowserProxyDoesNotChangeQuery(uri, result);
+        }
+    }
+
+    // Test that only HTTP is used as fallback scheme if a protocol other than HTTP(S) or FTP is specified
+    public void assertQueryForBrowserProxyUsesHttpFallback(URI expected, URI result) {
+        if (expected.getScheme().equals("ftp") || expected.getScheme().startsWith("http")) {
+            assertEquals(expected.getScheme(), result.getScheme());
+        } else {
+            assertEquals(result.getScheme(), "http");
+        }
+    }
+
+    // Test that absolute resource paths do not result in double-slashes within the URI
+    public void assertQueryForBrowserProxyContainsNoDoubleSlashes(String uri) {
+        assertFalse(uri.contains("//"));
+    }
+
+    // Test that the query string of the URI is not changed
+    public void assertQueryForBrowserProxyDoesNotChangeQuery(URI expected, URI result) {
+        assertEquals(expected.getQuery(), result.getQuery());
+    }
+
 }