Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.
On 28/08/2013 00:09, Xueming Shen 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/ Looks ok to me Sherman. -Chris. And put a SPACE after if. updated. Thanks! -Sherman On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com 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
Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.
On 28/08/2013 00:09, Xueming Shen 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/ The updated webrev looks okay, just odd to see an entry named TestExtreTime (which may have been a continuation of a typo in the original test). -Alan.
Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.
On 08/28/2013 03:56 AM, Alan Bateman wrote: On 28/08/2013 00:09, Xueming Shen 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/ The updated webrev looks okay, just odd to see an entry named TestExtreTime (which may have been a continuation of a typo in the original test). Thanks Alan! That was indeed a typo:-) fixed in updated webrev. -Sherman
Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.
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 xueming.s...@oracle.com
mailto:xueming.s...@oracle.com 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!
-Sherman
On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen xueming.s...@oracle.com
mailto:xueming.s...@oracle.com 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
Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.
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/ And put a SPACE after if. updated. Thanks! -Sherman On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com 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
