From 1385c05026f9c84178f3942b4c5e3841af26f60a Mon Sep 17 00:00:00 2001 From: Konrad Windszus Date: Sat, 26 Feb 2022 22:03:43 +0100 Subject: [PATCH] OAK-9695 correctly evaluate property type from definition during protected check --- .../oak/jcr/delegate/NodeDelegate.java | 32 ++++++++++++------- .../oak/jcr/delegate/PropertyDelegate.java | 2 +- .../jackrabbit/oak/jcr/PropertyTest.java | 31 +++++++++++++++++- 3 files changed, 51 insertions(+), 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 index b851892be36..45735e43718 100644 --- 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 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 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 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 index 573ba758089..c0b15be9a22 100644 --- 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 index 1218a6e4abb..d564ca79a0b 100644 --- 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 + } + } } \ No newline at end of file