Thanks for the review Max. Comments inline..
On 27/08/2020 14:45, Weijun Wang wrote:
I’m OK with using one warning, but prefer it to a little more formal like
"POSIX file permission and/or symlink attributes detected…”.
One nit in ZipFile.java:
1098 // only set posix perms value via ZipEntry constructor for
now
1099 @Override
1100 public int getExtraAttributes(ZipEntry ze) {
Maybe you can just remove the comment.
Do you also want to rename the “posixPermsDetected" field and loacl variable
“perms” in JarSigner.java?
Good points. Edits made.
http://cr.openjdk.java.net/~coffeys/webrev.8250968.v3/webrev/
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.
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.
regards,
Sean.
No other comment.
Thanks,
Max
On Aug 27, 2020, at 3:26 AM, Seán Coffey <sean.cof...@oracle.com> wrote:
updated webrev:
http://cr.openjdk.java.net/~coffeys/webrev.8250968.v2/webrev/
regards,
Sean.
On 27/08/2020 07:42, Seán Coffey wrote:
Hi Max,
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 ?
+ {"posix.attributes.detected", "POSIX file permission attributes detected.
These attributes are ignored when signing and are not protected by the signature."},
replace with:
+ {"extra.attributes.detected", "POSIX file permission/symlink attributes
detected. These attributes are ignored when signing and are not protected by the signature."},
regards,
Sean.
On 26/08/2020 23:15, Weijun Wang wrote:
Are you going to update the warning text or create a new one?
Thanks,
Max
On Aug 26, 2020, at 2:26 PM, Seán Coffey <sean.cof...@oracle.com> wrote:
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.
I haven't been able to automate a test for this since zipfs doesn't seem to
support symlinks. Manual testing looks good.
https://bugs.openjdk.java.net/browse/JDK-8250968
http://hmsjpse.uk.oracle.com/home/scoffey/ws/jdk-jdk/open/webrev.8250968/webrev/index.html
regards,
Sean.
