On Mar 27, 2021, at 12:44 PM, Alan Bateman 
<al...@openjdk.java.net<mailto:al...@openjdk.java.net>> wrote:


Dear Lance,

Sorry that I reply so late, I got stuck at some work recently.

* We should have tests that include some , but not all of the additional fields 
as I believe they are all optional according to the RFC.

I see Joe's comments in the CSR about the encode of the byte array of 
String-alike field such file name, I checked that the RFC has description it is 
"ISO8859-1". So do you think it is ok to change the argument type to String and 
add the encoding decription in the documented comments?

And Joe also suggested to make all optional header field as a class (I propose 
to name it "gzipHeaderMetaRecord" ). And then the constructor could accept it 
and process related flag and fields.  Moreover it seems this class cloud also 
be used in GzipInputStream,Do you think it is ok to also change it here?  
Thanks!


BRs,
Lin

GZIPInputStream/GZIPOutputStream are JDK 1.1 era classes that aren't well 
specified and I think we'll have to do some improvements as part of extending 
the support. For example, the GZIPOutputStream constructors (existing and 
proposed) do not specify that they write the member header.

That is a reasonable suggestion to indicate what is and is not done.

As regards the new constructors then I think new parameters will need to be 
clearly specified and/or linked to their description in the RFC. The types of 
some of the parameters may need to be re-examined, maybe the file name and 
comment should be provided as Strings (that will help with the discussion about 
the encoding to 8859_1).

Agree and that will also align it with ZipEntry(String), Zipentry::setComment

I think the javadoc will need to make it clear on what flags/values are allowed 
and the behavior when other values are used.

The Gzip spec is kind of vague in this area for example with the SI1 and SI2 
for the Extra Field only one value set is specified but I can imagine 
additional values being somewhere.

It might be that the class will need enums and other classes to provide a 
better API.

I am not sure how much if at all these extra header fields are used.  Certainly 
if the API now sets them, we should probably provide a way to access them as 
well.

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

PR: https://git.openjdk.java.net/jdk/pull/3072

[cid:E1C4E2F0-ECD0-4C9D-ADB4-B16CA7BCB7FC@home]



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com<mailto:lance.ander...@oracle.com>



Reply via email to