On Tue, 14 Nov 2023 12:21:42 GMT, Alan Bateman <[email protected]> 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
