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.
tdf · Jul 3, 2024 · 10f2329 · 10f2329
1 parent af01168
commit 10f2329
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 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
@@ -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
@@ -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<String, ZipArchiveEntry> zipEntries) throws IOException, SAXException {
     String firstEntryName = null;
     if (mZipFile != null) {
@@ -76,40 +70,6 @@ String entriesToMap(Map<String, ZipArchiveEntry> 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
@@ -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
@@ -121,6 +121,14 @@
                 <artifactId>commons-compress</artifactId>
                 <version>1.26.2</version>
             </dependency>
+            <!-- commons-compress requires at least version 2.12 but somehow an
+              older version gets used for running tests in odfdom resulting in
+              NoClassDefFoundError on org.apache.commons.io.build.AbstractStreamBuilder -->
+            <dependency>
+                <groupId>commons-io</groupId>
+                <artifactId>commons-io</artifactId>
+                <version>2.16.1</version>
+            </dependency>
             <!-- https://mvnrepository.com/artifact/org.apache.commons/commons-lang3 -->
             <dependency>
                 <groupId>org.apache.commons</groupId>