RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalFileAttributes [v5]

Jaikiran Pai jpai at openjdk.org
Sun Jun 30 04:51:31 UTC 2024


On Sat, 29 Jun 2024 18:24:46 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` to `ZipEntry.externalFileAttributes`.
>> 
>> This field was introduced in [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under the name `ZipEntry.posixPerms`. [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the field to `ZipEntry.extraAttributes` and extended its semantics to hold the full two-byte value of the `external file attributes` field, as defined by `APPNOTE.TXT`
>> 
>> The name `extraAttributes` is misleading. It has nothing to do with the `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although the name indicates it does.
>> 
>> To prevent confusion and make life easier for future maintainers, I suggest we rename this field to `ZipEntry.externalFileAttributes` and update related methods, parameters and comments accordingly.
>> 
>> While this change is a straightforward renaming, reviewers should consider whether it carries its weight, especially considering it might complicate future backports. 
>> 
>> As a note to reviewers, this PR includes the following intended updates:
>> 
>> - Rename `ZipEntry.extraAttributes` and any references to this field to `ZipEntry.externalFileAttributes`
>> - Update `JavaUtilZipFileAccess` to similarly rename methods to `setExternalFileAttributes` and `getExternalFileAttributes` 
>> - Rename the field `JarSigner.extraAttrsDetected` to `JarSigner.externalFileAttrsDetected`
>> - Rename a local variable in `JarSigner.writeEntry` to `externalFileAttributes`
>> - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to `externalFileAttributesDetected`
>> - Rename resource string key names in `s.s.t.jarsigner.Resources` from `extra.attributes.detected` to `external.file.attributes.detected`
>> - Rename method `SymlinkTest.verifyExtraAttrs` to `verifyExternalFileAttributes`, also updated two references to 'extra attributes' in this method
>> - Updated copyright in all affected files
>> 
>> If the resource file changes should be dropped and instead handled via `msgdop` updates, let me know and I can revert the non-default files.
>> 
>> I did a search across the code base to find 'extraAttrs', 'extra.attr' after these updates, and found nothing related to zip/jar. The `zip` and `jar` tests run clean:
>> 
>> 
>> make test TEST="test/jdk/java/util/jar"
>> make test TEST="test/jdk/java/util/zip"
>
> Eirik Bjørsnøs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits:
> 
>  - Merge branch 'master' into zipentry-external-attributes
>  - Rename SymlinkTest.verifyExternalAttrs to verifyExternalFileAttributes
>  - Rename ZipFileSystem.Entry.posixPerms to externalFileAttributes
>  - For clarity, use "externalFileAttributes" instead of "externalAttributes"
>  - Merge branch 'master' into zipentry-external-attributes
>  - Update copyright years for 2024
>  - Merge branch 'master' into zipentry-external-attributes
>  - Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes

Hello Eirik, I'll run some tests and review this PR this coming week.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16952#issuecomment-2198432408



More information about the security-dev mailing list