JDK-8244650 : Amend JarURLConnection::getJarFile() to return JarFile object reference for nonexistent JAR file entry URL
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P4
  • Status: Draft
  • Resolution: Unresolved
  • Fix Versions: tbd
  • Submitted: 2020-05-08
  • Updated: 2020-05-20
Related Reports
CSR :  
Description
Summary
-------

The request is to change the behaviour of JarURLConnection::getJarFile() method to return a reference to a JarFile (instead of throwing an exception), when URL points to a nonexistent entry inside that JAR file.

Problem
-------

Currently JarURLConnection::getJarFile() method invokes connect() that retrieves JarFile via JarFileFactory. After that, when URL points to a an entry in the JAR file, connect() attempts to open that entry. If such entry doesn't exist, FileNotFoundException is thrown. In this case JarFile is successfully retrieved and cached (caching is enabled by default), but is not returned to client and thus cannot be closed by client. This effectively makes this JarFile object and its associated resources to became a "leak". On Windows it can be observed by client because corresponding file handle remains open and JAR file cannot be deleted from file system.  

Solution
--------

The proposed solution is to change the behaviour of JarURLConnection::getJarFile() method, catching the possible exception from connect() call and returning JarFile to client if it was retrieved successfully, even if access to the JAR entry failed.

Possible alternative (variation to the above) would be to return "null" (instead of re-throwing the exception) in case when JAR file itself is not accessible ("jarFile" remains "null").

Another alternative solution (workaround) may be to disable URLConnection caching for JAR files by default.

Specification
-------------

Proposed code change:

```
--- old/src/java.base/share/classes/sun/net/www/protocol/jar/JarURLConnection.java	2020-05-11 23:17:15.455179557 +0100
+++ new/src/java.base/share/classes/sun/net/www/protocol/jar/JarURLConnection.java	2020-05-11 23:17:15.349178793 +0100
@@ -89,8 +89,16 @@
     }
 
     public JarFile getJarFile() throws IOException {
-        connect();
-        return jarFile;
+        try {
+            connect();
+            return jarFile;
+        } catch (IOException e) {
+            if (jarFile != null) {
+                return jarFile;
+            } else {
+                throw e;
+            }
+        }
     }
 
     public JarEntry getJarEntry() throws IOException {
```

Proposed specification change for the JarURLConnection::getJarFile() method, clarifying the changed behaviour:

```
--- old/src/java.base/share/classes/java/net/JarURLConnection.java	2020-05-11 23:17:15.044176594 +0100
+++ new/src/java.base/share/classes/java/net/JarURLConnection.java	2020-05-11 23:17:14.937175823 +0100
@@ -215,7 +215,8 @@
      *
      * @return the JAR file for this connection. If the connection is
      * a connection to an entry of a JAR file, the JAR file object is
-     * returned
+     * returned. The JAR file is also returned if the connection to
+     * a JAR file entry fails, but the JAR file itself is accessible.
      *
      * @throws    IOException if an IOException occurs while trying to
      * connect to the JAR file for this connection.
```

Proposed specification change for the JarURLConnection::getManifest() method, it is necessary because client code may expect to get an exception when connection points to a nonexistent entry in a JAR file, and with the proposed code change Manifest instance will be returned instead:

```
@@ -228,7 +229,10 @@
      * Returns the Manifest for this connection, or null if none.
      *
      * @return the manifest object corresponding to the JAR file object
-     * for this connection.
+     * for this connection. If the connection is a connection to an
+     * entry of a JAR file, the manifest object is also returned if the
+     * connection to a JAR file entry fails, but the JAR file itself is
+     * accessible.
      *
      * @throws    IOException if getting the JAR file for this
      * connection causes an IOException to be thrown.
```

Proposed specification change for the JarURLConnection::getJarEntry() method, it is necessary because with the proposed code change its "@throws" specification is no longer correct:

```
@@ -248,7 +252,8 @@
      * the JAR URL for this connection points to a JAR file.
      *
      * @throws    IOException if getting the JAR file for this
-     * connection causes an IOException to be thrown.
+     * connection causes an IOException to be thrown, or if the
+     * connection to a JAR file entry fails.
      *
      * @see #getJarFile
      * @see #getJarEntry
```

Comments
Updated "Compatibility Risk Description" adding a reference to getMainAttributes. As nobody else commented on JCK, this is what I can see about the tests related to this change: api/java_net/JarURLConnection/GetJarEntry.html: It covers the following cases (excluding "success" cases): JarURLConnectionT220: "jar:" + testdirurl + "jardir/NOJARFILE.jar!/" JarURLConnectionT223: "jar:" + testdirurl + "jardir/NOJARFILE.jar!/NotAvailable.class" No new failures, case with accessible JAR file and nonexistent JAR entry is not covered. api/java_net/JarURLConnection/GetJarFile.html: JarURLConnectionT212: "jar:" + testdirurl + "jardir/NOJARFILE.jar!/" No new failures, case with accessible JAR file and nonexistent JAR entry is not covered. api/java_net/JarURLConnection/GetMainAttributes.html: JarURLConnectionT228: "jar:" + testdirurl + "jardir/NOJARFILE.jar!/" No new failures, case with accessible JAR file and nonexistent JAR entry is not covered. api/java_net/JarURLConnection/GetManifest.html: JarURLConnectionT216: "jar:" + testdirurl + "/jardir/T11.jar!/NOJARENTRY.class" 1 new failure: Failed. FAIL->getManifest() did not throw expected IOException for jar:file:[...]/jardir/T11.jar!/NOJARENTRY.class
20-05-2020

should the compatibility risk also include a reference to getMainAttributes its behaviour will now change ? Also would it be expected that both getJarFile and getMainAttributes behaviour changes would be alerted in the JCK test?
13-05-2020

Note: this trips the JCK test: api/java_net/JarURLConnection/GetManifest.html which expects an IOException in this case.
08-05-2020