OAK-9695 correctly evaluate property type from definition during prot…

…ected check
apache · Feb 26, 2022 · 1385c05 · 1385c05
1 parent a785a89
commit 1385c05
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 14 deletions.
diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/NodeDelegate.java
@@ -193,14 +193,14 @@ public boolean isProtected() throws InvalidItemStateException {
         return false;
     }
 
-    boolean isProtected(String property) throws InvalidItemStateException {
+    boolean isProtected(String propertyName, Type<?> propertyType) throws InvalidItemStateException {
         Tree tree = getTree();
         Tree typeRoot = sessionDelegate.getRoot().getTree(NODE_TYPES_PATH);
         List<Tree> types = TreeUtil.getEffectiveType(tree, typeRoot);
 
         boolean protectedResidual = false;
         for (Tree type : types) {
-            if (contains(TreeUtil.getNames(type, REP_PROTECTED_PROPERTIES), property)) {
+            if (contains(TreeUtil.getNames(type, REP_PROTECTED_PROPERTIES), propertyName)) {
                 return true;
             } else if (!protectedResidual) {
                 protectedResidual = TreeUtil.getBoolean(
@@ -209,17 +209,12 @@ boolean isProtected(String property) throws InvalidItemStateException {
         }
 
         // Special case: There are one or more protected *residual*
-        // child node definitions. Iterate through them to check whether
+        // property definitions. Iterate through them to check whether
         // there's a matching, protected one.
         if (protectedResidual) {
-            for (Tree type : types) {
-                Tree definitions = type.getChild(REP_RESIDUAL_PROPERTY_DEFINITIONS);
-                for (Tree definition : definitions.getChildren()) {
-                    // TODO: check for matching property type?
-                    if (TreeUtil.getBoolean(definition, JCR_PROTECTED)) {
-                        return true;
-                    }
-                }
+            Tree definition = findMatchingResidualPropertyDefinition(types, propertyType);
+            if (definition != null && TreeUtil.getBoolean(definition, JCR_PROTECTED)) {
+                return true;
             }
         }
 
@@ -617,6 +612,19 @@ private Tree findMatchingPropertyDefinition(
         }
 
         // Then look through any residual property definitions
+        return findMatchingResidualPropertyDefinition(fuzzyMatch, types, propertyType.isArray(), definedType, undefinedType, exactTypeMatch);
+    }
+
+    private Tree findMatchingResidualPropertyDefinition(List<Tree> types, Type<?> propertyType) {
+        String definedType = propertyType.toString();
+        String undefinedType = UNDEFINED.toString();
+        if (propertyType.isArray()) {
+            undefinedType = UNDEFINEDS.toString();
+        }
+        return findMatchingResidualPropertyDefinition(null, types, propertyType.isArray(), definedType, undefinedType, true);
+    }
+
+    private Tree findMatchingResidualPropertyDefinition(Tree fuzzyMatch, List<Tree> types, boolean isMultiValue, String definedType, String undefinedType, boolean exactTypeMatch) {
         for (Tree type : types) {
             Tree definitions = type.getChild(REP_RESIDUAL_PROPERTY_DEFINITIONS);
             Tree definition = definitions.getChild(definedType);
@@ -629,7 +637,7 @@ private Tree findMatchingPropertyDefinition(
             }
             if (!exactTypeMatch && fuzzyMatch == null) {
                 for (Tree def : definitions.getChildren()) {
-                    if (propertyType.isArray() == TreeUtil.getBoolean(def, JCR_MULTIPLE)) {
+                    if (isMultiValue == TreeUtil.getBoolean(def, JCR_MULTIPLE)) {
                         fuzzyMatch = def;
                         break;
                     }

diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/PropertyDelegate.java
@@ -94,7 +94,7 @@ public Status getStatus() {
     public boolean isProtected() throws InvalidItemStateException {
         NodeDelegate p = getParent();
         if (p != null) {
-            return p.isProtected(name);
+            return p.isProtected(name, getPropertyState().getType());
         } else {
             throw newInvalidItemStateException();
         }

diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/PropertyTest.java
@@ -20,16 +20,20 @@
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
 import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.ValueFormatException;
+import javax.jcr.nodetype.ConstraintViolationException;
 import javax.jcr.nodetype.NodeTypeManager;
 import javax.jcr.nodetype.NodeTypeTemplate;
 import javax.jcr.nodetype.PropertyDefinition;
 import javax.jcr.nodetype.PropertyDefinitionTemplate;
 
-import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.test.AbstractJCRTest;
+import org.apache.jackrabbit.value.ValueFactoryImpl;
+
+import com.google.common.collect.Iterators;
 
 public class PropertyTest extends AbstractJCRTest {
 
@@ -58,6 +62,12 @@ protected void setUp() throws Exception {
         pdt.setRequiredType(PropertyType.LONG);
         template.getPropertyDefinitionTemplates().add(pdt);
 
+        pdt = ntMgr.createPropertyDefinitionTemplate();
+        pdt.setName("*"); // residual property type
+        pdt.setRequiredType(PropertyType.URI);
+        pdt.setProtected(true);
+        template.getPropertyDefinitionTemplates().add(pdt);
+
         ntMgr.registerNodeType(template, true);
 
         node = testRootNode.addNode(nodeName2, NT_NAME);
@@ -274,4 +284,23 @@ public void testPropertyIteratorSize() throws Exception{
         assertEquals(3, pitr.getSize());
         assertEquals(3, Iterators.size(pitr));
     }
+
+    public void testSetAndRemoveUnprotectedProperty() throws RepositoryException {
+        Property property = node.setProperty(BOOLEAN_PROP_NAME, true);
+        assertNotNull(property);
+        property.setValue(false);
+        superuser.save();
+        property.remove();
+        superuser.save();
+    }
+
+    public void testSetProtectedResidualProperty() throws RepositoryException {
+        Value uriValue = ValueFactoryImpl.getInstance().createValue("http://example.com", PropertyType.URI);
+        try {
+            node.setProperty("test", uriValue);
+            fail("Setting protected property (according to residual property type definition) must throw ConstraintViolationException");
+        } catch (ConstraintViolationException e) {
+            // success
+        }
+    }
 }