On 08/28/2013 10:45 AM, Martin Buchholz wrote:
get16 can not return negative values, right?
correct. will remove it in a separate push. thanks!
So elide the sz < 0 below 666 if (sz < 0 || (off + 4 + sz) > len) { 667 break; 668 } Otherwise, looks good to me. On Tue, Aug 27, 2013 at 4:09 PM, Xueming Shen <[email protected] <mailto:[email protected]>> wrote: On 08/27/2013 03:07 PM, Martin Buchholz wrote:It does seem vaguely reasonable to support any extra data. Don't you want to also handle arbitrary byte arrays, if e.g. one the 16-bit size fields overflows the extra data? It looks to me like getExtraLen could return a negative number.probably I should. The webrev has been updated to simply copy the rest of the extra data, if the "sz" is either < 0 or out of the range. http://cr.openjdk.java.net/~sherman/8023713/webrev/ <http://cr.openjdk.java.net/%7Esherman/8023713/webrev/>And put a SPACE after "if".updated. Thanks! -ShermanOn Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen <[email protected] <mailto:[email protected]>> wrote: Hi, Please help review the change for #8023713 http://cr.openjdk.java.net/~sherman/8023713/webrev <http://cr.openjdk.java.net/%7Esherman/8023713/webrev> The root cause is that the newly introduced ZOS.writeExtra() (for #8015666) fails to handle "irregular" extra data field. The zip spec requires the the extra data stars with 4 bytes of "tag + size" pair and then followed by the actual "extra data". The "offending" zip file actually has the "irregular" extra data field with 1 single byte as the extra data. That said, the implementation (ZOS) should still be able handle this kind of zip entry correctly and appropriately. The proposed solution is to simply copy the specified extra data into the output stream. Thanks! -Sherman
