On Thu, 15 Apr 2021 12:41:17 GMT, Lin Zang <lz...@openjdk.org> wrote:

>> 4890732: GZIPOutputStream doesn't support optional GZIP fields
>
> Lin Zang has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains ten additional commits since 
> the last revision:
> 
>  - Merge remote-tracking branch 'upstream/master' into gzip-field
>  - remove trailing spaces
>  - Use record and Builder pattern
>  - add class GZIPHeaderData, refine testcases
>  - update copyright
>  - reuse arguments constructor for non-argument one.
>  - add test case
>  - remove trailing spaces
>  - 4890732: support optional GZIP fields in GZIPOutputStream

Dear Lance, 
    Sorry for late response.
    
>  the updates to the PR does not include any proposed changes to 
> GZIPInputStream and this should be something we should come to an agreement 
> on as it can possibly impact the direction. I am not sure we need to add 
> multiple constructors to GZIPOutputStream as part of the proposed change.

With the latest change, the gzip header data could be composed by 
GZIPHeaderBuilder, But it seems ithere still need a way to write the header 
data when GZipOutputStream is created.Therefore IMHO at least one more 
constructor is required which could accept the header data and then write them.

For GZIPInputStream, as there is GZIPHeaderBuilder now, I think there is no 
need to add any constructor for it. We can use the builder to generate gzip 
header data when parsing the Gzip header.  For example, re-write the 
readHeader() to generate gzip header data, and provide api that user could 
access the header data.

> It would also be useful to know where is the actual pain point, that is, is 
> there a tool or API not having these fields settable for that is causing an 
> issue? I ask so that we can make sure that we are taking that into 
> consideration.

Frankly, I only experienced the use of extra gzip header field when work on the 
gziped heap dump file generated by  `jmap -dump` with `gz` option. It uses the 
extra field to provide some meta info that maybe used for decompressing.  I 
didn't see the usage of these header info in other place.  So I believe that 
not having these fields settable is OK  at present (at least).

The intention that I try to provide this PR is that I think, since the gzip 
file spec has described the header field, maybe in some scenario user may need 
a way to access/set it in Java.  

Thanks.
Lin

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

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

Reply via email to