REL: v0.16.0 (#210)

* DEV: v0.15.0 dev version. (#202) * FIX: Double validation reporting (#201) - removed the validation report output from profile reporting. * FIX: Policy crash when document unparsable (#203) * Dev/0.15 (#204) * DEV: v0.15.0 dev version. * DEV: v0.15.0 dev version. * FIX: Policy crash when package has no manifest (#205) - added check for null manifest when attempting to list package XML files. Closes #193 * FIX: Crashing policy macro check of unreadable entries. (#206) - null stream entries are no longer checked for macros. * FIX: Version detection issues (#207) * FIX: Version detection issues - using full manifest entry name rather than simply file name; and - use appropriate namespace for detection. * FIX: Version detection fall-through. * FIX: Handling of encrypted entries (#208) - `PackageParser` changes: - parsing of `mimetype` and `META-INF/manifest.xml` are now done up front; - simpleified entry handling; - dedicated methods for mimetype and manifest parsing; - cleaned up handling of bad zip entries prior to moving this to the zip classes; - `OdfPackage` now has an `isEncrypted()` method to check if the package contains encrypted entries; - simplifed message processing in `ValidatingParser`; - addded encryption detection to prevent validation and profiling of encrypted package entries; - CLI reports incomplete validation for packages with encrypted entries, though this is still a little hacky. * REL: v0.16.0 (#209) - bumped maven version -> 0.16.0; - updated version in batch start files; and - updated version in README and other documentation.
openpreserve · Oct 30, 2024 · e46888a · e46888a
1 parent 79186a1
commit e46888a
Show file tree

Hide file tree

Showing 27 changed files with 256 additions and 124 deletions.
diff --git a/README.md b/README.md
@@ -1,6 +1,6 @@
 # ODF Validator
 
-Latest version is 0.14.0.
+Latest version is 0.16.0.
 
 ## About
 

diff --git a/docs/developer/index.md b/docs/developer/index.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.14.0</version>
+    <version>0.16.0</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.14.0</version>
+    <version>0.16.0</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

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
@@ -125,10 +125,14 @@ private static Integer results(final Path path, final ValidationReport report) {
             ConsoleFormatter.info(FACTORY.getInfo("APP-3").getMessage());
         }
         results(report.documentMessages.getMessages());
-        outputSummary(report.documentMessages);
+        outputSummary(isEncrypted(report), report.documentMessages);
         return status;
     }
 
+    private static boolean isEncrypted(final ValidationReport report) {
+        return report.document.isPackage() && report.document.getPackage().isEncrypted();
+    }
+
     private static Integer results(final Map<String, List<Message>> messageMap) {
         Integer status = 0;
         for (Map.Entry<String, List<Message>> entry : messageMap.entrySet()) {
@@ -140,9 +144,6 @@ private static Integer results(final Map<String, List<Message>> messageMap) {
     private static Integer results(final Path path, final ProfileResult report) {
         Integer status = 0;
         ConsoleFormatter.colourise(FACTORY.getInfo("APP-5", report.getName(), path.toString(), "bold"));
-        if (report.getValidationReport() != null) {
-            status = results(report.getValidationReport().documentMessages.getMessages());
-        }
         for (Map.Entry<String, List<Message>> entry : report.getMessageLog().getMessages().entrySet()) {
             status = Math.max(status, results(entry.getKey(), entry.getValue()));
         }
@@ -153,7 +154,7 @@ private static Integer results(final Path path, final ProfileResult report) {
                 ? report.getValidationReport().documentMessages
                 : Messages.messageLogInstance();
         profileMessages.add(report.getMessageLog().getMessages());
-        outputSummary(profileMessages);
+        outputSummary(isEncrypted(report.getValidationReport()), profileMessages);
         return status;
     }
 
@@ -168,8 +169,12 @@ private static Integer results(final String path, final List<Message> messages)
         return status;
     }
 
-    private static void outputSummary(final MessageLog messageLog) {
-        if (messageLog.hasErrors()) {
+    private static void outputSummary(final boolean isEncrypted, final MessageLog messageLog) {
+        if (isEncrypted) {
+            ConsoleFormatter.error(String.format(
+                    "INCOMPLETE encrypted entries are not supported, %d errors, %d warnings and %d info messages.",
+                    messageLog.getErrorCount(), messageLog.getWarningCount(), messageLog.getInfoCount()));
+        } else if (messageLog.hasErrors()) {
             ConsoleFormatter.error(String.format("NOT VALID, %d errors, %d warnings and %d info messages.",
                     messageLog.getErrorCount(), messageLog.getWarningCount(), messageLog.getInfoCount()));
         } else if (messageLog.hasWarnings()) {

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.14.0</version>
+    <version>0.16.0</version>
     <relativePath>../pom.xml</relativePath>
   </parent>
 

diff --git a/odf-core/src/main/java/org/openpreservation/odf/pkg/OdfPackage.java b/odf-core/src/main/java/org/openpreservation/odf/pkg/OdfPackage.java
@@ -230,4 +230,11 @@ public interface OdfPackage {
      * @return true if the file uses any namespaces outside of the ODF
      */
     public boolean isExtended();
+
+    /**
+     * Discover if the package had any encrypted entries.
+     *
+     * @return true if the package has encrypted entries
+     */
+    public boolean isEncrypted();
 }
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
@@ -218,8 +218,10 @@ public List<String> getXmlEntryPaths() {
     @Override
     public Set<FileEntry> getXmlEntries() {
         Set<FileEntry> entries = new HashSet<>();
-        for (FileEntry entry : this.manifest.getEntriesByMediaType("text/xml")) {
-            entries.add(entry);
+        if (this.manifest != null) {
+            for (FileEntry entry : this.manifest.getEntriesByMediaType("text/xml")) {
+                entries.add(entry);
+            }
         }
         return entries;
     }
@@ -338,4 +340,17 @@ public boolean isExtended() {
         }
         return false;
     }
+
+    @Override
+    public boolean isEncrypted() {
+        if (this.manifest == null) {
+            return false;
+        }
+        for (FileEntry entry : this.manifest.getEntries()) {
+            if (entry.isEncrypted()) {
+                return true;
+            }
+        }
+        return false;
+    }
 }
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
@@ -32,9 +32,11 @@
 import org.xml.sax.SAXException;
 
 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";
+    private static final String TO_PARSE = "toParse";
+    private static final String MESS_BAD_FEATURE = "Unsupported Zip feature: %s";
+    private static final String MESS_IO_EXCEPTION = "IO Exception reading stream: %s";
+    private static final String[] VERSION_FILE_PATHS = { Constants.PATH_MANIFEST, Constants.NAME_SETTINGS,
+            Constants.NAME_CONTENT };
 
     static final PackageParser getInstance() {
         return new PackageParserImpl();
@@ -75,19 +77,19 @@ private PackageParserImpl() {
 
     @Override
     public OdfPackage parsePackage(final Path toParse) throws ParseException, FileNotFoundException {
-        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "Path"));
+        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, TO_PARSE, "Path"));
         return this.parsePackage(toParse, toParse.getFileName().toString());
     }
 
     @Override
     public OdfPackage parsePackage(final File toParse) throws ParseException, FileNotFoundException {
-        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "File"));
+        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, TO_PARSE, "File"));
         return this.parsePackage(toParse.toPath(), toParse.getName());
     }
 
     @Override
     public OdfPackage parsePackage(final InputStream toParse, final String name) throws ParseException {
-        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, toParseConst, "InputStream"));
+        Objects.requireNonNull(toParse, String.format(Checks.NOT_NULL, TO_PARSE, "InputStream"));
         Objects.requireNonNull(name, String.format(Checks.NOT_NULL, name, "String"));
         try (BufferedInputStream bis = new BufferedInputStream(toParse)) {
             final Path temp = Files.createTempFile("odf", ".pkg");
@@ -98,22 +100,22 @@ public OdfPackage parsePackage(final InputStream toParse, final String name) thr
         }
     }
 
