RFR: 8303866: Allow ZipInputStream.readEnd to parse small Zip64 ZIP files [v12]
Eirik Bjørsnøs
eirbjo at openjdk.org
Tue Jan 16 14:37:33 UTC 2024
On Tue, 16 Jan 2024 13:42:30 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> src/java.base/share/classes/java/util/zip/ZipInputStream.java line 534:
>>
>>> 532:
>>> 533: long csize = get32(tmpbuf, LOCSIZ);
>>> 534: long size = get32(tmpbuf, LOCLEN);
>>
>> Hello Eirik, I suspect this part of the change has an issue. Before reading the `tmpbuf` for compressed and uncompressed sizes, there will be 32 bits of CRC, which should be read first. This now skips those 32 CRC bits and reads them (in the else block) after reading these sizes and that can cause incorrect LOC data.
>
> The Github actions job which runs tier1 is all successful with this proposed change. So I'm a bit surprised that the tests didn't catch any issues, which makes me wonder if we have enough test coverage that covers this change.
> Hello Eirik, I suspect this part of the change has an issue. Before reading the `tmpbuf` for compressed and uncompressed sizes, there will be 32 bits of CRC, which should be read first. This now skips those 32 CRC bits and reads them (in the else block) after reading these sizes and that can cause incorrect LOC data.
How does the order of the reads from the byte array matter? Outside cache efficiency I would presume they could be read in any order?
(These reads are from a temp buffer, not the stream)
Could you elaborate? I'm not sure I'm following :-)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12524#discussion_r1453516319
More information about the core-libs-dev
mailing list