From 0c77aaac912beeaad3b95f979c2cfa0c0e45b3e7 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Wed, 3 Jul 2024 12:56:03 +0200 Subject: [PATCH] odfdom: always use ZipFile instead of ZipArchiveInputStream The documentation points out numerous pitfalls with ZipArchiveInputStream, so don't use it in main code: https://commons.apache.org/proper/commons-compress/zip.html#ZipArchiveInputStream_vs_ZipFile The zip file was already read into a byte[] before, so just wrap that in SeekableInMemoryByteChannel. This compiles fine but somehow a too old commons-io is used to run unit tests, so fix its version in the top-level pom.xml. --- .../org/odftoolkit/odfdom/pkg/OdfPackage.java | 6 +- .../org/odftoolkit/odfdom/pkg/ZipHelper.java | 76 ++----------------- .../odftoolkit/odfdom/pkg/PackageTest.java | 7 +- pom.xml | 8 ++ 4 files changed, 26 insertions(+), 71 deletions(-) diff --git a/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/OdfPackage.java b/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/OdfPackage.java index 1f72649ecb..c74bef4b06 100644 --- a/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/OdfPackage.java +++ b/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/OdfPackage.java @@ -497,7 +497,11 @@ private void initializeZip(InputStream odfStream) throws SAXException, IOExcepti StreamHelper.transformStream(odfStream, tempBuf); byte[] mTempByteBuf = tempBuf.toByteArray(); tempBuf.close(); - if (mTempByteBuf.length < 3) { + if (mTempByteBuf.length < 4 + || mTempByteBuf[0] != 'P' + || mTempByteBuf[1] != 'K' + || mTempByteBuf[2] != 3 + || mTempByteBuf[3] != 4) { OdfValidationException ve = new OdfValidationException(OdfPackageConstraint.PACKAGE_IS_NO_ZIP, getBaseURI()); if (mErrorHandler != null) { diff --git a/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/ZipHelper.java b/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/ZipHelper.java index b5d5bee87e..cc2573bd2a 100644 --- a/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/ZipHelper.java +++ b/odfdom/src/main/java/org/odftoolkit/odfdom/pkg/ZipHelper.java @@ -23,19 +23,15 @@ */ package org.odftoolkit.odfdom.pkg; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.Enumeration; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.zip.ZipException; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; -import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream; import org.apache.commons.compress.archivers.zip.ZipFile; +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.xml.sax.ErrorHandler; import org.xml.sax.SAXException; @@ -51,16 +47,14 @@ public ZipHelper(OdfPackage pkg, ZipFile zipFile) { mPackage = pkg; } - public ZipHelper(OdfPackage pkg, byte[] buffer) { + public ZipHelper(OdfPackage pkg, byte[] buffer) throws IOException { mZipBuffer = buffer; - mZipFile = null; + SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(mZipBuffer); + mZipFile = + new ZipFile.Builder().setSeekableByteChannel(c).setUseUnicodeExtraFields(false).get(); mPackage = pkg; } - public static ZipArchiveInputStream createZipInputStream(InputStream is) { - return new ZipArchiveInputStream(is, StandardCharsets.UTF_8.toString(), true, true); - } - String entriesToMap(Map zipEntries) throws IOException, SAXException { String firstEntryName = null; if (mZipFile != null) { @@ -76,40 +70,6 @@ String entriesToMap(Map zipEntries) throws IOException, } } } - } else { - ZipArchiveInputStream inputStream = - createZipInputStream(new ByteArrayInputStream(mZipBuffer)); - ZipArchiveEntry zipEntry = null; - try { - zipEntry = inputStream.getNextZipEntry(); - } catch (ZipException e) { - // Unit tests expect us to return an empty map in this case. - } - if (zipEntry != null) { - firstEntryName = zipEntry.getName(); - while (zipEntry != null) { - addZipEntry(zipEntry, zipEntries); - try { - zipEntry = inputStream.getNextZipEntry(); - } catch (java.util.zip.ZipException e) { - if (e.getMessage().contains("only DEFLATED entries can have EXT descriptor")) { - Logger.getLogger(ZipHelper.class.getName()) - .finer("ZIP seems to contain encoded parts!"); - throw e; - } - // JDK 6 -- the try/catch is workaround for a specific JDK 5 only problem - if (!e.getMessage().contains("missing entry name") - && !"1.5.0".equals(System.getProperty("Java.version"))) { - Logger.getLogger(ZipHelper.class.getName()).finer("ZIP ENTRY not found"); - throw e; - } - // ToDo: Error: "only DEFLATED entries can have EXT descriptor" - // ZipInputStream does not expect (and does not know how to handle) an EXT descriptor - // when the associated data was not DEFLATED (i.e. was stored uncompressed, as-is). - } - } - } - inputStream.close(); } return firstEntryName; } @@ -149,37 +109,15 @@ InputStream getInputStream(ZipArchiveEntry entry) throws IOException { if (mZipFile != null) { return mZipFile.getInputStream(entry); } else { - ZipArchiveInputStream inputStream = - createZipInputStream(new ByteArrayInputStream(mZipBuffer)); - ZipArchiveEntry zipEntry = inputStream.getNextZipEntry(); - while (zipEntry != null) { - if (zipEntry.getName().equalsIgnoreCase(entry.getName())) { - return readAsInputStream(inputStream); - } - zipEntry = inputStream.getNextZipEntry(); - } return null; } } - private InputStream readAsInputStream(ZipArchiveInputStream inputStream) throws IOException { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - if (outputStream != null) { - byte[] buf = new byte[4096]; - int r = 0; - while ((r = inputStream.read(buf, 0, 4096)) > -1) { - outputStream.write(buf, 0, r); - } - inputStream.close(); - } - return new ByteArrayInputStream(outputStream.toByteArray()); - } - void close() throws IOException { if (mZipFile != null) { mZipFile.close(); - } else { - mZipBuffer = null; + mZipFile = null; } + mZipBuffer = null; } } diff --git a/odfdom/src/test/java/org/odftoolkit/odfdom/pkg/PackageTest.java b/odfdom/src/test/java/org/odftoolkit/odfdom/pkg/PackageTest.java index c3bd6c8462..3cc2e20028 100644 --- a/odfdom/src/test/java/org/odftoolkit/odfdom/pkg/PackageTest.java +++ b/odfdom/src/test/java/org/odftoolkit/odfdom/pkg/PackageTest.java @@ -32,6 +32,7 @@ import java.io.FileInputStream; import java.io.InputStream; import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import java.util.logging.Level; @@ -82,6 +83,10 @@ public class PackageTest { public PackageTest() {} + public static ZipArchiveInputStream createZipInputStream(InputStream is) { + return new ZipArchiveInputStream(is, StandardCharsets.UTF_8.toString(), true, true); + } + @Test public void testNotCompressImages() throws Exception { // create test presentation @@ -95,7 +100,7 @@ public void testNotCompressImages() throws Exception { // test if the image is not compressed ZipArchiveInputStream zinput = - ZipHelper.createZipInputStream(ResourceUtilities.getTestOutputAsStream(IMAGE_PRESENTATION)); + createZipInputStream(ResourceUtilities.getTestOutputAsStream(IMAGE_PRESENTATION)); ZipArchiveEntry entry = zinput.getNextZipEntry(); while (entry != null) { String entryName = entry.getName(); diff --git a/pom.xml b/pom.xml index 86610d433e..c1071e0761 100644 --- a/pom.xml +++ b/pom.xml @@ -121,6 +121,14 @@ commons-compress 1.26.2 + + + commons-io + commons-io + 2.16.1 + org.apache.commons