-    private final OdfPackage parsePackage(final Path toParse, final String name) throws ParseException, FileNotFoundException {
+    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);
-            }
+            checkZipEntries();
             this.version = detectVersion();
+            this.mimetype = getMimeEntryValue();
         } catch (final IOException e) {
             // Simply catch the exception and return a sparsely populated OdfPackage
             return OdfPackageImpl.Builder.builder().name(name).format(this.format).build();
         }
         try {
+            this.manifest = parseManifest();
             this.processZipEntries();
             return this.makePackage(name);
         } catch (ParserConfigurationException | SAXException e) {
@@ -123,45 +125,47 @@ private final OdfPackage parsePackage(final Path toParse, final String name) thr
         }
     }
 
-    private final Map<String, String> checkZipEntries() {
+    private final String getMimeEntryValue() throws IOException {
+        return (this.cache.getZipEntry(OdfFormats.MIMETYPE) == null) ? ""
+                : new String(this.cache.getEntryInputStream(OdfFormats.MIMETYPE).readAllBytes(),
+                        StandardCharsets.UTF_8);
+    }
+
+    private void checkZipEntries() throws ParseException {
         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()));
+                badEntries.put(entry.getName(), String.format(MESS_BAD_FEATURE, e.getFeature().toString()));
             } catch (IOException e) {
-                badEntries.put(entry.getName(), String.format(ioException, e.getMessage()));
+                badEntries.put(entry.getName(), String.format(MESS_IO_EXCEPTION, e.getMessage()));
             }
         }
