8264048: Fix caching in Jar URL connections when an entry is missing

Co-authored-by: Daniel Fuchs <dfuchs@openjdk.org>
Reviewed-by: bchristi, dfuchs

src/java.base/share/classes/jdk/internal/loader/URLClassPath.java

@@ -644,14 +644,16 @@ Resource getResource(final String name, boolean check) {
                     URLClassPath.check(url);
                 }
                 uc = url.openConnection();
-                InputStream in = uc.getInputStream();
+
                 if (uc instanceof JarURLConnection) {
                     /* Need to remember the jar file so it can be closed
                      * in a hurry.
                      */
                     JarURLConnection juc = (JarURLConnection)uc;
                     jarfile = JarLoader.checkJar(juc.getJarFile());
                 }
+
+                InputStream in = uc.getInputStream();
             } catch (Exception e) {
                 return null;
             }

src/java.base/share/classes/sun/net/www/protocol/jar/JarURLConnection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -117,36 +117,56 @@ public void close () throws IOException {
         }
     }
 
-
-
     public void connect() throws IOException {
         if (!connected) {
-            /* the factory call will do the security checks */
-            jarFile = factory.get(getJarFileURL(), getUseCaches());
+            boolean useCaches = getUseCaches();
+            String entryName = this.entryName;
 
-            /* we also ask the factory the permission that was required
-             * to get the jarFile, and set it as our permission.
-             */
-            if (getUseCaches()) {
-                boolean oldUseCaches = jarFileURLConnection.getUseCaches();
-                jarFileURLConnection = factory.getConnection(jarFile);
-                jarFileURLConnection.setUseCaches(oldUseCaches);
-            }
+            /* the factory call will do the security checks */
+            URL url = getJarFileURL();
+            // if we have an entry name, and the jarfile is local,
+            // don't put the jar into the cache until after we have
+            // validated that the entry name exists
+            jarFile = entryName == null
+                    ? factory.get(url, useCaches)
+                    : factory.getOrCreate(url, useCaches);
 
             if ((entryName != null)) {
-                jarEntry = (JarEntry)jarFile.getEntry(entryName);
+                jarEntry = (JarEntry) jarFile.getEntry(entryName);
                 if (jarEntry == null) {
                     try {
-                        if (!getUseCaches()) {
-                            jarFile.close();
-                        }
+                        // only close the jar file if it isn't in the
+                        // cache. If the jar file is local, it won't be
+                        // in the cache yet, and so will be closed here.
+                        factory.closeIfNotCached(url, jarFile);
                     } catch (Exception e) {
                     }
                     throw new FileNotFoundException("JAR entry " + entryName +
-                                                    " not found in " +
-                                                    jarFile.getName());
+                            " not found in " +
+                            jarFile.getName());
                 }
             }
+
+            // we have validated that the entry exists.
+            // if useCaches was requested, update the cache now.
+            if (useCaches && entryName != null) {
+                // someone may have beat us and updated the cache
+                // already - in which case - cacheIfAbsent will
+                // return false. cacheIfAbsent returns true if
+                // our jarFile is in the cache when the method
+                // returns, whether because it put it there or
+                // because it found it there.
+                useCaches = factory.cacheIfAbsent(url, jarFile);
+            }
+
+            /* we also ask the factory the permission that was required
+             * to get the jarFile, and set it as our permission.
+             */
+            if (useCaches) {
+                boolean oldUseCaches = jarFileURLConnection.getUseCaches();
+                jarFileURLConnection = factory.getConnection(jarFile);
+                jarFileURLConnection.setUseCaches(oldUseCaches);
+            }
             connected = true;
         }
     }

src/java.base/share/classes/sun/net/www/protocol/jar/URLJarFile.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -104,7 +104,7 @@ private URLJarFile(URL url, URLJarFileCloseController closeController, Runtime.V
         this.closeController = closeController;
     }
 
