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

[Roger Riggs]

Hi Deepak,

Looks fine.  :)

Thanks, Roger


On 12/03/2018 09:25 AM, Deepak Kejriwal 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/%7Erpatil/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  
>
>           
>
>         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
>
>           
>
>           
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20181203/3e00d88a/attachment-0001.html>