RFR: JDK-8319122: Improve documentation of various Zip-file related APIs [v2]

Alan Bateman alanb at openjdk.org
Tue Nov 14 12:24:30 UTC 2023


On Fri, 10 Nov 2023 15:44:19 GMT, Yakov Shafranovich <duke at openjdk.org> wrote:

>> The various Zip/Jar-file related Java APIs have some long-standing differences or peculiarities with respect to the ZIP-file specification or compared to other implementations which should be documented in the API-doc. This documents the following:
>> - Cache of JAR files in JarURLConnection class
>> - Cache of JAR/ZIP files in JarFile and ZipFile classes
>> - Unexpected behavior when parsing ZIP files with duplicate entries in JarFile and ZipFile classes, as well as the zipfs provider
>> - Directories and filenames with the same name considered to be the same in ZipFile class
>> - Possible issues when local and central headers conflict in ZipInputStream class
>> 
>> Related JBS report:
>> https://bugs.openjdk.org/browse/JDK-8319122
>
> Yakov Shafranovich has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fixed more line breaks
>  - fixed line breaks

src/java.base/share/classes/java/util/zip/ZipInputStream.java line 77:

> 75:  *
> 76:  * Whenever possible, {@linkplain ZipFile} should be used for parsing ZIP
> 77:  * archives since it correctly reads data from the central directory.

I think it's okay to extend the existing API note to say that the stream may contain ZIP file entries that are not are not named in the central directory.  I think I would replace the sentence "Additionally ..." with "Consequently, a ZipInputStream that reads from a ZIP file may read ZIP file entries that are not in the ZIP file's central directory". 

The sentence "This class might also fail to properly parse ZIP archives that have prepended data" will confuse readers as it hints of unreliability in scenarios and begs too many questions on where the data is prepended. So I think drop this unless you can come up with something better.

Replacing the sentence "ZipFile may be used ..." is okay but I don't think the proposed text works. The API note starts with text to say that a ZipOutputStream doesn't read the CEN so saying "correctly reads from the central directory" is very confusing. ZipInputStream is a JDK 1.1 era API, it is what it is and would be disruptive to deprecate.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16424#discussion_r1392506679


More information about the security-dev mailing list