FIX: Issue parsing not well formed manifest XML

- added null manifest tests to `PackageParserImpl`; - same for `ValidatingParserImpl`; - added test file and unit test for diagnosis and regression protection; - bumped Maven version to `0.9.1`; and - updated docs with the new version.
openpreserve · Nov 8, 2023 · eb3b4d9 · eb3b4d9
1 parent 260da00
commit eb3b4d9
Show file tree

Hide file tree

Showing 10 changed files with 34 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -1,5 +1,7 @@
 # ODF Validator
 
+Latest version is 0.9.1
+
 ## About
 
 [Open Preservation Foundation](https://openpreservation.org/)'s OpenDocument Format Validator (OPF ODF Validator) enables your organisation to validate the file format standard and a set of file format policy rules created for improving the preservation effort of any files saved in the OpenDocument Format.

diff --git a/docs/DEVELOPER.md b/docs/DEVELOPER.md
@@ -44,7 +44,7 @@ To include the core validation library in your project, add the following depend
 <dependency>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-core</artifactId>
-    <version>0.9.0</version>
+    <version>0.9.1</version>
 </dependency>
 ```
 

diff --git a/odf-apps/pom.xml b/odf-apps/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-validator</artifactId>
-    <version>0.9.0</version>
+    <version>0.9.1</version>
   </parent>
 
   <artifactId>odf-apps</artifactId>

diff --git a/odf-core/pom.xml b/odf-core/pom.xml
@@ -5,7 +5,7 @@
   <parent>
     <groupId>org.openpreservation.odf</groupId>
     <artifactId>odf-validator</artifactId>
-    <version>0.9.0</version>
+    <version>0.9.1</version>
   </parent>
 
   <groupId>org.openpreservation.odf</groupId>

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
@@ -77,13 +77,12 @@ private final OdfPackage parsePackage(final Path toParse, final String name) thr
     private final OdfPackage makePackage(final String name, final Formats format)
             throws ParserConfigurationException, IOException, SAXException {
         OdfPackageImpl.Builder builder = OdfPackageImpl.Builder.builder().name(name).archive(this.cache).format(format)
-                .mimetype(mimetype)
-                .manifest(manifest);
-        if (this.manifest == null) {
-            return builder.build();
-        }
-        for (FileEntry docEntry : manifest.getDocumentEntries()) {
-            builder.document(docEntry.getFullPath(), makeDocument(docEntry));
+                .mimetype(mimetype);
+        if (this.manifest != null) {
+            builder.manifest(manifest);
+            for (FileEntry docEntry : manifest.getDocumentEntries()) {
+                builder.document(docEntry.getFullPath(), makeDocument(docEntry));
+            }
         }
         for (Entry<String, OdfXmlDocument> docEntry : this.xmlDocumentMap.entrySet()) {
             if (isMetaInf(docEntry.getKey())) {
@@ -147,9 +146,11 @@ private final void processEntry(final ZipEntry entry)
             OdfXmlDocument xmlDoc = OdfXmlDocuments.xmlDocumentFrom(is);
             if (xmlDoc != null) {
                 this.xmlDocumentMap.put(path, xmlDoc);
+                if (xmlDoc.getParseResult().isWellFormed()) {
+                    this.parseOdfXml(path);
+                }
             }
         }
-        this.parseOdfXml(path);
     }
 
     private final boolean isOdfXml(final String entrypath) {

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
@@ -185,22 +185,24 @@ private final List<Message> validateMimeEntry(final ZipEntry mimeEntry, final bo
     private List<Message> validateManifest(final OdfPackage odfPackage) {
         final List<Message> messages = new ArrayList<>();
         Manifest manifest = odfPackage.getManifest();
-        if (manifest.getEntry("/") == null) {
+        if (manifest != null && manifest.getEntry("/") == null) {
             if (!odfPackage.hasMimeEntry()) {
                 messages.add(FACTORY.getWarning("PKG-19"));
             } else {
                 messages.add(FACTORY.getError("PKG-11"));
             }
-        } else if (!hasManifestRootMime(manifest) || (odfPackage.hasMimeEntry()
+        } else if (hasManifestRootMime(manifest) && (odfPackage.hasMimeEntry()
                 && !manifest.getRootMediaType().equals(odfPackage.getMimeType()))) {
             messages.add(FACTORY.getError("PKG-12", manifest.getRootMediaType(), odfPackage.getMimeType()));
         }
-        messages.addAll(checkManifestEntries(odfPackage));
+        if (manifest != null) {
+            messages.addAll(checkManifestEntries(odfPackage));
+        }
         return messages;
     }
 
     private boolean hasManifestRootMime(final Manifest manifest) {
-        return manifest.getRootMediaType() != null;
+        return manifest != null && manifest.getRootMediaType() != null;
     }
 
     private List<Message> checkManifestEntries(final OdfPackage odfPackage) {

diff --git a/odf-core/src/test/java/org/openpreservation/odf/fmt/TestFiles.java b/odf-core/src/test/java/org/openpreservation/odf/fmt/TestFiles.java
@@ -13,6 +13,7 @@ public class TestFiles {
     final static String EMBEDDED_TEST_ROOT = PKG_TEST_ROOT + "embedded/";
     final static String ENCRYPTED_TEST_ROOT = PKG_TEST_ROOT + "encrypted/";
     final static String DSIG_TEST_ROOT = PKG_TEST_ROOT + "dsigs/";
+    final static String INVALID_PKG_ROOT = PKG_TEST_ROOT + "invalid/";
     final static String XML_TEST_ROOT = TEST_ROOT + "xml/";
     final static String FILE_TEST_ROOT = TEST_ROOT + "files/";
     public final static URL EMPTY_ODS = ClassLoader.getSystemResource(ZIP_TEST_ROOT + "empty.ods");
@@ -56,4 +57,5 @@ public class TestFiles {
     public final static URL DSIG_INVALID = ClassLoader.getSystemResource(DSIG_TEST_ROOT + "dsigs.odt");
     public final static URL DSIG_VALID = ClassLoader.getSystemResource(DSIG_TEST_ROOT + "dsigs-valid.ods");
     public final static URL DSIG_BADNAME = ClassLoader.getSystemResource(DSIG_TEST_ROOT + "bad_dsig_name.ods");
+    public final static URL MANIFEST_NOT_WF = ClassLoader.getSystemResource(INVALID_PKG_ROOT + "manifest_not_wf.ods");
 }
diff --git a/odf-core/src/test/java/org/openpreservation/odf/pkg/PackageParserTest.java b/odf-core/src/test/java/org/openpreservation/odf/pkg/PackageParserTest.java
@@ -1,6 +1,7 @@
 package org.openpreservation.odf.pkg;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
@@ -103,4 +104,14 @@ public void testDsigParsing() throws ParserConfigurationException, SAXException,
         assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null" , result);
         assertTrue("Package should have a well formed dsig for META-INF/documentsignatures.xml" , result.isWellFormed());
     }
+
+    @Test
+    public void testManifestNotWF() throws IOException {
+        PackageParser parser = OdfPackages.getPackageParser();
+        InputStream is = TestFiles.MANIFEST_NOT_WF.openStream();
+        OdfPackage pkg = parser.parsePackage(is, TestFiles.MANIFEST_NOT_WF.toString());
+        ParseResult result = pkg.getEntryXmlParseResult("META-INF/manifest.xml");
+        assertNotNull("Dsig file META-INF/documentsignatures.xml result should not be null" , result);
+        assertFalse("Package should NOT have a well formed META-INF/manifest.xml" , result.isWellFormed());
+    }
 }
diff --git a/odf-core/src/test/resources/org/openpreservation/odf/fmt/pkg/invalid/manifest_not_wf.ods b/odf-core/src/test/resources/org/openpreservation/odf/fmt/pkg/invalid/manifest_not_wf.ods
diff --git a/pom.xml b/pom.xml
@@ -10,7 +10,7 @@
 
   <groupId>org.openpreservation.odf</groupId>
   <artifactId>odf-validator</artifactId>
-  <version>0.9.0</version>
+  <version>0.9.1</version>
   <packaging>pom</packaging>
 
   <modules>