RFR: 8313765: Invalid CEN header (invalid zip64 extra data field size) [v2]
Volker Simonis
simonis at openjdk.org
Mon Aug 14 21:41:08 UTC 2023
On Mon, 14 Aug 2023 17:44:06 GMT, Lance Andersen <lancea at openjdk.org> wrote:
>> src/java.base/share/classes/java/util/zip/ZipFile.java line 1364:
>>
>>> 1362: * As the fields must appear in order, the block size indicates which
>>> 1363: * fields to expect:
>>> 1364: * 0 - May be written out by Ant and Apache Commons Compress Library
>>
>> I don't like that `isZip64ExtBlockSizeValid()` still accepts `0` as *valid* input. I think we should fully handle the zero case in `checkZip64ExtraFieldValues()` (also see my comments there).
>
> Hi Volker,
>
> I understand your point and I had done that previously but decided I did not like the flow of the code that way which is why I moved the check. I prefer to leave it as is.
I don't think this is a question of "taste" because `isZip64ExtBlockSizeValid()` suggests that the method will check for *valid* sizes and to my understanding `0` is not a valid input. This method might also be called from other places in the future which do not handle the zero case appropriately.
In any case, I'm ready to accept this as a case of "Disagree and Commit" :) but in that case please update at least the comment below to something like "*..Note we do not need to check blockSize is >= 8 as we know its length is at least 8 by now*" because "*..from the call to isZip64ExtBlockSizeValid()*" isn't true any more.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15273#discussion_r1293994175
More information about the core-libs-dev
mailing list