[PATCH] JDK-8211765 - JarFile constructor throws undocumented exception

[Jaikiran Pai]

Thank you Chris.

I've attached a patch here which catches the InvalidPathException within
the implementation of ZipFile and wraps and throws a IOException. It's
the same patch which I mentioned earlier in this thread, but now also
includes jtreg testcase to reproduce the issue and verify the fix. I
have run the jtreg test with and without this patch on my *nix system
and it behaves as expected (fails without the fix and passes with it). I
don't have a Windows OS at hand to test this patch and the testcase on it.

I also did a very basic check and went through the JarFile/ZipFile
constructor code path to see if there are other places in that flow
which might need similar treatment. My not-so-thorough check didn't show
up any other such instance.

Could I please get help from someone who can sponsor this patch and help
with the review too?

-Jaikiran


On 05/10/18 6:21 PM, Chris Hegarty wrote:
>> On 5 Oct 2018, at 12:08, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
>>
>>> I don't have access to create an issue for this in OpenJDK JIRA, so I'll
>>> go ahead and create one at bugs.java.com.
>>>
>> I just submitted the report. Internal review id 9057522 has been
>> assigned to the issue.
> And now in the JDK project:
>
>   https://bugs.openjdk.java.net/browse/JDK-8211765
>
> -Chris.

-------------- next part --------------
# HG changeset patch
# User Jaikiran Pai <jaikiran.pai at gmail.com>
# Date 1538645309 -19800
#      Thu Oct 04 14:58:29 2018 +0530
# Node ID 957376a6e53c751fa144d0e41e063ef9179a9275
# Parent  e75f6076d391a6081e621d64641526c99bf8c5b2
JDK-8211765 Wrap and throw an IOException when a InvalidPathException is thrown from within ZipFile constructor

diff --git a/src/java.base/share/classes/java/util/zip/ZipFile.java b/src/java.base/share/classes/java/util/zip/ZipFile.java
--- a/src/java.base/share/classes/java/util/zip/ZipFile.java
+++ b/src/java.base/share/classes/java/util/zip/ZipFile.java
@@ -35,6 +35,7 @@
 import java.lang.ref.Cleaner.Cleanable;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.InvalidPathException;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.Files;
 import java.util.ArrayDeque;
@@ -1218,8 +1219,13 @@
 
 
         static Source get(File file, boolean toDelete) throws IOException {
-            Key key = new Key(file,
-                              Files.readAttributes(file.toPath(), BasicFileAttributes.class));
+            final Key key;
+            try {
+                key = new Key(file,
+                        Files.readAttributes(file.toPath(), BasicFileAttributes.class));
+            } catch (InvalidPathException ipe) {
+                throw new IOException(ipe);
+            }
             Source src;
             synchronized (files) {
                 src = files.get(key);
diff --git a/test/jdk/java/util/jar/JarFile/Constructor.java b/test/jdk/java/util/jar/JarFile/Constructor.java
--- a/test/jdk/java/util/jar/JarFile/Constructor.java
+++ b/test/jdk/java/util/jar/JarFile/Constructor.java
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 4842702
+ * @bug 4842702 8211765
  * @summary Check that constructors throw specified exceptions
  * @author Martin Buchholz
  */
@@ -63,5 +63,23 @@
 
         try { Unreached (new JarFile (new File ("NoSuchJar.jar"))); }
         catch (IOException e) {}
+
+        // operating system specific
+        final String osname = System.getProperty("os.name");
+        if (osname.startsWith("Windows")) {
+            doWindowsTests();
+        } else {
+            doUnixTests();
+        }
+    }
+
+    private static void doWindowsTests() throws Exception {
+        try { Unreached (new JarFile ("C:\\*")); } // invalid character
+        catch (IOException e) {}
+    }
+
+    private static void doUnixTests() throws Exception {
+        try { Unreached (new JarFile ("foo\u0000bar")); } // invalid character
+        catch (IOException e) {}
     }
 }