Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-13 Thread Michael Hall
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, -  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  > 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
>> 



Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-13 Thread Alan Bateman
On Sun, 12 May 2024 21:44:56 GMT, Alan Snyder  wrote:

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

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106776812


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread -
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  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, -  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 
> wrote:
>
>> > 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
>>
>>
>>
>>
>>
>> > On May 12, 2024, at 7:32 AM, Chen Liang  wrote:
>> >
>> > On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:
>> >
>> >> Add API to access ZipEntry.extraAttributes
>> >
>> > 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.
>> >
>> > -
>> >
>> > PR Comment:
>> https://git.openjdk.org/jdk/pull/19204#issuecomment-2106265843
>> >
>>
>>
>


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread -
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  wrote:

> > 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
>
>
>
>
>
> > On May 12, 2024, at 7:32 AM, Chen Liang  wrote:
> >
> > On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:
> >
> >> Add API to access ZipEntry.extraAttributes
> >
> > 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.
> >
> > -
> >
> > PR Comment:
> https://git.openjdk.org/jdk/pull/19204#issuecomment-2106265843
> >
>
>


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Alan Snyder
> 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





> On May 12, 2024, at 7:32 AM, Chen Liang  wrote:
> 
> On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:
> 
>> Add API to access ZipEntry.extraAttributes
> 
> 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.
> 
> -
> 
> PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106265843
> 



Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Chen Liang
On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:

> Add API to access ZipEntry.extraAttributes

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.

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106265843


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Alan Bateman
On Sun, 12 May 2024 09:48:42 GMT, xiaotaonan  wrote:

> 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

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106260883


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Chen Liang
On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:

> Add API to access ZipEntry.extraAttributes

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](https://en.wikipedia.org/wiki/ZIP_(file_format)#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 
core-libs-dev@openjdk.org.

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106251411


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Chen Liang
On Sun, 12 May 2024 09:48:42 GMT, xiaotaonan  wrote:

>> Add API to access ZipEntry.extraAttributes
>
>> /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.

@xiaotaonan Since you appear confused, "discussion on core libs" means 
discussion on core-libs-dev@openjdk.org 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.

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106237028


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread xiaotaonan
On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:

> Add API to access ZipEntry.extraAttributes

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

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106188354


Re: RFR: 8322332: Add API to access ZipEntry.extraAttributes

2024-05-12 Thread Alan Bateman
On Sun, 12 May 2024 02:48:31 GMT, xiaotaonan  wrote:

> Add API to access ZipEntry.extraAttributes

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.

-

PR Comment: https://git.openjdk.org/jdk/pull/19204#issuecomment-2106139645