<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Sean,<div class=""><br class=""></div><div class="">I think the changes are OK in the latest version. A couple of the files have a 2019 copyright still.<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Aug 27, 2020, at 10:58 AM, Weijun Wang <<a href="mailto:weijun.wang@oracle.com" class="">weijun.wang@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class="">On Aug 27, 2020, at 10:36 AM, Seán Coffey <<a href="mailto:sean.coffey@oracle.com" class="">sean.coffey@oracle.com</a>> wrote:<br class=""><br class="">Thanks for the review Max. Comments inline..<br class=""><br class="">On 27/08/2020 14:45, Weijun Wang wrote:<br class=""><blockquote type="cite" class="">I’m OK with using one warning, but prefer it to a little more formal like "POSIX file permission and/or symlink attributes detected…”.<br class=""><br class="">One nit in ZipFile.java:<br class=""><br class="">1098 // only set posix perms value via ZipEntry constructor for now<br class="">1099 @Override<br class="">1100 public int getExtraAttributes(ZipEntry ze) {<br class=""><br class="">Maybe you can just remove the comment.<br class=""><br class="">Do you also want to rename the “posixPermsDetected" field and loacl variable “perms” in JarSigner.java?<br class=""></blockquote><br class="">Good points. Edits made.<br class=""><br class=""><a href="http://cr.openjdk.java.net/~coffeys/webrev.8250968.v3/webrev/" class="">http://cr.openjdk.java.net/~coffeys/webrev.8250968.v3/webrev/</a><br class=""><br class=""><blockquote type="cite" class=""><br class="">I’m not sure about the test but if zipfs is able to keep permissions inside a zip file then that POSIX byte (or whatever it’s named) is already there and we can modify it to include more bits.<br class=""></blockquote><br class="">Looks like it was a conscious design decision to only allow recording of POSIX permission bits for this field (& 0xFFF). I don't see anything about symlink support in zipfs docs.</blockquote></div></blockquote><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">As long as that *byte* is there and it’s not difficult to locate, we can manually add the *bit* for symlink and see if jarsigner can keep it.</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""></div></blockquote><div><br class=""></div>We can create an RFE for adding support for this with Zip FS.<br class=""><blockquote type="cite" class=""><div class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;" class="">—Max</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><br class="">regards,<br class="">Sean.<br class=""><br class=""><blockquote type="cite" class=""><br class="">No other comment.<br class=""><br class="">Thanks,<br class="">Max<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Aug 27, 2020, at 3:26 AM, Seán Coffey <<a href="mailto:sean.coffey@oracle.com" class="">sean.coffey@oracle.com</a>> wrote:<br class=""><br class="">updated webrev:<br class=""><a href="http://cr.openjdk.java.net/~coffeys/webrev.8250968.v2/webrev/" class="">http://cr.openjdk.java.net/~coffeys/webrev.8250968.v2/webrev/</a><br class=""><br class="">regards,<br class="">Sean.<br class=""><br class="">On 27/08/2020 07:42, Seán Coffey wrote:<br class=""><blockquote type="cite" class="">Hi Max,<br class=""><br class="">I looked at updating the warning string but figured that it might have been of no interest to end users. How about this edit then ?<br class=""><br class="">+ {"posix.attributes.detected", "POSIX file permission attributes detected. These attributes are ignored when signing and are not protected by the signature."},<br class=""><br class=""><blockquote type="cite" class=""><blockquote type="cite" class="">replace with:<br class=""></blockquote></blockquote>+ {"extra.attributes.detected", "POSIX file permission/symlink attributes detected. These attributes are ignored when signing and are not protected by the signature."},<br class=""><br class="">regards,<br class="">Sean.<br class=""><br class="">On 26/08/2020 23:15, Weijun Wang wrote:<br class=""><blockquote type="cite" class="">Are you going to update the warning text or create a new one?<br class=""><br class="">Thanks,<br class="">Max<br class=""><br class=""><blockquote type="cite" class="">On Aug 26, 2020, at 2:26 PM, Seán Coffey <sean.coffey@oracle.com> wrote:<br class=""><br class="">This is a follow on from the recent 8218021 fix. The jarsigner tool removes symlink attribute data from zipfiles when signing them. jarsigner should preserve this data. The fix involves preserving the 16 bits associated with the file attributes (instead of the current 12). That's done in ZipFile. All other changes are just a refactor of the variable name.<br class=""><br class="">I haven't been able to automate a test for this since zipfs doesn't seem to support symlinks. Manual testing looks good.<br class=""><br class="">https://bugs.openjdk.java.net/browse/JDK-8250968<br class="">http://hmsjpse.uk.oracle.com/home/scoffey/ws/jdk-jdk/open/webrev.8250968/webrev/index.html<br class=""><br class="">regards,<br class="">Sean.</blockquote></blockquote></blockquote></blockquote></blockquote></blockquote></div></blockquote></div><br class=""><div class="">
<div style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><span><br class="">Best<br class="">Lance<br class=""></span></div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;"><div class=""><span>------------------</span></div></span><br class="Apple-interchange-newline"><span><img apple-inline="yes" id="1DB4C05B-614F-4F59-9A41-183376584B90" src="cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home" class=""></span><div style="font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><br class="Apple-interchange-newline"><br class="">Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037<br class="">Oracle Java Engineering <br class="">1 Network Drive <br class="">Burlington, MA 01803<br class=""><a href="mailto:Lance.Andersen@oracle.com" class="">Lance.Andersen@oracle.com</a></div><div style="font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=""><br class=""></div><br class="Apple-interchange-newline" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: Helvetica; font-size: 18px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;"><br class="Apple-interchange-newline">
</div>
<br class=""></div></body></html>