--- a/ChangeLog	Wed Jan 06 09:53:38 2016 +0100
+++ b/ChangeLog	Wed Jan 06 10:49:31 2016 +0100
@@ -1,3 +1,16 @@
+2016-01-06  Jiri Vanek  <jvanek@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@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 set of tests to test behavior under various redirect codes
+	Added FAILING tests for 2591 - counting ITW requests to test server
 	* tests/reproducers/simple/simpletest1/testcases/SimpleTestDefaultRedirects.java:
-	Added FAILING tests for 2591 - counting ITW requests to test server
+	added set of tests to test behavior under various redirect codes
 
 2016-01-05  Jiri Vanek  <jvanek@redhat.com>
 

--- a/NEWS	Wed Jan 06 09:53:38 2016 +0100
+++ b/NEWS	Wed Jan 06 10:49:31 2016 +0100
@@ -11,6 +11,7 @@
 New in release 1.6.2 (YYYY-MM-DD):
 * all connection restrictions now consider also port
 * 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
 * NetX
   - main-class attribute trimmed by default
   - in strict mode, main-class attribute checked for invalid characters

--- a/netx/net/sourceforge/jnlp/cache/ResourceDownloader.java	Wed Jan 06 09:53:38 2016 +0100
+++ b/netx/net/sourceforge/jnlp/cache/ResourceDownloader.java	Wed Jan 06 10:49:31 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 @@
 
     }
 
-
 }

--- a/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java	Wed Jan 06 09:53:38 2016 +0100
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/ResourceDownloaderTest.java	Wed Jan 06 10:49:31 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