<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;
mso-fareast-language:EN-US;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="DE" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-US">here’s an update of my webrev: <a href="http://cr.openjdk.java.net/~clanger/webrevs/8213031.1/">
http://cr.openjdk.java.net/~clanger/webrevs/8213031.1/</a> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">I added synchronization to the updating of
</span><span lang="EN-US">permCache</span><span lang="EN-US"> in ZipUtils.java to avoid concurrent modifications.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">As per request from Alan, I’m adding security-dev to get a review from security perspective.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Thanks<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Christoph<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="mso-fareast-language:DE">From:</span></b><span lang="EN-US" style="mso-fareast-language:DE"> Langer, Christoph
<br>
<b>Sent:</b> Freitag, 26. Oktober 2018 17:13<br>
<b>To:</b> core-libs-dev <core-libs-dev@openjdk.java.net>; nio-dev <nio-dev@openjdk.java.net>; 'Xueming Shen' <xueming.shen@oracle.com><br>
<b>Cc:</b> Schmelter, Ralf <ralf.schmelter@sap.com>; 'Volker Simonis' <volker.simonis@gmail.com><br>
<b>Subject:</b> RFR 8213031: Enhance jdk.nio.zipfs to support Posix File Permissions<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-US">please review this enhancement of jdk.nio.zipfs to support Posix file permissions.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Webrev: <a href="http://cr.openjdk.java.net/~clanger/webrevs/8213031.0/">
http://cr.openjdk.java.net/~clanger/webrevs/8213031.0/</a><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8213031">
https://bugs.openjdk.java.net/browse/JDK-8213031</a><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">I had already posted this change as part of a larger fix for “6194856: Zip Files lose ALL ownership and permissions of the files” [1]. With the original proposal I was also addressing the java.util.zip API and the jar
tool. However, I’ve decided to split this endeavor into 2 parts. While I still want to follow up on java.util.zip and jar, I’d like to bring the enhancement to jdk.zipfs in first. Both places don’t have dependencies to each other and since zipfs does not change
an external API, I guess the bar here is lower. Maybe it is even a candidate for down-porting to jdk11u in the future.</span><span lang="EN-US" style="font-size:8.0pt;font-family:"Arial",sans-serif;color:#666666;mso-fareast-language:DE"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">After my fix, zipfs will support the PosixFileAttributeView. Posix file attributes can’t be fully supported since owner and group are not handled in zip files. So methods supposed to get these attributes will return an
UnsupportedOperationException. But the posix permissions will now be correctly handled, that is stored into and read from the zip file.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">@Sherman:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Following suggestions from your review, I removed the test with the binary zip file. I initially thought it is a good idea to test on a well-known file. However, testWriteAndReadArchiveWithPosixPerms tests both, writing
a zip file and reading it again so I guess test coverage is quite complete here.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Furthermore, I made some public declarations in ZipUtils package private which should suffice.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">I also tried to address your performance concerns with permsToFlags and permsFromFlags. In permsToFlags I will now simply iterate to the set of permissions and add the flags to the return value. It’s probably more performant
than the streaming approach and doesn’t look much worse in the code. In permsFromFlags I added a cache of permission sets which should avoid constant calls to the streaming. However, as the return value needs to be mutable, I have to clone the cached permissions.
Maybe it would have the same or even better performance if we iteratively fill a new set of permissions each time permsToFlags gets called. What do you think?<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Do we need a CSR for this patch?<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Thanks & Best regards<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Christoph<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">[1] <a href="http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-October/055971.html">
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-October/055971.html</a><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
</div>
</div>
</body>
</html>