You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/12/16 08:41:13 UTC

[jackrabbit-filevault] branch master updated: JCRVLT-578 Prevent NoSuchElementException in (#189)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git


The following commit(s) were added to refs/heads/master by this push:
     new 432342d  JCRVLT-578 Prevent NoSuchElementException in (#189)
432342d is described below

commit 432342d7735265488366a81359f3db9043b24894
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Thu Dec 16 09:41:06 2021 +0100

    JCRVLT-578 Prevent NoSuchElementException in (#189)
    
    ValidationExecutor.validateGenericJcrData
    
    This happened in case validator "jackrabbit-docviewparser" was disabled
    and there were no nodePaths being collected
---
 .../vault/validation/ValidationExecutor.java       | 27 ++++++++++-----
 .../impl/DocumentViewParserValidatorFactory.java   |  3 +-
 .../vault/validation/ValidationExecutorTest.java   | 40 ++++++++++++++++++++--
 3 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
index 2195566..ee96d47 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
@@ -21,16 +21,18 @@ import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.apache.commons.io.FilenameUtils;
+import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.util.Constants;
 import org.apache.jackrabbit.vault.util.PlatformNameFormat;
-import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.validation.impl.util.EnhancedBufferedInputStream;
 import org.apache.jackrabbit.vault.validation.impl.util.ResettableInputStream;
 import org.apache.jackrabbit.vault.validation.impl.util.ValidatorException;
@@ -50,6 +52,7 @@ import org.apache.jackrabbit.vault.validation.spi.Validator;
 import org.apache.jackrabbit.vault.validation.spi.impl.AdvancedFilterValidator;
 import org.apache.jackrabbit.vault.validation.spi.impl.AdvancedPropertiesValidator;
 import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidator;
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidatorFactory;
 import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -108,6 +111,13 @@ public final class ValidationExecutor {
                 DocumentViewParserValidator.class.cast(validator).setDocumentViewXmlValidators(documentViewXmlValidators);
             }
         }
+        
+        // jcr path validators requires documentviewparservalidator (as that retrieves the JCR paths in the first place)
+        if (!(jcrPathValidators.isEmpty() || nodePathValidators.isEmpty()) && !genericJcrDataValidators.containsKey(DocumentViewParserValidatorFactory.ID)) {
+            Set<String> pathValidatorIds = new HashSet<>(jcrPathValidators.keySet());
+            pathValidatorIds.addAll(nodePathValidators.keySet());
+            throw new IllegalStateException("For the validators with id(s) '" + String.join(", ", pathValidatorIds) + "' it is mandatory to also enable validator '" + DocumentViewParserValidatorFactory.ID + "'");
+        }
     }
 
     /**
@@ -325,15 +335,16 @@ public final class ValidationExecutor {
             nodePathsAndLineNumbers.put(filePathToNodePath(filePath), 0);
         }
 
-        // generate node context
-        NodeContext nodeContext = new NodeContextImpl(nodePathsAndLineNumbers.keySet().iterator().next(), filePath, basePath);
-        for (Map.Entry<String, JcrPathValidator> entry : jcrPathValidators.entrySet()) {
-            Collection<ValidationMessage> messages = entry.getValue().validateJcrPath(nodeContext, input == null, isDocViewXml);
-            if (messages != null && !messages.isEmpty()) {
-                enrichedMessages.addAll(ValidationViolation.wrapMessages(entry.getKey(), messages, filePath, basePath, null, 0, 0));
+        if (!jcrPathValidators.isEmpty() || !nodePathValidators.isEmpty()) {
+            NodeContext nodeContext = new NodeContextImpl(nodePathsAndLineNumbers.keySet().stream().findFirst().orElseThrow(() -> new IllegalStateException("No node paths have been collected")), filePath, basePath);
+            for (Map.Entry<String, JcrPathValidator> entry : jcrPathValidators.entrySet()) {
+                Collection<ValidationMessage> messages = entry.getValue().validateJcrPath(nodeContext, input == null, isDocViewXml);
+                if (messages != null && !messages.isEmpty()) {
+                    enrichedMessages.addAll(ValidationViolation.wrapMessages(entry.getKey(), messages, filePath, basePath, null, 0, 0));
+                }
             }
+            enrichedMessages.addAll(validateNodePaths(filePath, basePath, nodePathsAndLineNumbers));
         }
-        enrichedMessages.addAll(validateNodePaths(filePath, basePath, nodePathsAndLineNumbers));
         return enrichedMessages;
     }
 
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
index 0cb1f72..509c34f 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
@@ -34,6 +34,7 @@ public class DocumentViewParserValidatorFactory implements ValidatorFactory {
 
     private SAXParser saxParser;
 
+    public static final String ID = ValidatorFactory.ID_PREFIX_JACKRABBIT + "docviewparser";
     public DocumentViewParserValidatorFactory() throws ParserConfigurationException, SAXException {
 
         SAXParserFactory spf = SAXParserFactory.newInstance();
@@ -53,7 +54,7 @@ public class DocumentViewParserValidatorFactory implements ValidatorFactory {
 
     @Override
     public @NotNull String getId() {
-        return ValidatorFactory.ID_PREFIX_JACKRABBIT + "docviewparser";
+        return ID;
     }
 
     @Override
diff --git a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
index 9860812..de9adc7 100644
--- a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
+++ b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
@@ -44,6 +44,7 @@ import org.apache.jackrabbit.vault.validation.spi.ValidationContext;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessage;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessageSeverity;
 import org.apache.jackrabbit.vault.validation.spi.Validator;
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidatorFactory;
 import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
@@ -91,7 +92,7 @@ public class ValidationExecutorTest {
         validators.put("propertiesid", propertiesValidator);
         validators.put("genericmetadataid", genericMetaInfDataValidator);
         validators.put("genericmetadataid2", genericMetaInfDataValidator2);
-        validators.put("genericjcrdataid", genericJcrDataValidator);
+        validators.put(DocumentViewParserValidatorFactory.ID, genericJcrDataValidator);
         validators.put("genericjcrdataid2", genericJcrDataValidator2);
         validators.put("jcrpathid", jcrPathValidator);
         validators.put("metainfpathid", metaInfPathValidator);
@@ -173,7 +174,7 @@ public class ValidationExecutorTest {
         try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/genericfile.xml")) {
             Collection<ValidationViolation> messages = validate(input, executor, Paths.get(""), "apps/genericfile.xml", false);
             assertViolation(messages, 
-                    new ValidationViolation("genericjcrdataid", ValidationMessageSeverity.WARN, "error1", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null),
+                    new ValidationViolation(DocumentViewParserValidatorFactory.ID, ValidationMessageSeverity.WARN, "error1", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null),
                     new ValidationViolation("jcrpathid", ValidationMessageSeverity.ERROR, "patherror", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null));
             Assert.assertEquals("Test", answer.getValue());
             Assert.assertEquals("Test", answer2.getValue());
@@ -198,6 +199,39 @@ public class ValidationExecutorTest {
     }
 
     @Test
+    public void testGenericJcrDataWithNoGenericJcrDataValidator()
+            throws URISyntaxException, IOException, SAXException, ParserConfigurationException, ConfigurationException {
+        Map<String, Validator> validators = new LinkedHashMap<>();
+        validators.put("docviewid", docViewXmlValidator);
+        validators.put("propertiesid", propertiesValidator);
+        validators.put("genericmetadataid", genericMetaInfDataValidator);
+        validators.put("genericmetadataid2", genericMetaInfDataValidator2);
+        validators.put("jcrpathid", jcrPathValidator);
+        validators.put("metainfpathid", metaInfPathValidator);
+        validators.put("nodepathid", nodePathValidator);
+        validators.put("unusedid", unusedValidator);
+        Assert.assertThrows(IllegalStateException.class, () -> new ValidationExecutor(validators));
+    }
+
+    @Test
+    public void testGenericJcrDataWithNoGenericJcrDataAndNoPathValidator()
+            throws URISyntaxException, IOException, SAXException, ParserConfigurationException, ConfigurationException {
+        Map<String, Validator> validators = new LinkedHashMap<>();
+        validators.put("docviewid", docViewXmlValidator);
+        validators.put("propertiesid", propertiesValidator);
+        validators.put("genericmetadataid", genericMetaInfDataValidator);
+        validators.put("genericmetadataid2", genericMetaInfDataValidator2);
+        validators.put("metainfpathid", metaInfPathValidator);
+        validators.put("unusedid", unusedValidator);
+        executor = new ValidationExecutor(validators);
+        try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/genericfile.xml")) {
+            Collection<ValidationViolation> messages = validate(input, executor, Paths.get(""), "apps/genericfile.xml", false);
+            MatcherAssert.assertThat(messages, AnyValidationViolationMatcher.noValidationInCollection());
+            Mockito.verify(genericJcrDataValidator, Mockito.never()).validateJcrData(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+        }
+    }
+    
+    @Test
     public void testJcrRootFolder() throws URISyntaxException, IOException, SAXException {
         Collection<ValidationViolation> messages = validateFolder(executor, Paths.get(""), "apps.dir", false);
         MatcherAssert.assertThat(messages, AnyValidationViolationMatcher.noValidationInCollection());
@@ -211,7 +245,7 @@ public class ValidationExecutorTest {
         Mockito.when(genericJcrDataValidator.done()).thenReturn(Collections.singleton(new ValidationMessage(ValidationMessageSeverity.ERROR, "test1")));
         Mockito.when(genericJcrDataValidator2.done()).thenReturn(Collections.singleton(new ValidationMessage(ValidationMessageSeverity.WARN, "test2")));
         
-        assertViolation(executor.done(), new ValidationViolation("genericjcrdataid", ValidationMessageSeverity.ERROR, "test1"), new ValidationViolation("genericjcrdataid2", ValidationMessageSeverity.WARN, "test2")); 
+        assertViolation(executor.done(), new ValidationViolation(DocumentViewParserValidatorFactory.ID, ValidationMessageSeverity.ERROR, "test1"), new ValidationViolation("genericjcrdataid2", ValidationMessageSeverity.WARN, "test2")); 
     }
 
     private Collection<ValidationViolation> validate(InputStream input, ValidationExecutor executor, Path basePath, String resourcePath,