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?

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.

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.
>>>> 

Reply via email to