-    private static boolean isFileURL(URL url) {
+    static boolean isFileURL(URL url) {
         if (url.getProtocol().equalsIgnoreCase("file")) {
             /*
              * Consider this a 'file' only if it's a LOCAL file, because

src/java.base/unix/classes/sun/net/www/protocol/jar/JarFileFactory.java

@@ -71,6 +71,75 @@ public JarFile get(URL url) throws IOException {
         return get(url, true);
     }
 
+    /**
+     * Get or create a {@code JarFile} for the given {@code url}.
+     * If {@code useCaches} is true, this method attempts to find
+     * a jar file in the cache, and if so, returns it.
+     * If no jar file is found in the cache, or {@code useCaches}
+     * is false, the method creates a new jar file.
+     * If the URL points to a local file, the returned jar file
+     * will not be put in the cache yet.
+     * The caller should then call {@link #cacheIfAbsent(URL, JarFile)}
+     * with the returned jar file, if updating the cache is desired.
+     * @param url the jar file url
+     * @param useCaches whether the cache should be used
+     * @return a new or cached jar file.
+     * @throws IOException if the jar file couldn't be created
+     */
+    JarFile getOrCreate(URL url, boolean useCaches) throws IOException {
+        if (useCaches == false) {
+            return get(url, false);
+        }
+
+        if (!URLJarFile.isFileURL(url)) {
+            // A temporary file will be created, we can prepopulate
+            // the cache in this case.
+            return get(url, useCaches);
+        }
+
+        // We have a local file. Do not prepopulate the cache.
+        JarFile result;
+        synchronized (instance) {
+            result = getCachedJarFile(url);
+        }
+        if (result == null) {
+            result = URLJarFile.getJarFile(url, this);
+        }
+        if (result == null)
+            throw new FileNotFoundException(url.toString());
+        return result;
+    }
+
+    /**
+     * Close the given jar file if it isn't present in the cache.
+     * Otherwise, does nothing.
+     * @param url the jar file URL
+     * @param jarFile the jar file to close
+     * @return true if the jar file has been closed, false otherwise.
+     * @throws IOException if an error occurs while closing the jar file.
+     */
+    boolean closeIfNotCached(URL url, JarFile jarFile) throws IOException {
+        JarFile result;
+        synchronized (instance) {
+            result = getCachedJarFile(url);
+        }
+        if (result != jarFile) jarFile.close();
+        return result != jarFile;
+    }
+
+    boolean cacheIfAbsent(URL url, JarFile jarFile) {
+        JarFile cached;
+        synchronized (instance) {
+            String key = urlKey(url);
+            cached = fileCache.get(key);
+            if (cached == null) {
+                fileCache.put(key, jarFile);
+                urlCache.put(jarFile, url);
+            }
+        }
+        return cached == null || cached == jarFile;
+    }
+
     JarFile get(URL url, boolean useCaches) throws IOException {
 
         JarFile result;
@@ -106,7 +175,7 @@ JarFile get(URL url, boolean useCaches) throws IOException {
 
     /**
      * Callback method of the URLJarFileCloseController to
-     * indicate that the JarFile is close. This way we can
+     * indicate that the JarFile is closed. This way we can
      * remove the JarFile from the cache
      */
     public void close(JarFile jarFile) {

src/java.base/windows/classes/sun/net/www/protocol/jar/JarFileFactory.java

@@ -71,17 +71,99 @@ public JarFile get(URL url) throws IOException {
         return get(url, true);
     }
 
-    JarFile get(URL url, boolean useCaches) throws IOException {
+    /**
+     * Get or create a {@code JarFile} for the given {@code url}.
+     * If {@code useCaches} is true, this method attempts to find
+     * a jar file in the cache, and if so, returns it.
+     * If no jar file is found in the cache, or {@code useCaches}
+     * is false, the method creates a new jar file.
+     * If the URL points to a local file, the returned jar file
+     * will not be put in the cache yet.
+     * The caller should then call {@link #cacheIfAbsent(URL, JarFile)}
+     * with the returned jar file, if updating the cache is desired.
+     * @param url the jar file url
+     * @param useCaches whether the cache should be used
+     * @return a new or cached jar file.
+     * @throws IOException if the jar file couldn't be created
+     */
+    JarFile getOrCreate(URL url, boolean useCaches) throws IOException {
+        if (useCaches == false) {
+            return get(url, false);
+        }
+        URL patched = urlFor(url);
+        if (!URLJarFile.isFileURL(patched)) {
+            // A temporary file will be created, we can prepopulate
+            // the cache in this case.
+            return get(url, useCaches);
+        }
+
+        // We have a local file. Do not prepopulate the cache.
+        JarFile result;
+        synchronized (instance) {
+            result = getCachedJarFile(patched);
+        }
+        if (result == null) {
+            result = URLJarFile.getJarFile(patched, this);
+        }
+        if (result == null)
+            throw new FileNotFoundException(url.toString());
+        return result;
+    }
+
+    /**
+     * Close the given jar file if it isn't present in the cache.
+     * Otherwise, does nothing.
+     * @param url the jar file URL
+     * @param jarFile the jar file to close
+     * @return true if the jar file has been closed, false otherwise.
+     * @throws IOException if an error occurs while closing the jar file.
+     */
+    boolean closeIfNotCached(URL url, JarFile jarFile) throws IOException {
+        url = urlFor(url);
+        JarFile result;
+        synchronized (instance) {
+            result = getCachedJarFile(url);
+        }
+        if (result != jarFile) jarFile.close();
+        return result != jarFile;
+    }
+
+    boolean cacheIfAbsent(URL url, JarFile jarFile) {
+        try {
+            url = urlFor(url);
+        } catch (IOException x) {
+            // should not happen
+            return false;
+        }
+        JarFile cached;
+        synchronized (instance) {
+            String key = urlKey(url);
+            cached = fileCache.get(key);
+            if (cached == null) {
+                fileCache.put(key, jarFile);
+                urlCache.put(jarFile, url);
+            }
+        }
+        return cached == null || cached == jarFile;
+    }
+
+    private URL urlFor(URL url) throws IOException {
         if (url.getProtocol().equalsIgnoreCase("file")) {
             // Deal with UNC pathnames specially. See 4180841
 
             String host = url.getHost();
             if (host != null && !host.isEmpty() &&
-                !host.equalsIgnoreCase("localhost")) {
+                    !host.equalsIgnoreCase("localhost")) {
 
                 url = new URL("file", "", "//" + host + url.getPath());
             }
         }
+        return url;
+    }
+
+    JarFile get(URL url, boolean useCaches) throws IOException {
+
+        url = urlFor(url);
 
         JarFile result;
         JarFile local_result;
@@ -116,7 +198,7 @@ JarFile get(URL url, boolean useCaches) throws IOException {
 
     /**
      * Callback method of the URLJarFileCloseController to
-     * indicate that the JarFile is close. This way we can
+     * indicate that the JarFile is closed. This way we can
      * remove the JarFile from the cache
      */
     public void close(JarFile jarFile) {