From f7aacc0737eac1fdc7816acba294f803c287e7d6 Mon Sep 17 00:00:00 2001 From: sherman Date: Wed, 28 Aug 2013 09:46:55 -0700 Subject: [PATCH] 8023713: ZipFileSystem crashes on old zip file Summary: to handle extra data field copy correctly even the extra data does not follow the spec Reviewed-by: alanb, martin, chegar --- .../java/util/zip/ZipOutputStream.java | 10 +++ test/java/util/zip/TestExtraTime.java | 63 +++++++++++++------ 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/share/classes/java/util/zip/ZipOutputStream.java b/src/share/classes/java/util/zip/ZipOutputStream.java index 4072fbed52..3b76ecaab2 100644 --- a/src/share/classes/java/util/zip/ZipOutputStream.java +++ b/src/share/classes/java/util/zip/ZipOutputStream.java @@ -663,6 +663,9 @@ private int getExtraLen(byte[] extra) { while (off + 4 <= len) { int tag = get16(extra, off); int sz = get16(extra, off + 2); + if (sz < 0 || (off + 4 + sz) > len) { + break; + } if (tag == EXTID_EXTT || tag == EXTID_ZIP64) { skipped += (sz + 4); } @@ -684,11 +687,18 @@ private void writeExtra(byte[] extra) throws IOException { while (off + 4 <= len) { int tag = get16(extra, off); int sz = get16(extra, off + 2); + if (sz < 0 || (off + 4 + sz) > len) { + writeBytes(extra, off, len - off); + return; + } if (tag != EXTID_EXTT && tag != EXTID_ZIP64) { writeBytes(extra, off, sz + 4); } off += (sz + 4); } + if (off < len) { + writeBytes(extra, off, len - off); + } } } diff --git a/test/java/util/zip/TestExtraTime.java b/test/java/util/zip/TestExtraTime.java index 9923ea693e..b96c85f7c0 100644 --- a/test/java/util/zip/TestExtraTime.java +++ b/test/java/util/zip/TestExtraTime.java @@ -23,7 +23,7 @@ /** * @test - * @bug 4759491 6303183 7012868 8015666 + * @bug 4759491 6303183 7012868 8015666 8023713 * @summary Test ZOS and ZIS timestamp in extra field correctly */ @@ -32,6 +32,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.FileTime; +import java.util.Arrays; import java.util.TimeZone; import java.util.concurrent.TimeUnit; import java.util.zip.ZipEntry; @@ -52,24 +53,26 @@ public static void main(String[] args) throws Throwable{ FileTime ctime = FileTime.from(time - 300000, TimeUnit.MILLISECONDS); TimeZone tz = TimeZone.getTimeZone("Asia/Shanghai"); - test(mtime, null, null, null); - // ms-dos 1980 epoch problem - test(FileTime.from(10, TimeUnit.MILLISECONDS), null, null, null); - // non-default tz - test(mtime, null, null, tz); + for (byte[] extra : new byte[][] { null, new byte[] {1, 2, 3}}) { + test(mtime, null, null, null, extra); + // ms-dos 1980 epoch problem + test(FileTime.from(10, TimeUnit.MILLISECONDS), null, null, null, extra); + // non-default tz + test(mtime, null, null, tz, extra); - test(mtime, atime, null, null); - test(mtime, null, ctime, null); - test(mtime, atime, ctime, null); + test(mtime, atime, null, null, extra); + test(mtime, null, ctime, null, extra); + test(mtime, atime, ctime, null, extra); - test(mtime, atime, null, tz); - test(mtime, null, ctime, tz); - test(mtime, atime, ctime, tz); + test(mtime, atime, null, tz, extra); + test(mtime, null, ctime, tz, extra); + test(mtime, atime, ctime, tz, extra); + } } } static void test(FileTime mtime, FileTime atime, FileTime ctime, - TimeZone tz) throws Throwable { + TimeZone tz, byte[] extra) throws Throwable { System.out.printf("--------------------%nTesting: [%s]/[%s]/[%s]%n", mtime, atime, ctime); TimeZone tz0 = TimeZone.getDefault(); @@ -78,8 +81,8 @@ static void test(FileTime mtime, FileTime atime, FileTime ctime, } ByteArrayOutputStream baos = new ByteArrayOutputStream(); ZipOutputStream zos = new ZipOutputStream(baos); - ZipEntry ze = new ZipEntry("TestExtreTime.java"); - + ZipEntry ze = new ZipEntry("TestExtraTime.java"); + ze.setExtra(extra); ze.setLastModifiedTime(mtime); if (atime != null) ze.setLastAccessTime(atime); @@ -87,6 +90,14 @@ static void test(FileTime mtime, FileTime atime, FileTime ctime, ze.setCreationTime(ctime); zos.putNextEntry(ze); zos.write(new byte[] { 1,2 ,3, 4}); + + // append an extra entry to help check if the length and data + // of the extra field are being correctly written (in previous + // entry). + if (extra != null) { + ze = new ZipEntry("TestExtraEntry"); + zos.putNextEntry(ze); + } zos.close(); if (tz != null) { TimeZone.setDefault(tz0); @@ -96,23 +107,23 @@ static void test(FileTime mtime, FileTime atime, FileTime ctime, new ByteArrayInputStream(baos.toByteArray())); ze = zis.getNextEntry(); zis.close(); - check(mtime, atime, ctime, ze); + check(mtime, atime, ctime, ze, extra); // ZipFile Path zpath = Paths.get(System.getProperty("test.dir", "."), - "TestExtraTimp.zip"); + "TestExtraTime.zip"); Files.copy(new ByteArrayInputStream(baos.toByteArray()), zpath); ZipFile zf = new ZipFile(zpath.toFile()); - ze = zf.getEntry("TestExtreTime.java"); + ze = zf.getEntry("TestExtraTime.java"); // ZipFile read entry from cen, which does not have a/ctime, // for now. - check(mtime, null, null, ze); + check(mtime, null, null, ze, extra); zf.close(); Files.delete(zpath); } static void check(FileTime mtime, FileTime atime, FileTime ctime, - ZipEntry ze) { + ZipEntry ze, byte[] extra) { /* System.out.printf(" mtime [%tc]: [%tc]/[%tc]%n", mtime.to(TimeUnit.MILLISECONDS), @@ -130,5 +141,17 @@ static void check(FileTime mtime, FileTime atime, FileTime ctime, ctime.to(TimeUnit.SECONDS) != ze.getCreationTime().to(TimeUnit.SECONDS)) throw new RuntimeException("Timestamp: storing ctime failed!"); + if (extra != null) { + // if extra data exists, the current implementation put it at + // the end of the extra data array (implementation detail) + byte[] extra1 = ze.getExtra(); + if (extra1 == null || extra1.length < extra.length || + !Arrays.equals(Arrays.copyOfRange(extra1, + extra1.length - extra.length, + extra1.length), + extra)) { + throw new RuntimeException("Timestamp: storing extra field failed!"); + } + } } }