-        return badEntries;
+        if (!badEntries.isEmpty()) {
+            throw new ParseException(badEntries);
+        }
     }
 
     final Version detectVersion() throws IOException {
         Version detectedVersion = Version.UNKNOWN;
-        try (InputStream is = getVersionStreamName()) {
-            if (is != null) {
-                ParseResult result = new XmlParser().parse(is);
-                return Version.fromVersion(result.getRootAttributeValue("office:version"));
+        for (final String versionPath : VERSION_FILE_PATHS) {
+            try (InputStream is = this.cache.getEntryInputStream(versionPath)) {
+                if (is != null) {
+                    ParseResult result = new XmlParser().parse(is);
+                    detectedVersion = Version.fromVersion(
+                            result.getRootAttributeValue(String.format("%s:version", result.getRootPrefix())));
+                    if (!Version.UNKNOWN.equals(detectedVersion)) {
+                        return detectedVersion;
+                    }
+                }
+            } catch (ParserConfigurationException | SAXException e) {
+                throw new IOException(e);
             }
-        } catch (ParserConfigurationException | SAXException e) {
-            throw new IOException(e);
         }
         return detectedVersion;
     }
 
-    private final InputStream getVersionStreamName() throws IOException {
-        InputStream retVal = null;
-        retVal = this.cache.getEntryInputStream(Constants.NAME_MANIFEST);
-        if (retVal == null) {
-            retVal = this.cache.getEntryInputStream(Constants.NAME_SETTINGS);
-        }
-        if (retVal == null) {
-            retVal = this.cache.getEntryInputStream(Constants.NAME_CONTENT);
-        }
-        return retVal;
-    }
-
     private final void processZipEntries() throws ParserConfigurationException, SAXException, IOException {
         for (final ZipEntry entry : this.cache.getZipEntries()) {
             processEntry(entry);
@@ -171,30 +175,31 @@ private final void processZipEntries() throws ParserConfigurationException, SAXE
     private final void processEntry(final ZipEntry entry)
             throws ParserConfigurationException, SAXException, IOException {
         final String path = entry.getName();
-        if (entry.isDirectory()) {
-            // No need to process directories
-            return;
-        }
-        if (OdfFormats.MIMETYPE.equals(path)) {
-            // Grab the mimetype value from the MIMETYPE file
-            this.mimetype = new String(this.cache.getEntryInputStream(entry.getName()).readAllBytes(),
-                    StandardCharsets.UTF_8);
-            return;
-        }
-        if (!isOdfXml(path) && !isMetaInf(path)) {
+        if (entry.isDirectory() || (!isOdfXml(path) && !isMetaInf(path))) {
             return;
         }
         try (InputStream is = this.cache.getEntryInputStream(path)) {
             final OdfXmlDocument xmlDoc = OdfXmlDocuments.xmlDocumentFrom(is);
             if (xmlDoc != null) {
                 this.xmlDocumentMap.put(path, xmlDoc);
-                if (xmlDoc.getParseResult().isWellFormed() && Constants.PATH_MANIFEST.equals(path)) {
-                    this.manifest = ManifestImpl.from(this.cache.getEntryInputStream(path));
-                }
             }
         }
     }
 
+    private final Manifest parseManifest() throws IOException, ParserConfigurationException, SAXException {
+        final ZipEntry manifestEntry = this.cache.getZipEntry(Constants.PATH_MANIFEST);
+        if (manifestEntry == null) {
+            return null;
+        }
+        try (InputStream is = this.cache.getEntryInputStream(Constants.PATH_MANIFEST)) {
+            final OdfXmlDocument xmlDoc = OdfXmlDocuments.xmlDocumentFrom(is);
+            if (xmlDoc != null && xmlDoc.getParseResult().isWellFormed()) {
+                    return ManifestImpl.from(this.cache.getEntryInputStream(Constants.PATH_MANIFEST));
+            }
+        }
+        return null;
+    }
+
     private final void initialise() {
         this.format = Formats.UNKNOWN;
         this.mimetype = null;

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
@@ -6,6 +6,7 @@
 import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -104,19 +105,29 @@ private ValidationReport validate(final OdfPackage odfPackage) {
 
     private final Map<String, List<Message>> validateOdfXmlEntries(final OdfPackage odfPackage) {
         final Map<String, List<Message>> messages = new HashMap<>();
-        for (final String xmlPath : odfPackage.getXmlEntryPaths()) {
-            ParseResult parseResult = odfPackage.getEntryXmlParseResult(xmlPath);
-            if (parseResult == null) {
-                continue;
-            }
-            List<Message> messageList = (parseResult.isWellFormed())
-                    ? validateOdfXmlDocument(odfPackage, xmlPath, parseResult)
-                    : parseResult.getMessages();
-            messages.put(xmlPath, messageList);
+        for (final FileEntry xmlEntry : odfPackage.getXmlEntries()) {
+            messages.put(xmlEntry.getFullPath(), validateXmlEntry(xmlEntry, odfPackage));
         }
         return messages;
     }
 
+    private final List<Message> validateXmlEntry(final FileEntry xmlEntry, final OdfPackage odfPackage) {
+        final String xmlPath = xmlEntry.getFullPath();
+        if (xmlPath.equals("/")) {
+            return new ArrayList<>();
+        }
+        if (xmlEntry.isEncrypted()) {
+            return Arrays.asList(FACTORY.getWarning("PKG-10", xmlPath));
+        }
+        ParseResult parseResult = odfPackage.getEntryXmlParseResult(xmlPath);
+        if (parseResult == null) {
+            return new ArrayList<>();
+        }
+        return (parseResult.isWellFormed())
+                ? validateOdfXmlDocument(odfPackage, xmlPath, parseResult)
+                : parseResult.getMessages();
+    }
+
     private final List<Message> validateOdfXmlDocument(final OdfPackage odfPackage, final String xmlPath,
             final ParseResult parseResult) {
         List<Message> messageList = new ArrayList<>();

diff --git a/odf-core/src/main/java/org/openpreservation/odf/validation/rules/AbstractRule.java b/odf-core/src/main/java/org/openpreservation/odf/validation/rules/AbstractRule.java
@@ -4,6 +4,7 @@
 
 import org.openpreservation.messages.Message.Severity;
 import org.openpreservation.messages.MessageLog;
+import org.openpreservation.messages.Messages;
 import org.openpreservation.odf.pkg.PackageParser.ParseException;
 import org.openpreservation.odf.validation.Rule;
 import org.openpreservation.odf.validation.ValidationReport;
@@ -52,7 +53,11 @@ public boolean isPrerequisite() {
 
     @Override
     public MessageLog check(ValidationReport report) throws ParseException {
-        return report.document.isPackage() ? check(report.document.getPackage()) : check(report.document.getDocument().getXmlDocument());
+        if (report.document == null) {
+            return Messages.messageLogInstance();
+        }
+        return report.document.isPackage() ? check(report.document.getPackage())
+                : check(report.document.getDocument().getXmlDocument());
     }
 
     @Override