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

[Lance Andersen]

Hi Deepak

Looks OK.   Thank you for your efforts


> On Dec 3, 2018, at 9:25 AM, Deepak Kejriwal <deepak.kejriwal at oracle.com> wrote:
> 
> Hi Roger,
>  
> Thanks for review. I have modified the code to avoid creation of empty array.
>  
> Please find the below the updated webrev for fix.
> http://cr.openjdk.java.net/~rpatil/8199776/webrev.03/ <http://cr.openjdk.java.net/~rpatil/8199776/webrev.03/>
>  
> Regards,
> Deepak
>  
> From: Roger Riggs 
> Sent: Thursday, November 29, 2018 10:16 PM
> To: Deepak Kejriwal <deepak.kejriwal at oracle.com>; Lance Andersen <lance.andersen 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 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> <mailto:deepak.kejriwal at oracle.com>; nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>
> Cc: core-libs-dev <core-libs-dev at openjdk.java.net> <mailto: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 <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
>  
>  
>  
>  

 <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: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20181203/97827c0a/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oracle_sig_logo.gif
Type: image/gif
Size: 658 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20181203/97827c0a/oracle_sig_logo-0001.gif>