RFR: JDK-8197398 (zipfs) OutOfMemoryError when talking contents of empty JAR file

[Roger Riggs]

Hi Deepak,

A small change could avoid creating the array if it would be empty.
Move the array creation inside the if and compare (startIndex < endIndex).

Thanks, Roger




On 11/29/2018 05:10 AM, Deepak Kejriwal wrote:
>
> Hi Roger / Lance,
>
> Thanks for reviewing the fix. I have modified the fix as the review 
> comments given.
>
> Please find the below the updated webrev for fix.
>
> http://cr.openjdk.java.net/~rpatil/8199776/webrev.02/ 
> <http://cr.openjdk.java.net/%7Erpatil/8199776/webrev.02/>
> Regards,
> Deepak
>
> *From:*Roger Riggs
> *Sent:* Wednesday, November 28, 2018 3:15 AM
> *To:* Deepak Kejriwal <deepak.kejriwal at oracle.com>; 
> nio-dev at openjdk.java.net
> *Cc:* core-libs-dev <core-libs-dev at openjdk.java.net>
> *Subject:* Re: RFR: JDK-8197398 (zipfs) OutOfMemoryError when talking 
> contents of empty JAR file
>
> Hi,
>
> In ZipFileSystem.java, at 1071.
>
>  - The code will be more readable if you use character literal '/' 
> instead of 47.
>
>  - Save making an extra copy of the name array by testing for '/' 
> directly in the cen array
>    and create either the full name or the name without the '/'.
>
>  - 1073: The coding style should put the '{' on the same line as the 'if'.
>    Use the file's coding style.
>
> Thanks, Roger
>
> On 11/27/2018 04:51 AM, Deepak Kejriwal wrote:
>
>     Hi nio-dev team,
>
>       
>
>     Gentle reminder for review of below fix.
>
>       
>
>     Regards,
>
>     Deepak
>
>       
>
>     From: Deepak Kejriwal<deepak.kejriwal at oracle.com> <mailto:deepak.kejriwal at oracle.com>  
>
>     Sent: Thursday, November 8, 2018 12:17 AM
>
>     To: 'nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>'<nio-dev at openjdk.java.net> <mailto:nio-dev at openjdk.java.net>
>
>     Subject: RFR: JDK-8197398 (zipfs) OutOfMemoryError when talking contents of empty JAR file
>
>       
>
>     Hi all,
>
>       
>
>     Please help review the changes for JDK-8199776 to JDK8u:-
>
>       
>
>     Master Bug:https://bugs.openjdk.java.net/browse/JDK-8199776  
>
>     Webrev:http://cr.openjdk.java.net/~rpatil/8199776/webrev.00/
>     <http://cr.openjdk.java.net/%7Erpatil/8199776/webrev.00/>
>
>       
>
>       
>
>     Background:-
>
>     The Files.walkFileTree walk indefinitely while processing JAR file with "/" as a directory inside.
>
>       
>
>     Proposed fix:
>
>       
>
>     Files.walkFileTree api for jar internally uses ZipfileSystem. While iterating over the zip entries we have added a check to see if entry name is equal to "/" (absolute path). If the entry name is equal to "/" we don't add it to "inodes" map so that we don't end up with infinite loop.
>
>       
>
>     We have similar bug HYPERLINK"https://bugs.openjdk.java.net/browse/JDK-8197398"
>     <https://bugs.openjdk.java.net/browse/JDK-8197398>JDK-8197398 fixed in jdk 12. Below is the difference between fix for jdk 12 and jdk 8:-
>
>     The fix for above issue for jdk 12 handle a case where we can update an existing entry in jar with "/" (absolute path).  As in case of jdk 8 we cannot update the existing zipentry as doing so we get "FileAlreadyExistsException". Therefore, fix for jdk 8u does not consider this case.
>
>       
>
>     Regards,
>
>     Deepak
>
>       
>