On 9/1/14 9:05 AM, Alan Bateman wrote:
On 29/08/2014 20:12, Martin Buchholz wrote:
Hi Xueming and Alan,
I'd like you to do a code review.
https://bugs.openjdk.java.net/browse/JDK-8056934
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/
<http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/>
This seems like an atypical off-by-one, so I'm not sure how it
happened, and I have the nagging feeling I'm missing something.
Greg, this java code review contains a python program!
I looked at 4.3.9.3. For ZIP64 then we'll read 24 bytes which is 4
bytes too much if the signature is not present. The offset of those 4
bytes is position 20 (ZIP64_EXTHDR - ZIP64_EXTHDR with the current
constants). With ZIP then we'll need 16 bytes and need to unread 4
bytes from position 12 (EXTHDR - EXTCRC with the current constants).
So I think you're right that the -1, I can only assume that we haven't
encountered zip files that have a data descriptor without the signature.
The zip64 part was the copy/paste of the existing code, and we did not
add any test for this corner case we
implemented zip64. Before the fix of 4635869, the implementation simply
throws an exception. With the "fix"
it appears it can work well with the first entry of such a zip file, but
skip the rest...my guess is that the test
case zip file in original bug report happens to be a single entry zip file.
-Sherman
For the comment in ZipInputStream then it might be better to move that
to readEnd. My preference would be to not include the part starting
"As of 2014-08, phyton ..." as that might not interesting in years to
come.
-Alan.
