RFR: 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named "." inside [v4]

[Lance Andersen]

On Jul 2, 2021, at 12:13 PM, Lance Andersen <lance.andersen at oracle.com<mailto:lance.andersen at oracle.com>> wrote:



On Jul 2, 2021, at 8:08 AM, Jaikiran Pai <jai.forums2013 at gmail.com<mailto:jai.forums2013 at gmail.com>> wrote:

Hello Lance,

On 02/07/21 4:42 pm, Lance Andersen wrote:
Hi Jaikiran,

Consider:


        try (var os = Files.newOutputStream(ZIPFILE);
             ZipOutputStream zos = new ZipOutputStream(os)) {
            zos.putNextEntry(new ZipEntry("../Hello.txt"));
            zos.write("Hello World".getBytes(StandardCharsets.UTF_8));
        }


With your proposed fix, you will only return "/" when you walk the the Zip file and we should also return "/Hello.txt"

Thank you for noticing this issue in my change and bringing this up. I have a question around this use case. Please consider a small variation to your example as below:

try (var os = Files.newOutputStream(ZIPFILE);
             ZipOutputStream zos = new ZipOutputStream(os)) {
            zos.putNextEntry(new ZipEntry("../Hello.txt"));
            zos.write("Hello World".getBytes(StandardCharsets.UTF_8));
            zos.closeEntry();
            zos.putNextEntry(new ZipEntry("/Hello.txt"));
            zos.write("Bye bye".getBytes(StandardCharsets.UTF_8));
            zos.closeEntry();

        }

Notice that I now have a zip/jar which has 2 differently named entries "../Hello.txt" and "/Hello.txt". This creates the archive without any issues and those 2 entries are noted in its listing. Now assuming someone walks over this jar using the ZipFileSystem, starting at root ("/"), what should be the expected output? The path(s) returned will end up being "/" and "/Hello.txt" but which resource is expected to be served in this case? The one which has "Hello World" in it or the one which has "Bye bye"? This example can be extended a bit more by introducing a "./Hello.txt", in the jar, with yet another different content in that entry. Is there some specification for this that I can check and adapt the test case accordingly?


Consider Hello.zip created via:


try (var os = Files.newOutputStream(Path.of("Hello.zip"));
     ZipOutputStream zos = new ZipOutputStream(os)) {
    zos.putNextEntry(new ZipEntry("../Hello.txt"));
    zos.write("Hello".getBytes(StandardCharsets.UTF_8));
    zos.putNextEntry(new ZipEntry("Hello.txt"));
    zos.write("Another Hello".getBytes(StandardCharsets.UTF_8));

}

Winzip does not handle this case well.

InfoZip will :

-------------
unzip ../Hello.zip
Archive:  ../Hello.zip
warning:  skipped "../" path component(s) in ../Hello.txt
  inflating: Hello.txt
replace Hello.txt? [y]es, [n]o, [A]ll, [N]one, [r]ename: r
new name: foo.txt
  inflating: foo.txt
% ls
Hello.txt       foo.txt
 % cat Hello.txt
Hello
 % cat foo.txt
Another Hello
%
————————

This scenario is not well covered in the Zip spec, so we can do what we think is best.

 Personally, I am OK with resolving the path.  The odds of having multiple relative paths to the same file would be a mistake in creating the Zip.

That being said, if we want to follow Alan’s suggestion and throw an Exception, I am OK with that as well.

Either way, we currently cannot access the file via Zip FS due to the call to ZipPath::getResolvedPath() for all access and the path is only normalized when the Inodes are created.

Alan, do you have a specific preference?

One more datapoint

Consider:


try (FileSystem zipfs =
             FileSystems.newFileSystem(Path.of("ZipFSHello.zip"), Map.of("create", "true"))) {
    Files.writeString(zipfs.getPath("HelloZipfs.txt"), "Hello");
    Files.writeString(zipfs.getPath("../HelloZipfs.txt"), "A ZipFS Hello");

}


The above will result in a Zip with a single entry of “HelloZipfs.txt” and a value of “A ZipFS Hello”

As mentioned in my previous comment, this is due to the use of ZipPath::getResolvedPath() which brings me back to the suggestion of doing the same when we create the Inodes given this is an a virtual FS.

HTH

Best
Lance


-Jaikiran




<oracle_sig_logo.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>

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC at home]



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>