diff options
8 files changed, 369 insertions, 32 deletions
@@ -1,3 +1,26 @@ +2013-09-26 Andrew Azores <[email protected]> + + Fix for PR1204. Absolute paths in resource URLs are correctly handled when + appended to host URLs and URL query strings are not removed. + * netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java: + (getVersionedUrlUsingQuery) renamed to getVersionedUrl, refactored + construction of URL + * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java: + (requestPluginProxyInfo) extracted proxy URI logic. + (processProxyUri) new method for finding proxy URIs, handles absolute + resource paths correctly + * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceUrlCreatorTest.java: + added tests for ResourceUrlCreator#getVersionedUrl + * tests/netx/unit/sun/applet/PluginAppletViewerTest.java: added tests for + PluginAppletViewer.processProxyUri + * tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.html: + new reproducer checks that absolute paths and query strings in resource + URLs are properly handled, and caching still works + * tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.jnlp: + same + * tests/reproducers/simple/AbsolutePathsAndQueryStrings/testcases/AbsolutePathsAndQueryStrings.java: + same + 2013-09-25 Andrew Azores <[email protected]> * Makefile.am: clean up summary_unit.txt and summary_reproducers.txt for diff --git a/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java b/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java index 68ab009..0da5466 100644 --- a/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java +++ b/netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java @@ -38,6 +38,8 @@ exception statement from your version. */ package net.sourceforge.jnlp.cache; import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.util.LinkedList; import java.util.List; @@ -89,7 +91,7 @@ public class ResourceUrlCreator { } } - url = getVersionedUrlUsingQuery(resource); + url = getVersionedUrl(resource); urls.add(url); urls.add(resource.getLocation()); @@ -146,29 +148,29 @@ public class ResourceUrlCreator { } /** - * Returns the URL for a resource, relying on HTTP query for getting the - * right version + * Returns the URL for a resource, including the resource's version number in the query string * * @param resource the resource to get the url for */ - protected URL getVersionedUrlUsingQuery(Resource resource) { - String actualLocation = resource.getLocation().getProtocol() + "://" - + resource.getLocation().getHost(); - if (resource.getLocation().getPort() != -1) { - actualLocation += ":" + resource.getLocation().getPort(); - } - actualLocation += resource.getLocation().getPath(); - if (resource.requestVersion != null - && resource.requestVersion.isVersionId()) { - actualLocation += "?version-id=" + resource.requestVersion; - } - URL versionedURL; + protected URL getVersionedUrl(Resource resource) { + URL resourceUrl = resource.getLocation(); try { - versionedURL = new URL(actualLocation); + String query = resourceUrl.getQuery(); // returns null if there was no query string + if (resource.requestVersion != null && resource.requestVersion.isVersionId()) { + if (query == null) { + query = ""; + } else { + query += "&"; + } + query += "version-id=" + resource.requestVersion; + } + URI uri = new URI(resourceUrl.getProtocol(), resourceUrl.getUserInfo(), resourceUrl.getHost(), resourceUrl.getPort(), resourceUrl.getPath(), query, null); + return uri.toURL(); } catch (MalformedURLException e) { - return resource.getLocation(); + return resourceUrl; + } catch (URISyntaxException e) { + return resourceUrl; } - return versionedURL; } -}
\ No newline at end of file +} diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java index f0c1cc5..bd4c0d5 100644 --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java @@ -86,6 +86,7 @@ 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; @@ -1243,17 +1244,7 @@ public class PluginAppletViewer extends XEmbeddedFrame Long reference = getRequestIdentifier(); try { - - // 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(); - String port = uri.getPort() != -1 ? ":" + uri.getPort() : ""; - if (!uri.getScheme().startsWith("http") && !uri.getScheme().equals("ftp")) - scheme = "http"; - - requestURI = UrlUtil.encode(scheme + "://" + uri.getHost() + port + "/" + uri.getPath(), "UTF-8"); + 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); @@ -1283,6 +1274,21 @@ 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/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceUrlCreatorTest.java b/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceUrlCreatorTest.java index 208180d..5c5b6b1 100644 --- a/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceUrlCreatorTest.java +++ b/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceUrlCreatorTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertEquals; import java.net.MalformedURLException; import java.net.URL; +import net.sourceforge.jnlp.DownloadOptions; import net.sourceforge.jnlp.Version; import org.junit.Test; @@ -37,7 +38,55 @@ public class ResourceUrlCreatorTest { @Test public void testVersionAndPackEncode() throws MalformedURLException { Resource resource = Resource.getResource(new URL("http://test.jar"), new Version("1.1"), null); - URL result = ResourceUrlCreator.getUrl(resource, true /*use pack suffix*/, true/*use version suffix*/); + URL result = ResourceUrlCreator.getUrl(resource, true /*use pack suffix*/, true /*use version suffix*/); assertEquals("http://test__V1.1.jar.pack.gz", result.toString()); } + + @Test + public void testGetVersionedUrl() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://foo.com/bar.jar"), new Version("1.1"), null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, true)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://foo.com/bar.jar?version-id=1.1", result.toString()); + } + + @Test + public void testGetNonVersionIdUrl() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://foo.com/some.jar"), new Version("version two"), null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, true)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://foo.com/some.jar", result.toString()); + } + + @Test + public void testGetVersionedUrlWithQuery() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://bar.com/bar.jar?i=1234abcd"), new Version("1.1"), null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, true)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://bar.com/bar.jar?i=1234abcd&version-id=1.1", result.toString()); + } + + @Test + public void testGetVersionedUrlWithoutVersion() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://baz.com/bar.jar"), null, null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, false)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://baz.com/bar.jar", result.toString()); + } + + @Test + public void testGetVersionedUrlWithoutVersionWithQuery() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://rhat.com/bar.jar?i=1234abcd"), null, null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, false)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://rhat.com/bar.jar?i=1234abcd", result.toString()); + } + + @Test + public void testGetVersionedUrlWithLongQuery() throws MalformedURLException { + Resource resource = Resource.getResource(new URL("http://yyz.com/bar.jar?i=1234&j=abcd"), new Version("2.0"), null); + ResourceUrlCreator ruc = new ResourceUrlCreator(resource, new DownloadOptions(false, true)); + URL result = ruc.getVersionedUrl(resource); + assertEquals("http://yyz.com/bar.jar?i=1234&j=abcd&version-id=2.0", result.toString()); + } } diff --git a/tests/netx/unit/sun/applet/PluginAppletViewerTest.java b/tests/netx/unit/sun/applet/PluginAppletViewerTest.java index cecedfc..69e5cb1 100644 --- a/tests/netx/unit/sun/applet/PluginAppletViewerTest.java +++ b/tests/netx/unit/sun/applet/PluginAppletViewerTest.java @@ -42,8 +42,11 @@ import static org.junit.Assert.assertEquals; import static sun.applet.PluginPipeMockUtil.getPluginStoreId; import static sun.applet.PluginPipeMockUtil.getPluginStoreObject; +import java.net.URI; import java.util.concurrent.Callable; +import junit.framework.Assert; + import net.sourceforge.jnlp.AsyncCall; import net.sourceforge.jnlp.ServerAccess; @@ -156,6 +159,43 @@ 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 * **************************************************************************/ @@ -238,4 +278,4 @@ public class PluginAppletViewerTest { int expectedLength = 6; return parseAndCheckJSMessage(message, expectedLength, "ToString", contextObjectID); } -}
\ No newline at end of file +} diff --git a/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.html b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.html new file mode 100644 index 0000000..f2fc6eb --- /dev/null +++ b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.html @@ -0,0 +1,48 @@ +<!-- + +This file is part of IcedTea. + +IcedTea is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2, or (at your option) +any later version. + +IcedTea is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with IcedTea; see the file COPYING. If not, write to the +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +02110-1301 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. + + --> +<html> + <head></head> + <body> + <embed code="StripHttpPathParams" + archive="/StripHttpPathParams.jar?i=1234abcd" + type="application/x-java-applet;version=1.6" + width="800" + height="600"> + </embed> + </body> +</html> diff --git a/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.jnlp b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.jnlp new file mode 100644 index 0000000..c499bc8 --- /dev/null +++ b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/resources/AbsolutePathsAndQueryStrings.jnlp @@ -0,0 +1,53 @@ +<!-- + +This file is part of IcedTea. + +IcedTea is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2, or (at your option) +any later version. + +IcedTea is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with IcedTea; see the file COPYING. If not, write to the +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +02110-1301 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. + + --> +<?xml version="1.0" encoding="utf-8"?> +<jnlp spec="1.0" href="AbsolutePathsAndQueryStrings.jnlp" codebase="."> + <information> + <title>AbsolutePathsAndQueryStrings</title> + <vendor>IcedTea</vendor> + <homepage href="http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web"/> + <description>Use relative codebase URLs to append to host names, and correctly parse query strings</description> + <offline/> + </information> + <resources> + <j2se version="1.4+"/> + <jar href="/StripHttpPathParams.jar?i=1234abcd"/> + </resources> + <application-desc main-class="StripHttpPathParams"> + </application-desc> +</jnlp> diff --git a/tests/reproducers/simple/AbsolutePathsAndQueryStrings/testcases/AbsolutePathsAndQueryStrings.java b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/testcases/AbsolutePathsAndQueryStrings.java new file mode 100644 index 0000000..7f561e7 --- /dev/null +++ b/tests/reproducers/simple/AbsolutePathsAndQueryStrings/testcases/AbsolutePathsAndQueryStrings.java @@ -0,0 +1,116 @@ +/* AbsolutePathsAndQueryStrings.java +Copyright (C) 2013 Red Hat, Inc. + +This file is part of IcedTea. + +IcedTea is free software; you can redistribute it and/or +modify it under the terms of the GNU General Public License as published by +the Free Software Foundation, version 2. + +IcedTea is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with IcedTea; see the file COPYING. If not, write to +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +02110-1301 USA. + +Linking this library statically or dynamically with other modules is +making a combined work based on this library. Thus, the terms and +conditions of the GNU General Public License cover the whole +combination. + +As a special exception, the copyright holders of this library give you +permission to link this library with independent modules to produce an +executable, regardless of the license terms of these independent +modules, and to copy and distribute the resulting executable under +terms of your choice, provided that you also meet, for each linked +independent module, the terms and conditions of the license of that +module. An independent module is a module which is not derived from +or based on this library. If you modify this library, you may extend +this exception to your version of the library, but you are not +obligated to do so. If you do not wish to do so, delete this +exception statement from your version. + */ + +import java.io.File; +import java.net.URL; +import net.sourceforge.jnlp.ProcessResult; +import net.sourceforge.jnlp.ServerAccess.AutoClose; +import net.sourceforge.jnlp.annotations.Bug; +import net.sourceforge.jnlp.annotations.KnownToFail; +import net.sourceforge.jnlp.annotations.NeedsDisplay; +import net.sourceforge.jnlp.annotations.TestInBrowsers; +import net.sourceforge.jnlp.browsertesting.BrowserTest; +import net.sourceforge.jnlp.browsertesting.Browsers; +import net.sourceforge.jnlp.closinglisteners.AutoOkClosingListener; +import net.sourceforge.jnlp.ServerAccess; +import net.sourceforge.jnlp.cache.CacheUtil; +import net.sourceforge.jnlp.runtime.JNLPRuntime; +import net.sourceforge.jnlp.config.DeploymentConfiguration; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.AfterClass; + +public class AbsolutePathsAndQueryStrings extends BrowserTest { + + private static final String appletCloseString = AutoOkClosingListener.MAGICAL_OK_CLOSING_STRING; + + @Bug(id="PR1204") + @NeedsDisplay + @Test + @TestInBrowsers(testIn={Browsers.one}) + public void testAbsolutePathAndQueryStringBrowser() throws Exception { + /* HTML specifies absolute path and path params, ensure that this is able to launch correctly */ + ProcessResult pr = server.executeBrowser("/AbsolutePathsAndQueryStrings.html", AutoClose.CLOSE_ON_BOTH); + Assert.assertTrue("stdout should contain " + appletCloseString + " but did not", pr.stdout.contains(appletCloseString)); + } + + @Bug(id="PR1204") + @NeedsDisplay + @Test + public void testAbsolutePathAndQueryStringWebstart() throws Exception { + /* JNLP specifies absolute path and path params, ensure that this is able to launch correctly */ + ProcessResult pr = server.executeJavawsHeadless("/AbsolutePathsAndQueryStrings.jnlp"); + Assert.assertTrue("stdout should contain \"running\"but did not", pr.stdout.contains("running")); + } + + @Bug(id="PR1204") + @Test + public void testCaching() throws Exception { + /* Test that caching ignores path parameters and double-slash issue from absolute codebase paths + */ + URL plainLocation = new URL("http://localhost:1234/StripHttpPathParams.jar"); + URL paramLocation = new URL("http://localhost:1234/StripHttpPathParams.jar?i=abcd"); + URL absoluteLocation = new URL("http://localhost:1234//StripHttpPathParams.jar"); + URL absoluteParamLocation = new URL("http://localhost:1234//StripHttpPathParams.jar?i=abcd"); + + DeploymentConfiguration config = JNLPRuntime.getConfiguration(); + config.load(); + String cacheLocation = config.getProperty(DeploymentConfiguration.KEY_USER_CACHE_DIR) + File.separator; + File cacheDir = new File(cacheLocation); + Assert.assertTrue(cacheDir.isDirectory()); + + boolean hasCachedCopy = false; + for (File cache : cacheDir.listFiles()) { + File[] cacheFiles = new File[] { + CacheUtil.urlToPath(plainLocation, cache.getPath()), + CacheUtil.urlToPath(paramLocation, cache.getPath()), + CacheUtil.urlToPath(absoluteLocation, cache.getPath()), + CacheUtil.urlToPath(absoluteParamLocation, cache.getPath()), + }; + for (File f : cacheFiles) { + if (f.isFile()) + hasCachedCopy = true; + for (File g : cacheFiles) { + Assert.assertEquals(f.getPath(), g.getPath()); + } + } + } + Assert.assertTrue(hasCachedCopy); + } + +} |