RFR: 4890732: GZIPOutputStream doesn't support, in fact thwarts, use of optional GZIP fields [v3]

[Lance Andersen]

On Mar 27, 2021, at 12:44 PM, Alan Bateman <alanb at openjdk.java.net<mailto:alanb at 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 at home]



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