RFR: 8303923: ZipOutStream::putEntry should include an apiNote to indicate that the STORED compression method should be used when writing directory entries [v2]

[Eirik Bjorsnos]

On Sun, 19 Mar 2023 14:02:18 GMT, Lance Andersen <lancea at openjdk.org> wrote:

> Also the copyright should be changed to 2023 with your next update.

Imagine if jcheck could remind people of this, reviewers would be out of a job :-)

> src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 208:
> 
>> 206:      *
>> 207:      * This ensures strict compliance with the ZIP specification and
>> 208:      * allows optimal performance when processing directory entries.
> 
> I think we can remove at least the first part of the sentence regarding "strict compliance" as  "file data" can be interpreted as the contents of the file as 4.1.3 of the App.Note allows for the default compression method to be DEFAULT.  The intent of the apiNote is to remind developers that the use of the STORED compression method is preferred and may be more optimal

4.3.8  File data

      Immediately following the local header for a file
      SHOULD be placed the compressed or stored data for the file.
      If the file is encrypted, the encryption header for the file 
      SHOULD be placed after the local header and before the file 
      data. The series of [local file header][encryption header]
      [file data][data descriptor] repeats for each file in the 
      .ZIP archive. 

      Zero-byte files, directories, and other file types that 
      contain no content MUST NOT include file data.



My interpretation of section 4.3.8 is that 'file data' in the last sentence of this section refers to what is defined in the first sentence: 'Immediately following the local header for a file SHOULD be placed the compressed or stored data for the file.'

While I'm not sure I fully understand you interpretation, it seems you are saying that a DEFLATED entry with the two-byte 'empty final' DEFLATE blocks does not have file data? (Because it is just an encoding of 'no content')?

In any case, I'm happy to remove this since as it stands it is a bit vague  and as we've seen open for interpretation.

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

PR: https://git.openjdk.org/jdk/pull/12899