Fix broken Zip64 extended info size checks.
We assumed that all fields listed in the extended info layout were
always present. This is not the case. Only the size / uncompressed
size are required fields (in the LFH) and the directory offset is
required only if the corresponding field in the regular record is
0xFFFFFFFF. The same goes for disk number too, but we don't support
spanned archives, so we can assume that it's never present.
This change also fixes a spec violation where we were *not* writing
0xFFFFFFFF to the central directories LFH offset field when we were
putting the actual information in the Zip64 Extended info field.
This change also adds lower level unit tests to some of the Zip64
parsing functions since it's pretty hard (impossible ?) to find a
Zip64 tool that outputs stuff in the format we want.
bug: 22851464
Change-Id: I3474fe501d0b7d5b91803976fd0b59d06fa127e8
diff --git a/luni/src/main/java/java/util/zip/Zip64.java b/luni/src/main/java/java/util/zip/Zip64.java
index 9be3d1c..3060670 100644
--- a/luni/src/main/java/java/util/zip/Zip64.java
+++ b/luni/src/main/java/java/util/zip/Zip64.java
@@ -19,6 +19,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
+import java.nio.BufferOverflowException;
import java.nio.BufferUnderflowException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
@@ -37,8 +38,10 @@
/**
* The maximum supported entry / archive size for standard (non zip64) entries and archives.
+ *
+ * @hide
*/
- static final long MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE = 0x00000000ffffffffL;
+ public static final long MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE = 0x00000000ffffffffL;
/**
* The header ID of the zip64 extended info header. This value is used to identify
@@ -46,11 +49,6 @@
*/
private static final short ZIP64_EXTENDED_INFO_HEADER_ID = 0x0001;
- /**
- * The minimum size of the zip64 extended info header. This excludes the 2 byte header ID
- * and the 2 byte size.
- */
- private static final int ZIP64_EXTENDED_INFO_MIN_SIZE = 28;
/*
* Size (in bytes) of the zip64 end of central directory locator. This will be located
@@ -191,35 +189,32 @@
if (extendedInfoSize != -1) {
extendedInfoStart = buf.position();
try {
- if (extendedInfoSize < ZIP64_EXTENDED_INFO_MIN_SIZE) {
- throw new ZipException("Invalid zip64 extended info size: " + extendedInfoSize);
- }
-
// The size & compressed size only make sense in the central directory *or* if
// we know them beforehand. If we don't know them beforehand, they're stored in
// the data descriptor and should be read from there.
+ //
+ // Note that the spec says that the local file header "MUST" contain the
+ // original and compressed size fields. We don't care too much about that.
+ // The spec claims that the order of fields is fixed anyway.
if (fromCentralDirectory || (ze.getMethod() == ZipEntry.STORED)) {
- final long zip64Size = buf.getLong();
if (ze.size == MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE) {
- ze.size = zip64Size;
+ ze.size = buf.getLong();
}
- final long zip64CompressedSize = buf.getLong();
if (ze.compressedSize == MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE) {
- ze.compressedSize = zip64CompressedSize;
+ ze.compressedSize = buf.getLong();
}
}
// The local header offset is significant only in the central directory. It makes no
// sense within the local header itself.
if (fromCentralDirectory) {
- final long zip64LocalHeaderRelOffset = buf.getLong();
if (ze.localHeaderRelOffset == MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE) {
- ze.localHeaderRelOffset = zip64LocalHeaderRelOffset;
+ ze.localHeaderRelOffset = buf.getLong();
}
}
} catch (BufferUnderflowException bue) {
- ZipException zipException = new ZipException("Error parsing extendend info ");
+ ZipException zipException = new ZipException("Error parsing extended info");
zipException.initCause(bue);
throw zipException;
}
@@ -273,8 +268,20 @@
*/
public static void insertZip64ExtendedInfoToExtras(ZipEntry ze) throws ZipException {
final byte[] output;
- // We add 4 to ZIP64_EXTENDED_INFO_MIN_SIZE to account for the 2 byte header and length.
- final int extendedInfoSize = ZIP64_EXTENDED_INFO_MIN_SIZE + 4;
+ // We always write the size, uncompressed size and local rel header offset in all our
+ // Zip64 extended info headers (in both the local file header as well as the central
+ // directory). We always omit the disk number because we don't support spanned
+ // archives anyway.
+ //
+ // 2 bytes : Zip64 Extended Info Header ID
+ // 2 bytes : Zip64 Extended Info Field Size.
+ // 8 bytes : Uncompressed size
+ // 8 bytes : Compressed size
+ // 8 bytes : Local header rel offset.
+ // ----------
+ // 28 bytes : total
+ final int extendedInfoSize = 28;
+
if (ze.extra == null) {
output = new byte[extendedInfoSize];
} else {
@@ -291,13 +298,15 @@
// This means that people that for ZipOutputStream users, the value ZipEntry.getExtra
// after an entry is written will be different from before. This shouldn't be an issue
// in practice.
- output = new byte[ze.extra.length + ZIP64_EXTENDED_INFO_MIN_SIZE + 4];
- System.arraycopy(ze.extra, 0, output, ZIP64_EXTENDED_INFO_MIN_SIZE + 4, ze.extra.length);
+ output = new byte[ze.extra.length + extendedInfoSize];
+ System.arraycopy(ze.extra, 0, output, extendedInfoSize, ze.extra.length);
}
ByteBuffer bb = ByteBuffer.wrap(output).order(ByteOrder.LITTLE_ENDIAN);
bb.putShort(ZIP64_EXTENDED_INFO_HEADER_ID);
- bb.putShort((short) ZIP64_EXTENDED_INFO_MIN_SIZE);
+ // We subtract four because extendedInfoSize includes the ID and field
+ // size itself.
+ bb.putShort((short) (extendedInfoSize - 4));
if (ze.getMethod() == ZipEntry.STORED) {
bb.putLong(ze.size);
@@ -311,7 +320,6 @@
// The offset is only relevant in the central directory entry, but we write it out here
// anyway, since we know what it is.
bb.putLong(ze.localHeaderRelOffset);
- bb.putInt(0); // disk number
ze.extra = output;
}
@@ -354,23 +362,29 @@
* we could calculate the correct sizes only after writing out the entry. In this case,
* the local file header would not contain real sizes, and they would be present in the
* data descriptor and the central directory only.
+ *
+ * We choose the simplest strategy of always writing out the size, compressedSize and
+ * local header offset in all our Zip64 Extended info records.
*/
public static void refreshZip64ExtendedInfo(ZipEntry ze) {
- if (ze.extra == null || ze.extra.length < ZIP64_EXTENDED_INFO_MIN_SIZE) {
+ if (ze.extra == null) {
throw new IllegalStateException("Zip64 entry has no available extras: " + ze);
}
-
ByteBuffer buf = ByteBuffer.wrap(ze.extra).order(ByteOrder.LITTLE_ENDIAN);
- if (getZip64ExtendedInfoSize(buf) == -1) {
+ final int extendedInfoSize = getZip64ExtendedInfoSize(buf);
+ if (extendedInfoSize == -1) {
throw new IllegalStateException(
"Zip64 entry extras has no zip64 extended info record: " + ze);
}
- buf.putLong(ze.size);
- buf.putLong(ze.compressedSize);
- buf.putLong(ze.localHeaderRelOffset);
- buf.putInt(0); // disk number.
+ try {
+ buf.putLong(ze.size);
+ buf.putLong(ze.compressedSize);
+ buf.putLong(ze.localHeaderRelOffset);
+ } catch (BufferOverflowException boe) {
+ throw new IllegalStateException("Invalid extended info extra", boe);
+ }
}
public static void writeZip64EocdRecordAndLocator(ByteArrayOutputStream baos,
diff --git a/luni/src/main/java/java/util/zip/ZipEntry.java b/luni/src/main/java/java/util/zip/ZipEntry.java
index 26f6863..a06f1b6 100644
--- a/luni/src/main/java/java/util/zip/ZipEntry.java
+++ b/luni/src/main/java/java/util/zip/ZipEntry.java
@@ -66,7 +66,8 @@
*/
public static final int STORED = 0;
- ZipEntry(String name, String comment, long crc, long compressedSize,
+ /** @hide - for testing only */
+ public ZipEntry(String name, String comment, long crc, long compressedSize,
long size, int compressionMethod, int time, int modDate, byte[] extra,
long localHeaderRelOffset, long dataOffset) {
this.name = name;
diff --git a/luni/src/main/java/java/util/zip/ZipOutputStream.java b/luni/src/main/java/java/util/zip/ZipOutputStream.java
index 7748cfd..dfd85b6 100644
--- a/luni/src/main/java/java/util/zip/ZipOutputStream.java
+++ b/luni/src/main/java/java/util/zip/ZipOutputStream.java
@@ -260,7 +260,11 @@
writeIntAsUint16(cDir, 0); // Disk Start
writeIntAsUint16(cDir, 0); // Internal File Attributes
writeLongAsUint32(cDir, 0); // External File Attributes
- writeLongAsUint32(cDir, offset);
+ if (currentEntryNeedsZip64) {
+ writeLongAsUint32(cDir, Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE);
+ } else {
+ writeLongAsUint32(cDir, currentEntry.localHeaderRelOffset);
+ }
cDir.write(nameBytes);
nameBytes = null;
if (currentEntry.extra != null) {
diff --git a/luni/src/test/java/libcore/java/util/zip/Zip64Test.java b/luni/src/test/java/libcore/java/util/zip/Zip64Test.java
new file mode 100644
index 0000000..e4b5baf
--- /dev/null
+++ b/luni/src/test/java/libcore/java/util/zip/Zip64Test.java
@@ -0,0 +1,100 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package libcore.java.util.zip;
+
+import junit.framework.TestCase;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.zip.Zip64;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
+
+public class Zip64Test extends TestCase {
+
+ // We shouldn't attempt to look inside the extended info if we have valid fields
+ // in the regular file header / central directory entry.
+ public void testParseZip64ExtendedInfo_noFieldsPresent() throws Exception {
+ ZipEntry ze = createZipEntry(null, 100, 200, ZipEntry.STORED, 300);
+ Zip64.parseZip64ExtendedInfo(ze, false /* fromCentralDirectory */);
+ Zip64.parseZip64ExtendedInfo(ze, true /* fromCentralDirectory */);
+ }
+
+ // We *should* attempt to look in the extended info if the local file header / central
+ // directory entry don't have the correct values.
+ public void testParseZip64ExtendedInfo_missingExtendedInfo() throws Exception {
+ ZipEntry ze = createZipEntry(null, Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE,
+ Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE, ZipEntry.STORED, 300);
+ try {
+ Zip64.parseZip64ExtendedInfo(ze, false /* fromCentralDirectory */);
+ fail();
+ } catch (ZipException expected) {
+ }
+
+ try {
+ Zip64.parseZip64ExtendedInfo(ze, true /* fromCentralDirectory */);
+ fail();
+ } catch (ZipException expected) {
+ }
+ }
+
+ // Test the case where the compressed / uncompressed sizes are in the extended info
+ // but the header offset isn't.
+ public void testParseZip64ExtendedInfo_partialInfo() throws Exception {
+ byte[] extras = new byte[20];
+ ByteBuffer buf = ByteBuffer.wrap(extras);
+ buf.order(ByteOrder.LITTLE_ENDIAN);
+ buf.putShort((short) 0x0001);
+ buf.putShort((short) 16);
+ buf.putLong(50);
+ buf.putLong(100);
+
+ ZipEntry ze = createZipEntry(extras, Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE,
+ Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE, ZipEntry.STORED, 300);
+
+ Zip64.parseZip64ExtendedInfo(ze, false /*fromCentralDirectory */);
+ assertEquals(50, ze.getSize());
+ assertEquals(100, ze.getCompressedSize());
+
+ ze = createZipEntry(extras, Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE,
+ Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE, ZipEntry.STORED, 300);
+ Zip64.parseZip64ExtendedInfo(ze, true /*fromCentralDirectory */);
+ assertEquals(50, ze.getSize());
+ assertEquals(100, ze.getCompressedSize());
+ }
+
+ public void testInsertZip64ExtendedInfo() throws Exception {
+ ZipEntry ze = createZipEntry(null, Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE + 300,
+ Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE + 500, ZipEntry.STORED, 300);
+ Zip64.insertZip64ExtendedInfoToExtras(ze);
+
+ assertNotNull(ze.getExtra());
+ ByteBuffer buf = ByteBuffer.wrap(ze.getExtra());
+ buf.order(ByteOrder.LITTLE_ENDIAN);
+ assertEquals(0x0001, buf.getShort());
+ assertEquals(24, buf.getShort());
+ assertEquals(Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE + 300, buf.getLong());
+ assertEquals(Zip64.MAX_ZIP_ENTRY_AND_ARCHIVE_SIZE + 500, buf.getLong());
+ }
+
+ private static ZipEntry createZipEntry(byte[] extras, long size, long compressedSize,
+ int compressionMethod, long headerOffset) {
+ return new ZipEntry("name", "comment", 42 /* crc */, compressedSize, size,
+ compressionMethod, 42 /* time */, 42 /* modDate */, extras, headerOffset,
+ 42 /* data offset */);
+ }
+}
