openpreserve · carlwilson · Jul 25, 2024 · Jul 24, 2024
diff --git a/odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java b/odf-apps/src/main/java/org/openpreservation/odf/apps/CliValidator.java
@@ -10,21 +10,19 @@
 import java.util.Map;
 import java.util.concurrent.Callable;
 
-import javax.xml.parsers.ParserConfigurationException;
-
 import org.openpreservation.messages.Message;
 import org.openpreservation.messages.Message.Severity;
 import org.openpreservation.messages.MessageFactory;
 import org.openpreservation.messages.MessageLog;
 import org.openpreservation.messages.Messages;
 import org.openpreservation.odf.pkg.OdfPackages;
 import org.openpreservation.odf.pkg.PackageParser;
+import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.validation.Profile;
 import org.openpreservation.odf.validation.ProfileResult;
 import org.openpreservation.odf.validation.ValidationReport;
 import org.openpreservation.odf.validation.Validator;
 import org.openpreservation.odf.validation.rules.Rules;
-import org.xml.sax.SAXException;
 
 import picocli.CommandLine;
 import picocli.CommandLine.Command;
@@ -76,8 +74,9 @@ private ValidationReport validatePath(final Path toValidate) {
             return validator.validate(toValidate);
         } catch (IllegalArgumentException | FileNotFoundException e) {
             this.logMessage(toValidate, Messages.getMessageInstance("APP-2", Severity.ERROR, e.getMessage()));
-        } catch (ParserConfigurationException | SAXException | IOException e) {
-            e.printStackTrace();
+        } catch (ParseException e) {
+            this.logMessage(toValidate, Messages.getMessageInstance("SYS-1", Severity.ERROR,
+                    "Package could not be parsed, due to an exception.", e.getMessage()));
         }
         return null;
     }
@@ -87,8 +86,9 @@ private ProfileResult profilePath(final Path toProfile) {
             return dnaProfile.check(parser.parsePackage(toProfile));
         } catch (IllegalArgumentException | FileNotFoundException e) {
             this.logMessage(toProfile, Messages.getMessageInstance("APP-2", Severity.ERROR, e.getMessage()));
-        } catch (IOException e) {
-            e.printStackTrace();
+        } catch (ParseException e) {
+            this.logMessage(toProfile, Messages.getMessageInstance("SYS-1", Severity.ERROR,
+                    "Package could not be parsed, due to an exception.", e.getMessage()));
         }
         return null;
     }

diff --git a/odf-apps/src/main/resources/org/openpreservation/odf/apps/messages/Messages.properties b/odf-apps/src/main/resources/org/openpreservation/odf/apps/messages/Messages.properties
@@ -6,5 +6,4 @@ APP-5 = %s Profile report for %s.
 SYS-1 = Supplied path could not be read, IO Exception %s.
 SYS-2 = Could not load internal schema %s due to Exception %s.
 SYS-3 = Could not configure SAX parser due to Exception %s.
