8322332: Add API to access ZipEntry.extraAttributes

[xiaotaonan]

Add API to access ZipEntry.extraAttributes

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires a CSR request matching fixVersion 23 to be approved (needs to be created)

Issue

• JDK-8322332: Add API to access ZipEntry.extraAttributes (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19204/head:pull/19204
$ git checkout pull/19204

Update a local copy of the PR:
$ git checkout pull/19204
$ git pull https://git.openjdk.org/jdk.git pull/19204/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19204

View PR using the GUI difftool:
$ git pr show -t 19204

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19204.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back xiaotaonan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@xiaotaonan The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (ff9001e2)

[AlanBateman]

I think this will require discussion on core libs before proposing APIs in this area. I think a starting point would be explain how you might use this, esp. with file permissions and sym links. Due to potential mis-use, I think, as before, have to be very cautious about introducing an API that provides access to the raw bits.

[AlanBateman]

/csr

[openjdk]

@AlanBateman has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@xiaotaonan please create a CSR request for issue JDK-8322332 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[xiaotaonan]

/csr
@AlanBateman

This issue was reported by a person named Alan Snyder, I don't have his or her contact information, how to create a CSR in this situation.

[liach]

@xiaotaonan Since you appear confused, "discussion on core libs" means discussion on [email protected] mailing list https://mail.openjdk.org/mailman/listinfo/core-libs-dev where we can agree on what is a safe and future-proof way to expose the extraAttributes.

The csr command is just routinely issued for any patch that changes the Java APIs to protect against accidentally changing the public API. You shouldn't submit your CSR until you are sure the API surface (including method signatures + API Documentation/Javadoc) are settled down. If anything changes, you must re-draft your CSR to go through the CSR review process again.

Also, if you want to update a pull request, you can just push more commits; you don't have to force-push or delete your branch. When a pull request is integrated, openjdk bot will automatically squash and rename the squashed commit to your bug title, so please don't spam pull requests whenever you push a code update.

[liach]

For example, with a quick check, you can find that this field itself is not a good candidate for direct setter exposure:

1. For context, this is part of the central directory file header for each Zip entry.
2. This is read from bytes 40-41 (most significant 2 bytes of "External file attributes") if the byte 5 is 3 (most significant byte of "Version made by"); it's stored in the same way.
3. Besides valid unsigned short values, this field also has a value of -1 (or all negative values) indicating this field is absent.

Thus, a direct setter doesn't indicate the restriction that the field is 2-byte and is optional.

A getter might be fine, but given there are other ZIP fields that we don't expose (such as bytes 36-37, 38-41) it's dubious whether such exposure would be needed at all. After all, we should continue the discussion on [email protected].

[AlanBateman]

This issue was reported by a person named Alan Snyder, I don't have his or her contact information, how to create a CSR in this situation.

He came to core-libs-dev in Dec 2023 [1] to discuss this. The context at the time was symbolic links. It might be interesting to explore that in the context of the zip file system provider, less sure about the java.util.zip APIs. I assume he created JDK-8322332 after that discussion to at least have something in JBS.

[1] https://mail.openjdk.org/pipermail/core-libs-dev/2023-December/116723.html

[liach]

Also see #16952, another patch that renames this field, which is planning for an integration soon; this PR will be out-of-date when that one is integrated.

[mlbridge]

Mailing list message from Alan Snyder on core-libs-dev:

It might be interesting to explore that in the context of the zip file system provider, less sure about the java.util.zip APIs.

I was not using the Zip file system. I was processing a Zip file.

Alan

[mlbridge]

Mailing list message from - on core-libs-dev:

Hi Alan Snyder,
Currently, JDK ZipEntry populates that extraAttributes (proposed to rename
to externalFileAttributes in JDK-8321274, PR #16952) only if the high byte
of the "version made by" field is 3 (indicating Linux compatibility) and
only records the most significant 2 bytes for the copied ZipEntry.
Since this is not the "external file attributes" field from the ZIP format,
do you still think this field should be exposed, or should we create a new
field to properly model the external file attributes? Also worth noting is
the support for adjusting the "version made by" field, the "external file
attributes" field might be misinterpreted without it.

P.S. Seems the Skara bridge is not propagating emails to GitHub comments.

On Sun, May 12, 2024 at 2:17?PM Alan Snyder <javalists at cbfiddle.com> wrote:

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240512/7384290b/attachment.htm>

[AlanBateman]

I was not using the Zip file system. I was processing a Zip file.

They are equivalent, the suggestion to look at the sym link support in the zip file system provider is that it's a much better fit for this extension. It already has optional support for POSIX file permissions and the APIs for dealing with sym links.

[mlbridge]

Mailing list message from - on core-libs-dev:

Hi Mike,
I think this particular field has been renamed a few times; and ZipEntry
only exposes part of the ZIP format's fields. So most likely there isn't
sufficient information for most 3rd-party ZIP processing libraries. I
personally don't really use ZipFile so am not quite sure of the recent API
changes either.

On Sun, May 12, 2024 at 6:10?PM Michael Hall <mik3hall at gmail.com> wrote:

I haven?t looked at any of this for sometime. But as I recall there was a
possibility that other 3rd party applications might also be making use of
these fields? IIRC there was some support for chaining multiple uses? Or
the api may of changed and these aren?t the same fields or for some other
reason what I remember is out of date.

Mike

On May 12, 2024, at 3:56?PM, - <liangchenblue at gmail.com> wrote:

Hi Alan Snyder,
Currently, JDK ZipEntry populates that extraAttributes (proposed to rename
to externalFileAttributes in JDK-8321274, PR #16952) only if the high byte
of the "version made by" field is 3 (indicating Linux compatibility) and
only records the most significant 2 bytes for the copied ZipEntry.
Since this is not the "external file attributes" field from the ZIP format,
do you still think this field should be exposed, or should we create a new
field to properly model the external file attributes? Also worth noting is
the support for adjusting the "version made by" field, the "external file
attributes" field might be misinterpreted without it.

P.S. Seems the Skara bridge is not propagating emails to GitHub comments.

On Sun, May 12, 2024 at 2:17?PM Alan Snyder <javalists at cbfiddle.com>
wrote:

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240512/bff0cd46/attachment.htm>

[mlbridge]

Mailing list message from Michael Hall on core-libs-dev:

I was thinking of zip api?s other than java?s. The field needs to be at a fixed place in the file format whatever the name? Unless a significant api change has been made.

A couple of links from googling on ?zip extra field chaining"

https://libzip.org/specifications/extrafld.txt
https://www.hackerfactor.com/blog/index.php?/archives/405-Keeping-Zip.html

If this were strictly jar files I would be less concerned but zip is widely used outside of java so more chances for conflict where java doesn?t have anything like ownership.
At the time I looked at it the provision for ?user? additions was very limited again making conflict more likely.

On May 12, 2024, at 9:32?PM, - <liangchenblue at gmail.com> wrote:

Hi Mike,
I think this particular field has been renamed a few times; and ZipEntry only exposes part of the ZIP format's fields. So most likely there isn't sufficient information for most 3rd-party ZIP processing libraries. I personally don't really use ZipFile so am not quite sure of the recent API changes either.

On Sun, May 12, 2024 at 6:10?PM Michael Hall <mik3hall at gmail.com <mailto:mik3hall at gmail.com>> wrote:

I haven?t looked at any of this for sometime. But as I recall there was a possibility that other 3rd party applications might also be making use of these fields? IIRC there was some support for chaining multiple uses? Or the api may of changed and these aren?t the same fields or for some other reason what I remember is out of date.

Mike

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240513/d4d768df/attachment.htm>