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

Yakov Shafranovich duke at openjdk.org
Wed Nov 22 22:26:08 UTC 2023


On Tue, 14 Nov 2023 12:21:42 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> 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 ZipInputStream 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.

Thank you, I will work on this

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

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



More information about the security-dev mailing list