Nested ZipFileSystems cause issues with URI parsing

[Mitchell Skaggs]

Alan,
> The file system API doesn't define a format for so-called compound URIs.
Okay, I just based my fix on the comment describing the syntax as "
jar:{uri}!/{entry}".
> if helps the more general case of N-levels of nesting
I believe it does resolve the issues with more than 1 level of nesting.
> there may be other other in the zipfs provider that will need attention
too
Thanks for reminding me, I included another change in my patch below.

Lance,
> please generate a diff  and a concise test
Here's a patch for the fix:

diff --git
a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java
@@ -69,7 +69,7 @@
         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);
             }
@@ -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

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:

Path zip1Path = Files.createTempFile("zip1", ".zip");
try {
    Files.delete(zip1Path);
    try (FileSystem zip1 = FileSystems.newFileSystem(zip1Path,
Map.of("create", true))) {
        Path zip2Path = zip1.getPath("zip2.zip");
        try (FileSystem zip2 = FileSystems.newFileSystem(zip2Path,
Map.of("create", true))) {
            Path nestedFilePath = zip2.getPath("test.txt");
            Path newPath = Path.of(nestedFilePath.toUri());
            assert nestedFilePath.equals(newPath);
        }
    }
} finally {
    try (Stream<Path> walk = Files.walk(zip1Path)) {
        walk.sorted(Comparator.reverseOrder())
                .forEach(path -> {
                    try {
                        Files.delete(path);
                    } catch (IOException e) {
                        throw new Error(e);
                    }
                });
    }
}

It builds a nested zip file in the system's temp directory, verifies the
path is parsed correctly, and deletes it when finished.

If you need me to build and test everything, I can transfer it to a linux
machine and do it there; it'll just take some time to get a build
environment set up.

Thanks,
Mitchell

On Sat, Jun 6, 2020 at 11:07 AM Alan Bateman <Alan.Bateman at oracle.com>
wrote:

> On 06/06/2020 09:17, Mitchell Skaggs wrote:
>
> Hi everyone,
>
> I ran into a bit of a complex/niche issue, and I'm not surprised it wasn't
> caught or brought up before. I have a situation brought on by Spring's
> Uber-jars where a classpath resource is located inside of two jar files.
> The standard getResourceAsStream, etc. works just fine because of a
> clever implementation of URLStreamHandler that does some zipfile magic to
> find the file offsets inside nested zips. Unfortunately, I need something
> more powerful than an InputStream - I need java.nio.Paths - so I instead
> use getResource and recursively consume that URI, opening/closing
> FileSystems as needed. I expected that to work fine, but due to the way the
> URI is processed by ZipFileSystem, it looks in the wrong place.
>
> The cause of (and fix for) the issue:
>
> After tracing through the code, here's what I believe to be the crux of
> the issue: the ZipFileSystemProvider.getPath(URI) method. That method
> uses String.indexOf on the string representation of the URI. Because of
> the structure of jar-scheme URIs (link
> <https://www.iana.org/assignments/uri-schemes/prov/jar>), it should
> instead use String.lastIndexOf because the first "jar:" scheme is always
> paired with the last "!/" separator. I believe this is a bug, and fixing
> it wouldn't change behavior in situations without nested jar files.
>
> The file system API doesn't define a format for so-called compound URIs.
> You'll find more on this in the Path::toUri javadoc where expectations that
> are set that invoking toUri() on a path to a file in a nested file system
> may not return a URI that encodes the URI of the enclosing file system.
> That said, the URI support for nesting of zip/JAR file systems does work
> for one level of nesting and if helps the more general case of N-levels of
> nesting then we should fix it. In addition to the getPath(URI)
> implementation, there may be other other in the zipfs provider that will
> need attention too. A good set of tests should help shake out any other
> issues.
>
> -Alan.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200606/0d8619be/attachment-0001.htm>