Nested ZipFileSystems cause issues with URI parsing

[Mitchell Skaggs]

Lance,

I finally got a dev environment set up and I've been working on tests (it
takes *forever* for Mercurial to update and for the project to build, so
I've had a lot of time to slack off <https://xkcd.com/303/>).

However, I've run into an issue with the two methods
FileSystemProvider#newFileSystem(Path,Map) and
FileSystemProvider#newFileSystem(URI,Map). It's only mentioned in passing,
but from FileSystemProvider#getFileSystem(URI)'s docs: "File systems
created (sic) the newFileSystem(Path,Map) method are not returned by this
method." This causes issues for ZipFileSystemProvider as the only way to
acquire a "raw" ZipFileSystem to work with is through the method taking a
Path. I tried adding new FileSystems created through the Path param
overload to the provider's map, but that causes test failures in
ZipFSTester#test1(FileSystem). Should I try to slightly alter the behavior
of FileSystemProvider#newFileSystem(Path,Map), or should I just use a hacky
workaround in my test?

This is my current patch including tests:

Index: test/jdk/jdk/nio/zipfs/NestedZipFSTests.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- test/jdk/jdk/nio/zipfs/NestedZipFSTests.java (revision
59637+:9610e9b776ca+)
+++ test/jdk/jdk/nio/zipfs/NestedZipFSTests.java (revision
59637+:9610e9b776ca+)
@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.Test;
+
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.*;
+import java.util.Map;
+
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @summary Validate that a multi-level (nested) jar files can be opened
by URI
+ * @modules jdk.zipfs
+ * @run testng/othervm NestedZipFSTests
+ */
+public class NestedZipFSTests {
+
+    /**
+     * Validate that a 2-level jar file can be opened by URI
+     */
+    @Test
+    public void testOpens2LevelJar() throws Exception {
+        Path jar1Path = Path.of("jar1.jar");
+
+        try (FileSystem jar1 = FileSystems.newFileSystem(new URI("jar",
jar1Path.toUri() + "!/", null), Map.of("create", true))) {
+            Path jar2Path = jar1.getPath("jar2.jar");
+            try (FileSystem jar2 = FileSystems.newFileSystem(new
URI("jar", jar2Path.toUri() + "!/", null), Map.of("create", true))) {
+                Path nestedFilePath = jar2.getPath("test.txt");
+                Path newPath = Path.of(nestedFilePath.toUri());
+
+                assertEquals(nestedFilePath, newPath);
+            }
+        }
+    }
+
+    /**
+     * Validate that a 3-level jar file can be opened by URI
+     */
+    @Test
+    public void testOpens3LevelJar() throws Exception {
+        Path jar1Path = Path.of("jar1.jar");
+
+        try (FileSystem jar1 = FileSystems.newFileSystem(new URI("jar",
jar1Path.toUri() + "!/", null), Map.of("create", true))) {
+            Path jar2Path = jar1.getPath("jar2.jar");
+            try (FileSystem jar2 = FileSystems.newFileSystem(new
URI("jar", jar2Path.toUri() + "!/", null), Map.of("create", true))) {
+                Path jar3Path = jar2.getPath("jar3.jar");
+                try (FileSystem jar3 = FileSystems.newFileSystem(new
URI("jar", jar3Path.toUri() + "!/", null), Map.of("create", true))) {
+                    Path nestedFilePath = jar3.getPath("test.txt");
+                    Path newPath = Path.of(nestedFilePath.toUri());
+
+                    assertEquals(nestedFilePath, newPath);
+                }
+            }
+        }
+    }
+}
Index: src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
(revision 59637:9610e9b776cae5cbcdcce2e48dcf471ef7a8da1c)
+++ src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
(revision 59637+:9610e9b776ca+)
@@ -69,11 +69,11 @@
         try {
             // only support legacy JAR URL syntax  jar:{uri}!/{entry} for
now
             String spec = uri.getRawSchemeSpecificPart();
-            int sep = spec.indexOf("!/");
+            int sep = spec.lastIndexOf("!/");
             if (sep != -1) {
                 spec = spec.substring(0, sep);
             }
-            return Paths.get(new URI(spec)).toAbsolutePath();
+            return Path.of(new URI(spec)).toAbsolutePath();
         } catch (URISyntaxException e) {
             throw new IllegalArgumentException(e.getMessage(), e);
         }
@@ -134,7 +134,7 @@
     @Override
     public Path getPath(URI uri) {
         String spec = uri.getSchemeSpecificPart();
-        int sep = spec.indexOf("!/");
+        int sep = spec.lastIndexOf("!/");
         if (sep == -1)
             throw new IllegalArgumentException("URI: "
                 + uri

The "hacky workaround" I mentioned is the manual construction of Jar URIs
in the tests. I feel it could be improved and made easier to use, but it
compiles and the tests pass.

Thanks,
Mitchell


On Sun, Jun 7, 2020 at 3:40 AM Alan Bateman <Alan.Bateman at oracle.com> wrote:

> On 06/06/2020 20:12, Mitchell Skaggs wrote:
> > :
> >
> > I have a test, but I'm not sure where to put it and how to build
> > everything on Windows. Here's a "vanilla" version of the test that
> > demonstrates the issue:
> >
> I see Lance is providing advice on how to create a test (thanks Lance).
> I could imagine expanding the test coverage quite a bit to to cover the
> various scenarios that arise with N levels of nesting, particularly in
> cases where an enclosing file system is closed (and unregistered) and
> the behavior of subsequent calls to Path.of(URI). Thanks for working on
> this.
>
> -Alan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200609/6b3fd6da/attachment-0001.htm>