RFR: 8250968: Symlinks attributes not preserved when using jarsigner on zip files

[Seán Coffey]

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