> Can I please get a review of this proposed fix for
> https://bugs.openjdk.java.net/browse/JDK-8251329?
>
> As noted in that issue, if a zip filesystem created on top of a jar
> containing a "./" entry is "walked" using the `Files.walkFileTree`, it leads
> to a infinite never ending iteration (which ultimately fails with Java heap
> space OOM).
>
> Alan notes in that issue that:
>
>> This is more likely an issue with the zipfs DirectoryStream implementation.
>> A DirectoryStream is specified to not include elements that for the special
>> links to the current or parent directory. It should be rare.
>
> This indeed turned out to be an issue in the
> `jdk.nio.zipfs.ZipDirectoryStream#iterator()` implementation where it calls
> the `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` implementation. The
> implementation, unlike what the javadoc of `java.nio.file.DirectoryStream`
> states, was including the "." and ".." paths in its returned iterator:
>
>> The elements returned by the iterator are in no specific order. Some file
> systems maintain special links to the directory itself and the directory's
> parent directory. Entries representing these links are not returned by the
> iterator.
>
>
> The proposed fix in this patch checks the paths for "." and "..", similar to
> what the `sun.nio.fs.UnixDirectoryStream` does in its `isSelfOrParent` and
> skips those paths from being added into the returned iterator. The
> `jdk.nio.zipfs.ZipFileSystem#iteratorOf(...)` (where this change has been
> done) is currently only used by `jdk.nio.zipfs.ZipDirectoryStream#iterator()`
> and has package-private visibility, so this change shouldn't impact any other
> usage/expectations.
>
> A new jtreg test has been added to reproduce this issue and verify the fix.
> Local testing of the `test/jdk/jdk/nio/` (including this new test) went fine
> without any issues after this change. I will be triggering a `tier1` test
> locally in a while.
Jaikiran Pai has updated the pull request with a new target base due to a merge
or a rebase. The incremental webrev excludes the unrelated changes brought in
by the merge/rebase. The pull request contains four additional commits since
the last revision:
- Merge latest from upstream master branch to bring in fixes that might help
fix the unrelated tier1 failures in Github Actions job runs
- implement review suggestions:
- reduce the toString() calls by creating a helper
- fs.getPath("/") instead of fs.getRootDirectories().iterator().next()
- implement review suggestion - move isSelfOrParent to ZipPath class
- 8251329: (zipfs) Files.walkFileTree walks infinitely if zip has dir named
"." inside
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4604/files
- new: https://git.openjdk.java.net/jdk/pull/4604/files/8e77d289..abae52bb
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4604&range=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4604&range=02-03
Stats: 10322 lines in 336 files changed: 6220 ins; 2802 del; 1300 mod
Patch: https://git.openjdk.java.net/jdk/pull/4604.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4604/head:pull/4604
PR: https://git.openjdk.java.net/jdk/pull/4604
