> 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 incrementally with one additional 
commit since the last revision:

  implement review suggestions:
   - reduce the toString() calls by creating a helper
   - fs.getPath("/") instead of fs.getRootDirectories().iterator().next()

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4604/files
  - new: https://git.openjdk.java.net/jdk/pull/4604/files/3971ad6f..8e77d289

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4604&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4604&range=01-02

  Stats: 18 lines in 1 file changed: 6 ins; 0 del; 12 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

Reply via email to