Re: 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.kejri...@oracle.com>; nio-...@openjdk.java.net

*Cc:* core-libs-dev <core-libs-dev@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.kejri...@oracle.com> <mailto:deepak.kejri...@oracle.com>

    Sent: Thursday, November 8, 2018 12:17 AM

    To: 'nio-...@openjdk.java.net 
<mailto:nio-...@openjdk.java.net>'<nio-...@openjdk.java.net> 
<mailto:nio-...@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