-
-
+SYS-4 = Package %s could not be parsed, due to an exception.
diff --git a/odf-core/src/main/java/org/openpreservation/format/zip/ZipArchive.java b/odf-core/src/main/java/org/openpreservation/format/zip/ZipArchive.java
@@ -16,7 +16,8 @@ public interface ZipArchive {
     /**
      * Get a <code>List<ZipEntry></code> of all of the zip entries in the archive
      *
-     * @return an ordered <code>List<ZipEntry></code> of all of the zip entries in the archive
+     * @return an ordered <code>List<ZipEntry></code> of all of the zip entries in
+     *         the archive
      */
     public List<ZipEntry> getZipEntries();
 
@@ -25,7 +26,8 @@ public interface ZipArchive {
      *
      * @param entryName the name of the <code>ZipEntry</code> to retrieve
      *
-     * @return the <code>ZipEntry</code> with the given <code>entryName</code>, or <code>null</code> if no match
+     * @return the <code>ZipEntry</code> with the given <code>entryName</code>, or
+     *         <code>null</code> if no match
      */
     public ZipEntry getZipEntry(final String entryName);
 

diff --git a/odf-core/src/main/java/org/openpreservation/format/zip/ZipArchiveCache.java b/odf-core/src/main/java/org/openpreservation/format/zip/ZipArchiveCache.java
@@ -5,18 +5,23 @@
 import java.util.List;
 
 /**
- * An extension of {@link ZipArchive} that caches the contents of the archive and provides access to the <code>InputStream</code>s.
+ * An extension of {@link ZipArchive} that caches the contents of the archive
+ * and provides access to the <code>InputStream</code>s.
  */
 public interface ZipArchiveCache extends ZipArchive {
     /**
      * Get a <code>List<String></code> of all of the cached entries in the archive
+     *
      * @return
      */
     public List<String> getCachedEntryNames();
 
     /**
-     * Get the <code>InputStream</code> for the entry with the passed name, equivalent to the path.
-     * @return the <code>InputStream</code> for the entry with the passed <code>name</code>, or <code>null</code> if no match
+     * Get the <code>InputStream</code> for the entry with the passed name,
+     * equivalent to the path.
+     *
+     * @return the <code>InputStream</code> for the entry with the passed
+     *         <code>name</code>, or <code>null</code> if no match
      */
     public InputStream getEntryInputStream(final String entryName) throws IOException;
 }
diff --git a/odf-core/src/main/java/org/openpreservation/format/zip/ZipFileProcessor.java b/odf-core/src/main/java/org/openpreservation/format/zip/ZipFileProcessor.java
@@ -15,6 +15,7 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import org.apache.commons.compress.archivers.zip.UnsupportedZipFeatureException;
 import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
 import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.openpreservation.utils.Checks;

diff --git a/odf-core/src/main/java/org/openpreservation/odf/pkg/OdfPackageImpl.java b/odf-core/src/main/java/org/openpreservation/odf/pkg/OdfPackageImpl.java
@@ -112,7 +112,8 @@ public OdfPackage build() {
 
     private OdfPackageImpl(final String name, final ZipArchiveCache archive, final Formats format,
             final Version version,
-            final String mimetype, final Manifest manifest, final Map<String, OdfPackageDocument> documentMap,
+            final String mimetype, final Manifest manifest,
+            final Map<String, OdfPackageDocument> documentMap,
             final Map<String, ParseResult> metaInfMap) {
         super();
         this.archive = archive;

diff --git a/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParser.java b/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParser.java
@@ -1,9 +1,11 @@
 package org.openpreservation.odf.pkg;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Path;
+import java.util.Map;
 
 import org.openpreservation.format.zip.ZipArchive;
 
@@ -22,7 +24,7 @@ public interface PackageParser {
      *                              package {@link ZipArchive}.
      * @throws NullPointerException when <code>toParse</code> is null
      */
-    public OdfPackage parsePackage(final Path is) throws IOException;
+    public OdfPackage parsePackage(final Path is) throws ParseException, FileNotFoundException;
 
     /**
      * Parse a Java File instance and return an {@link OdfPackage} instance.
@@ -35,7 +37,7 @@ public interface PackageParser {
      *                              package {@link ZipArchive}
      * @throws NullPointerException when <code>toParse</code> is null
      */
-    public OdfPackage parsePackage(final File toParse) throws IOException;
+    public OdfPackage parsePackage(final File toParse) throws ParseException, FileNotFoundException;
 
     /**
      * Parse an <code>InputStream</code> and return an {@link OdfPackage} instance.
@@ -52,5 +54,33 @@ public interface PackageParser {
      *                              is <code>null</code>
      */
     public OdfPackage parsePackage(final InputStream toParse, final String name)
-            throws IOException;
+            throws ParseException;
+
+    public static class ParseException extends Exception {
+        private static final long serialVersionUID = 1L;
+
+        public ParseException(final String message) {
+            super(message);
+        }
+
+        public ParseException(final Map<String, String> badEntries) {
+            super(messageFromEntryMap(badEntries));
+        }
+
+        public ParseException(final String message, final Throwable cause) {
+            super(message, cause);
+        }
+
+        private static final String messageFromEntryMap(final Map<String, String> badEntries) {
+            StringBuilder sb = new StringBuilder();
+            sb.append("The following zip entries could not be read: ");
+            badEntries.forEach((k, v) -> {
+                sb.append(k);
+                sb.append(": ");
+                sb.append(v);
+                sb.append("\n");
+            });
+            return sb.toString();
+        }
+    }
 }
diff --git a/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParserImpl.java b/odf-core/src/main/java/org/openpreservation/odf/pkg/PackageParserImpl.java
@@ -2,6 +2,7 @@
 
 import java.io.BufferedInputStream;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
@@ -15,6 +16,9 @@
 
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.apache.commons.compress.archivers.zip.UnsupportedZipFeatureException;
+import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
+import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.openpreservation.format.xml.ParseResult;
 import org.openpreservation.format.xml.XmlParser;
 import org.openpreservation.format.zip.ZipArchiveCache;
@@ -31,6 +35,8 @@
 
 final class PackageParserImpl implements PackageParser {
     private static String toParseConst = "toParse";
+    private static String badFeature = "Unsupported Zip feature: %s";
+    private static String ioException = "IO Exception reading stream: %s";
 
     static final PackageParser getInstance() {
         return new PackageParserImpl();
@@ -56,6 +62,7 @@ private static final Formats sniff(final Path toSniff) throws IOException {
             return FormatSniffer.sniff(bis);
         }
     }
+
     private ZipArchiveCache cache;
     private Formats format = Formats.UNKNOWN;
     private String mimetype = "";
@@ -69,34 +76,40 @@ private PackageParserImpl() {
     }
 
     @Override
-    public OdfPackage parsePackage(final Path toParse) throws IOException {
+    public OdfPackage parsePackage(final Path toParse) throws ParseException, FileNotFoundException {
         Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "Path"));
         return this.parsePackage(toParse, toParse.getFileName().toString());
     }
 
     @Override
-    public OdfPackage parsePackage(final File toParse) throws IOException {
+    public OdfPackage parsePackage(final File toParse) throws ParseException, FileNotFoundException {
         Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "File"));
         return this.parsePackage(toParse.toPath(), toParse.getName());
     }
 
     @Override
-    public OdfPackage parsePackage(final InputStream toParse, final String name) throws IOException {
+    public OdfPackage parsePackage(final InputStream toParse, final String name) throws ParseException {
         Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "InputStream"));
         Objects.requireNonNull(name, String.format(Checks.NOT_NULL, name, "String"));
         try (BufferedInputStream bis = new BufferedInputStream(toParse)) {
             final Path temp = Files.createTempFile("odf", ".pkg");
             Files.copy(bis, temp, StandardCopyOption.REPLACE_EXISTING);
             return this.parsePackage(temp, name);
+        } catch (IOException e) {
+            throw new ParseException("IOException occured when reading package.", e);
         }
     }
 
-    private final OdfPackage parsePackage(final Path toParse, final String name) throws IOException {
+    private final OdfPackage parsePackage(final Path toParse, final String name) throws ParseException, FileNotFoundException {
         Checks.existingFileCheck(toParse);
         this.initialise();
         try {
             this.format = sniff(toParse);
             this.cache = Zips.zipArchiveCacheInstance(toParse);
+            Map<String, String> badEntries = checkZipEntries();
+            if (!badEntries.isEmpty()) {
+                throw new ParseException(badEntries);
+            }
             this.version = detectVersion();
         } catch (final IOException e) {
             // Simply catch the exception and return a sparsely populated OdfPackage
@@ -106,13 +119,28 @@ private final OdfPackage parsePackage(final Path toParse, final String name) thr
             this.processZipEntries();
             return this.makePackage(name);
         } catch (ParserConfigurationException | SAXException e) {
-            throw new IOException(e);
+            throw new ParseException("SAX Exception while parsing package.", e);
+        } catch (IOException e) {
+            throw new ParseException("IOException while parsing package.", e);
         }
     }
 
+    private final Map<String, String> checkZipEntries() {
+        final Map<String, String> badEntries = new HashMap<>();
+        for (ZipEntry entry : this.cache.getZipEntries()) {
+            try {
+                this.cache.getEntryInputStream(entry.getName()).close();
+            } catch (UnsupportedZipFeatureException e) {
+                badEntries.put(entry.getName(), String.format(badFeature, e.getFeature().toString()));
+            } catch (IOException e) {
+                badEntries.put(entry.getName(), String.format(ioException, e.getMessage()));
+            }
+        }
+        return badEntries;
+    }
 
     final Version detectVersion() throws IOException {
-        Version version = Version.UNKNOWN;
+        Version detectedVersion = Version.UNKNOWN;
         try (InputStream is = getVersionStreamName()) {
             if (is != null) {
                 ParseResult result = new XmlParser().parse(is);
@@ -121,7 +149,7 @@ final Version detectVersion() throws IOException {
         } catch (ParserConfigurationException | SAXException e) {
             throw new IOException(e);
         }
-        return version;
+        return detectedVersion;
     }
 
     private final InputStream getVersionStreamName() throws IOException {
@@ -189,6 +217,7 @@ private final OdfPackage makePackage(final String name)
                 builder.document(docEntry.getFullPath(), makeDocument(docEntry));
             }
         }
+
         for (final Entry<String, OdfXmlDocument> docEntry : this.xmlDocumentMap.entrySet()) {
             if (isMetaInf(docEntry.getKey())) {
                 builder.metaInf(docEntry.getKey(), docEntry.getValue().getParseResult());

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/Profile.java b/odf-core/src/main/java/org/openpreservation/odf/validation/Profile.java
@@ -1,16 +1,16 @@
 package org.openpreservation.odf.validation;
 
-import java.io.IOException;
 import java.util.Set;
 
 import org.openpreservation.odf.pkg.OdfPackage;
+import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.xml.OdfXmlDocument;
 
 public interface Profile {
     public String getId();
     public String getName();
     public String getDescription();
-    public ProfileResult check(final OdfXmlDocument document) throws IOException;
-    public ProfileResult check(final OdfPackage odfPackage) throws IOException;
+    public ProfileResult check(final OdfXmlDocument document) throws ParseException;
+    public ProfileResult check(final OdfPackage odfPackage) throws ParseException;
     public Set<Rule> getRules();
 }
diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/Rule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/Rule.java
@@ -5,6 +5,7 @@
 import org.openpreservation.messages.MessageLog;
 import org.openpreservation.messages.Message.Severity;
 import org.openpreservation.odf.pkg.OdfPackage;
+import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.xml.OdfXmlDocument;
 
 public interface Rule {
@@ -13,6 +14,6 @@ public interface Rule {
     public String getDescription();
     public Severity getSeverity();
     public boolean isPrerequisite();
-    public MessageLog check(final OdfXmlDocument document) throws IOException;
-    public MessageLog check(final OdfPackage odfPackage) throws IOException;
+    public MessageLog check(final OdfXmlDocument document) throws ParseException;
+    public MessageLog check(final OdfPackage odfPackage) throws ParseException;
 }
diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/ValidatingParser.java b/odf-core/src/main/java/org/openpreservation/odf/validation/ValidatingParser.java
@@ -1,5 +1,6 @@
 package org.openpreservation.odf.validation;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 
 import org.openpreservation.odf.pkg.OdfPackage;
@@ -8,10 +9,12 @@
 public interface ValidatingParser extends PackageParser {
     /**
      * Validates the given ODF package.
+     * 
      * @param odfPackage the ODF Package to validate, this must not be null
      * @return a ValidationReport containing the results of the validation
-     * @throws IOException if there's a problem reading package elements from the zip file.
+     * @throws IOException if there's a problem reading package elements from the
+     *                     zip file.
      */
     public ValidationReport validatePackage(final OdfPackage odfPackage)
-            throws IOException;
+            throws ParseException, FileNotFoundException;
 }
diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/ValidatingParserImpl.java b/odf-core/src/main/java/org/openpreservation/odf/validation/ValidatingParserImpl.java
@@ -1,6 +1,7 @@
 package org.openpreservation.odf.validation;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Path;
@@ -70,17 +71,17 @@ public ValidationReport validatePackage(final OdfPackage toValidate) {
     }
 
     @Override
-    public OdfPackage parsePackage(Path toParse) throws IOException {
+    public OdfPackage parsePackage(Path toParse) throws ParseException, FileNotFoundException {
         return this.packageParser.parsePackage(toParse);
     }
 
     @Override
-    public OdfPackage parsePackage(File toParse) throws IOException {
+    public OdfPackage parsePackage(File toParse) throws ParseException, FileNotFoundException {
         return this.packageParser.parsePackage(toParse);
     }
 
     @Override
-    public OdfPackage parsePackage(final InputStream toParse, final String name) throws IOException {
+    public OdfPackage parsePackage(final InputStream toParse, final String name) throws ParseException {
         return this.packageParser.parsePackage(toParse, name);
     }