Skip to content

Commit

Permalink
8023713: ZipFileSystem crashes on old zip file
Browse files Browse the repository at this point in the history
Summary: to handle extra data field copy correctly even the extra data does not follow the spec
Reviewed-by: alanb, martin, chegar
  • Loading branch information
sherman committed Aug 28, 2013
1 parent 2a04961 commit f7aacc0
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 20 deletions.
10 changes: 10 additions & 0 deletions src/share/classes/java/util/zip/ZipOutputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
}
}

Expand Down
63 changes: 43 additions & 20 deletions test/java/util/zip/TestExtraTime.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -78,15 +81,23 @@ 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);
if (ctime != null)
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);
Expand All @@ -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),
Expand All @@ -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!");
}
}
}
}

0 comments on commit f7aacc0

Please sign in to comment.