Nested ZipFileSystems cause issues with URI parsing
Lance Andersen
lance.andersen at oracle.com
Fri Jun 12 12:22:42 UTC 2020
I created a webrev based on Mitchell’s proposed changes to make it easier for everyone to start to review. It can be found at: http://cr.openjdk.java.net/~lancea/8247441/webrev.00/ <http://cr.openjdk.java.net/~lancea/8247441/webrev.00/>
> On Jun 9, 2020, at 5:24 AM, Mitchell Skaggs <skaggsm333 at gmail.com> wrote:
>
> 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.
Correct currently that is the expected behavior at this time. A change here would require more discussion and if it was desired would require thought of backward compatibility and have to run through the CSR process
> 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?
Best
Lance
>
> 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 <http://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 <mailto: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
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200612/0bcb4189/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20200612/0bcb4189/oracle_sig_logo-0001.gif>
More information about the nio-dev
mailing list