/hg/icedtea-web: Fixed PR2591 - IcedTea-Web request resources tw...
jvanek at icedtea.classpath.org
jvanek at icedtea.classpath.org
Wed Jan 6 16:04:32 UTC 2016
changeset 2a4f622776e9 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=2a4f622776e9
author: Jiri Vanek <jvanek at redhat.com>
date: Wed Jan 06 10:53:12 2016 +0100
Fixed PR2591 - IcedTea-Web request resources twice for meta informations and causes ClientAbortException on tomcat in conjunction with JnlpDownloadServlet
diffstat:
ChangeLog | 17 +-
NEWS | 1 +
netx/net/sourceforge/jnlp/cache/ResourceDownloader.java | 121 ++++++---
tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java | 31 +-
4 files changed, 113 insertions(+), 57 deletions(-)
diffs (417 lines):
diff -r 6cbc3a7acc95 -r 2a4f622776e9 ChangeLog
--- a/ChangeLog Wed Jan 06 10:19:00 2016 +0100
+++ b/ChangeLog Wed Jan 06 10:53:12 2016 +0100
@@ -1,3 +1,16 @@
+2016-01-06 Jiri Vanek <jvanek at redhat.com>
+
+ Fixed PR2591 - IcedTea-Web request resources twice for meta informations and
+ causes ClientAbortException on tomcat in conjunction with JnlpDownloadServlet
+ * NEWS: mentioned PR2591
+ * netx/net/sourceforge/jnlp/cache/ResourceDownloader.java: CodeWithRedirect renamed
+ to UrlRequestResult and now cached also lastModified and length if available.
+ (initializeFromURL) now expects UrlRequestResult instead of URL, (findBestUrl)
+ now returns in same manner
+ (SimpleTest1CountRequests) now passes
+ * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java: adapted
+ to ResourceDownloader.
+
2016-01-06 Jiri Vanek <jvanek at redhat.com>
Added redirection tests
@@ -7,9 +20,9 @@
* tests/reproducers/simple/simpletest1/testcases/SimpleTest1Test.java: small
refactoring to reuse checking methods
* tests/reproducers/simple/simpletest1/testcases/SimpleTest1CountRequests.java:
+ Added FAILING tests for 2591 - counting ITW requests to test server
+ * tests/reproducers/simple/simpletest1/testcases/SimpleTestDefaultRedirects.java:
added set of tests to test behavior under various redirect codes
- * tests/reproducers/simple/simpletest1/testcases/SimpleTestDefaultRedirects.java:
- Added FAILING tests for 2591 - counting ITW requests to test server
2016-01-05 Jiri Vanek <jvanek at redhat.com>
diff -r 6cbc3a7acc95 -r 2a4f622776e9 NEWS
--- a/NEWS Wed Jan 06 10:19:00 2016 +0100
+++ b/NEWS Wed Jan 06 10:53:12 2016 +0100
@@ -14,6 +14,7 @@
* permissions sandbox and signed app and unsigned app with permissions all-permissions now run in sandbox instead of not at all.
* fixed DownloadService
* PR2779: html-gen.sh: Don't try to call hg if .hg directory isn't present
+* PR2591 - IcedTea-Web request resources twice for meta informations and causes ClientAbortException on tomcat in conjunction with JnlpDownloadServlet
* comments in deployment.properties now should persists load/save
* fixed bug in caching of files with query
* fixed issues with recreating of existing shortcut
diff -r 6cbc3a7acc95 -r 2a4f622776e9 netx/net/sourceforge/jnlp/cache/ResourceDownloader.java
--- a/netx/net/sourceforge/jnlp/cache/ResourceDownloader.java Wed Jan 06 10:19:00 2016 +0100
+++ b/netx/net/sourceforge/jnlp/cache/ResourceDownloader.java Wed Jan 06 10:53:12 2016 +0100
@@ -57,8 +57,8 @@
* HttpURLConnection.HTTP_OK and null if not.
* @throws IOException
*/
- static CodeWithRedirect getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, ResourceTracker.RequestMethods requestMethod) throws IOException {
- CodeWithRedirect result = new CodeWithRedirect();
+ static UrlRequestResult getUrlResponseCodeWithRedirectonResult(URL url, Map<String, String> requestProperties, ResourceTracker.RequestMethods requestMethod) throws IOException {
+ UrlRequestResult result = new UrlRequestResult();
URLConnection connection = ConnectionFactory.getConnectionFactory().openConnection(url);
for (Map.Entry<String, String> property : requestProperties.entrySet()) {
@@ -92,6 +92,9 @@
}
ConnectionFactory.getConnectionFactory().disconnect(connection);
+ result.lastModified = connection.getLastModified();
+ result.length = connection.getContentLengthLong();
+
return result;
}
@@ -120,7 +123,7 @@
private void initializeOnlineResource() {
try {
- URL finalLocation = findBestUrl(resource);
+ UrlRequestResult finalLocation = findBestUrl(resource);
if (finalLocation != null) {
initializeFromURL(finalLocation);
} else {
@@ -136,18 +139,25 @@
}
}
- private void initializeFromURL(URL location) throws IOException {
+ private void initializeFromURL(UrlRequestResult location) throws IOException {
CacheEntry entry = new CacheEntry(resource.getLocation(), resource.getRequestVersion());
entry.lock();
try {
- resource.setDownloadLocation(location);
- URLConnection connection = ConnectionFactory.getConnectionFactory().openConnection(location); // this won't change so should be okay not-synchronized
+ resource.setDownloadLocation(location.URL);
+ URLConnection connection = ConnectionFactory.getConnectionFactory().openConnection(location.URL); // this won't change so should be okay not-synchronized
connection.addRequestProperty("Accept-Encoding", "pack200-gzip, gzip");
File localFile = CacheUtil.getCacheFile(resource.getLocation(), resource.getDownloadVersion());
- long size = connection.getContentLengthLong();
+ Long size = location.length;
+ if (size == null) {
+ size = connection.getContentLengthLong();
+ }
+ Long lm = location.lastModified;
+ if (lm == null) {
+ lm = connection.getLastModified();
+ }
- boolean current = CacheUtil.isCurrent(resource.getLocation(), resource.getRequestVersion(), connection.getLastModified()) && resource.getUpdatePolicy() != UpdatePolicy.FORCE;
+ boolean current = CacheUtil.isCurrent(resource.getLocation(), resource.getRequestVersion(), lm) && resource.getUpdatePolicy() != UpdatePolicy.FORCE;
if (!current) {
if (entry.isCached()) {
entry.markForDelete();
@@ -168,14 +178,15 @@
resource.changeStatus(EnumSet.of(PRECONNECT, CONNECTING), EnumSet.of(CONNECTED, PREDOWNLOAD));
// check if up-to-date; if so set as downloaded
- if (current)
+ if (current) {
resource.changeStatus(EnumSet.of(PREDOWNLOAD, DOWNLOADING), EnumSet.of(DOWNLOADED));
+ }
}
// update cache entry
if (!current) {
- entry.setRemoteContentLength(connection.getContentLengthLong());
- entry.setLastModified(connection.getLastModified());
+ entry.setRemoteContentLength(size);
+ entry.setLastModified(lm);
}
entry.setLastUpdated(System.currentTimeMillis());
@@ -225,14 +236,14 @@
}
/**
- * Returns the 'best' valid URL for the given resource.
- * This first adjusts the file name to take into account file versioning
- * and packing, if possible.
+ * Returns the 'best' valid URL for the given resource. This first adjusts
+ * the file name to take into account file versioning and packing, if
+ * possible.
*
* @param resource the resource
* @return the best URL, or null if all failed to resolve
*/
- protected URL findBestUrl(Resource resource) {
+ protected UrlRequestResult findBestUrl(Resource resource) {
DownloadOptions options = resource.getDownloadOptions();
if (options == null) {
options = new DownloadOptions(false, false);
@@ -242,7 +253,6 @@
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Finding best URL for: " + resource.getLocation() + " : " + options.toString());
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "All possible urls for "
+ resource.toString() + " : " + urls);
-
for (ResourceTracker.RequestMethods requestMethod : ResourceTracker.RequestMethods.getValidRequestMethods()) {
for (int i = 0; i < urls.size(); i++) {
URL url = urls.get(i);
@@ -250,13 +260,13 @@
Map<String, String> requestProperties = new HashMap<>();
requestProperties.put("Accept-Encoding", "pack200-gzip, gzip");
- CodeWithRedirect response = getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod);
- if (response.shouldRedirect()){
+ UrlRequestResult response = getUrlResponseCodeWithRedirectonResult(url, requestProperties, requestMethod);
+ if (response.shouldRedirect()) {
if (response.URL == null) {
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Although " + resource.toString() + " got redirect " + response.result + " code for " + requestMethod + " request for " + url.toExternalForm() + " the target was null. Not following");
} else {
- OutputController.getLogger().log(OutputController.Level.MESSAGE_DEBUG, "Resource " + resource.toString() + " got redirect " + response.result + " code for " + requestMethod + " request for " + url.toExternalForm() + " adding " + response.URL.toExternalForm()+" to list of possible urls");
- if (!JNLPRuntime.isAllowRedirect()){
+ OutputController.getLogger().log(OutputController.Level.MESSAGE_DEBUG, "Resource " + resource.toString() + " got redirect " + response.result + " code for " + requestMethod + " request for " + url.toExternalForm() + " adding " + response.URL.toExternalForm() + " to list of possible urls");
+ if (!JNLPRuntime.isAllowRedirect()) {
throw new RedirectionException("The resource " + url.toExternalForm() + " is being redirected (" + response.result + ") to " + response.URL.toExternalForm() + ". This is disabled by default. If you wont to allow it, run javaws with -allowredirect parameter.");
}
urls.add(response.URL);
@@ -265,7 +275,11 @@
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "For " + resource.toString() + " the server returned " + response.result + " code for " + requestMethod + " request for " + url.toExternalForm());
} else {
OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "best url for " + resource.toString() + " is " + url.toString() + " by " + requestMethod);
- return url; /* This is the best URL */
+ if (response.URL == null) {
+ response.URL = url;
+ }
+ return response; /* This is the best URL */
+
}
} catch (IOException e) {
// continue to next candidate
@@ -289,18 +303,17 @@
String contentEncoding = connection.getContentEncoding();
- OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Downloading " + downloadTo + " using " +
- downloadFrom + " (encoding : " + contentEncoding + ") ");
+ OutputController.getLogger().log(OutputController.Level.ERROR_DEBUG, "Downloading " + downloadTo + " using "
+ + downloadFrom + " (encoding : " + contentEncoding + ") ");
- boolean packgz = "pack200-gzip".equals(contentEncoding) ||
- downloadFrom.getPath().endsWith(".pack.gz");
+ boolean packgz = "pack200-gzip".equals(contentEncoding)
+ || downloadFrom.getPath().endsWith(".pack.gz");
boolean gzip = "gzip".equals(contentEncoding);
// It's important to check packgz first. If a stream is both
// pack200 and gz encoded, then con.getContentEncoding() could
// return ".gz", so if we check gzip first, we would end up
// treating a pack200 file as a jar file.
-
if (packgz) {
downloadPackGzFile(resource, connection, new URL(downloadFrom + ".pack.gz"), downloadTo);
} else if (gzip) {
@@ -380,7 +393,7 @@
resource.incrementTransferred(rlen);
out.write(buf, 0, rlen);
}
-
+
in.close();
}
}
@@ -393,14 +406,14 @@
try (GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(CacheUtil
.getCacheFile(compressedLocation, version)))) {
InputStream inputStream = new BufferedInputStream(gzInputStream);
-
+
BufferedOutputStream outputStream = new BufferedOutputStream(new FileOutputStream(CacheUtil
.getCacheFile(uncompressedLocation, version)));
-
+
while (-1 != (rlen = inputStream.read(buf))) {
outputStream.write(buf, 0, rlen);
}
-
+
outputStream.close();
inputStream.close();
}
@@ -412,29 +425,51 @@
try (GZIPInputStream gzInputStream = new GZIPInputStream(new FileInputStream(CacheUtil
.getCacheFile(compressedLocation, version)))) {
InputStream inputStream = new BufferedInputStream(gzInputStream);
-
+
JarOutputStream outputStream = new JarOutputStream(new FileOutputStream(CacheUtil
.getCacheFile(uncompressedLocation, version)));
-
+
Pack200.Unpacker unpacker = Pack200.newUnpacker();
unpacker.unpack(inputStream, outputStream);
-
+
outputStream.close();
inputStream.close();
}
}
/**
- * Complex wrapper around return code with utility methods
- * Default is HTTP_OK
+ * Complex wrapper around url request Contains return code (default is
+ * HTTP_OK), length and last modified
+ *
+ * The storing of redirect target is quite obvious The storing length and
+ * last modified may be not, but appearently
+ * (http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=2591) the url
+ * conenction is not always chaced as expected, and so another request may
+ * be sent when length and lastmodified are checked
+ *
*/
- private static class CodeWithRedirect {
+ static class UrlRequestResult {
int result = HttpURLConnection.HTTP_OK;
URL URL;
+ Long lastModified;
+ Long length;
+
+ public UrlRequestResult() {
+ }
+
+ public UrlRequestResult(URL URL) {
+ this.URL = URL;
+ }
+
+ URL getURL() {
+ return URL;
+ }
+
/**
- * @return whether the result code is redirect one. Rigth now 301-303 and 307-308
+ * @return whether the result code is redirect one. Rigth now 301-303
+ * and 307-308
*/
public boolean shouldRedirect() {
return (result == 301
@@ -445,11 +480,20 @@
}
/**
- * @return whether the return code is OK one - anything except <200,300)
+ * @return whether the return code is OK one - anything except <200,300)
*/
public boolean isInvalid() {
return (result < 200 || result >= 300);
}
+
+ @Override
+ public String toString() {
+ return ""
+ + "url: " + (URL == null ? "null" : URL.toExternalForm()) + "; "
+ + "result:" + result + "; "
+ + "lastModified: " + (lastModified == null ? "null" : lastModified.toString()) + "; "
+ + "length: " + length == null ? "null" : length.toString() + "; ";
+ }
}
private static class RedirectionException extends RuntimeException {
@@ -464,5 +508,4 @@
}
-
}
diff -r 6cbc3a7acc95 -r 2a4f622776e9 tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java Wed Jan 06 10:19:00 2016 +0100
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java Wed Jan 06 10:53:12 2016 +0100
@@ -19,7 +19,6 @@
import java.util.jar.Pack200;
import java.util.zip.GZIPOutputStream;
-
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
@@ -35,7 +34,7 @@
import net.sourceforge.jnlp.util.logging.NoStdOutErrTest;
import net.sourceforge.jnlp.util.logging.OutputController;
-public class ResourceDownloaderTest extends NoStdOutErrTest{
+public class ResourceDownloaderTest extends NoStdOutErrTest {
public static ServerLauncher testServer;
public static ServerLauncher testServerWithBrokenHead;
@@ -78,7 +77,6 @@
OutputController.getLogger().setOut(new PrintStream(currentErrorStream));
OutputController.getLogger().setErr(new PrintStream(currentErrorStream));
-
}
@AfterClass
@@ -130,7 +128,8 @@
redirectErr();
try {
File f = File.createTempFile(nameStub1, nameStub2);
- int i = ResourceDownloader.getUrlResponseCode(testServer.getUrl(f.getName()), new HashMap<String, String>(), ResourceTracker.RequestMethods.HEAD); Assert.assertEquals(HttpURLConnection.HTTP_OK, i);
+ int i = ResourceDownloader.getUrlResponseCode(testServer.getUrl(f.getName()), new HashMap<String, String>(), ResourceTracker.RequestMethods.HEAD);
+ Assert.assertEquals(HttpURLConnection.HTTP_OK, i);
f.delete();
i = ResourceDownloader.getUrlResponseCode(testServer.getUrl(f.getName()), new HashMap<String, String>(), ResourceTracker.RequestMethods.HEAD);
Assert.assertEquals(HttpURLConnection.HTTP_NOT_FOUND, i);
@@ -227,30 +226,29 @@
Resource r2 = Resource.getResource(testServerWithBrokenHead.getUrl(fileForServerWithoutHeader.getName()), null, UpdatePolicy.NEVER);
Resource r3 = Resource.getResource(testServer.getUrl(versionedFileForServerWithHeader.getName()), new Version("1.0"), UpdatePolicy.NEVER);
Resource r4 = Resource.getResource(testServerWithBrokenHead.getUrl(versionedFileForServerWithoutHeader.getName()), new Version("1.0"), UpdatePolicy.NEVER);
- assertOnServerWithHeader(resourceDownloader.findBestUrl(r1));
- assertVersionedOneOnServerWithHeader(resourceDownloader.findBestUrl(r3));
- assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2));
- assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4));
+ assertOnServerWithHeader(resourceDownloader.findBestUrl(r1).getURL());
+ assertVersionedOneOnServerWithHeader(resourceDownloader.findBestUrl(r3).URL);
+ assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2).URL);
+ assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4).URL);
fileForServerWithHeader.delete();
Assert.assertNull(resourceDownloader.findBestUrl(r1));
- assertVersionedOneOnServerWithHeader(resourceDownloader.findBestUrl(r3));
- assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2));
- assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4));
+ assertVersionedOneOnServerWithHeader(resourceDownloader.findBestUrl(r3).URL);
+ assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2).URL);
+ assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4).URL);
versionedFileForServerWithHeader.delete();
Assert.assertNull(resourceDownloader.findBestUrl(r1));
Assert.assertNull(resourceDownloader.findBestUrl(r3));
- assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2));
- assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4));
+ assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2).URL);
+ assertVersionedOneOnServerWithoutHeader(resourceDownloader.findBestUrl(r4).URL);
versionedFileForServerWithoutHeader.delete();
Assert.assertNull(resourceDownloader.findBestUrl(r1));
Assert.assertNull(resourceDownloader.findBestUrl(r3));
- assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2));
+ assertOnServerWithoutHeader(resourceDownloader.findBestUrl(r2).URL);
Assert.assertNull(resourceDownloader.findBestUrl(r4));
-
fileForServerWithoutHeader.delete();
Assert.assertNull(resourceDownloader.findBestUrl(r1));
Assert.assertNull(resourceDownloader.findBestUrl(r3));
@@ -283,6 +281,7 @@
assertPort(u, testServer.getPort());
assertVersion(u);
}
+
private void assertCommonComponentsOfUrl(URL u) {
Assert.assertTrue(u.getProtocol().equals("http"));
Assert.assertTrue(u.getHost().equals("localhost"));
@@ -311,7 +310,7 @@
redirectErrBack();
cacheDir = PathsAndFiles.CACHE_DIR.getFullPath();
- PathsAndFiles.CACHE_DIR.setValue(System.getProperty("java.io.tmpdir") + File.separator + "tempcache");
+ PathsAndFiles.CACHE_DIR.setValue(System.getProperty("java.io.tmpdir") + File.separator + "tempcache");
}
@AfterClass
More information about the distro-pkg-dev
mailing list