You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2013/05/17 12:12:29 UTC

svn commit: r1483721 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/nodetype/ test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/

Author: jukka
Date: Fri May 17 10:12:28 2013
New Revision: 1483721

URL: http://svn.apache.org/r1483721
Log:
OAK-823: Missing validation for jcr:uuid property

As noted by Alex, the validator shouldn't cause code that removes and then re-adds a referenceable node to fail.
More generally, the validator should just check the consistency of the *after* state, without considering how that state was reached.

Also make the custom jcr:uuid processing only apply to mix:referenceable nodes.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java?rev=1483721&r1=1483720&r2=1483721&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/EffectiveType.java Fri May 17 10:12:28 2013
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.plugin
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.collect.Iterables.addAll;
 import static com.google.common.collect.Iterables.concat;
+import static com.google.common.collect.Iterables.contains;
 import static com.google.common.collect.Lists.newArrayListWithCapacity;
 import static com.google.common.collect.Sets.newHashSet;
 import static org.apache.jackrabbit.JcrConstants.JCR_DEFAULTPRIMARYTYPE;
@@ -66,6 +67,23 @@ class EffectiveType {
                 CONSTRAINT, code, path + this + ": " + message);
     }
 
+    /**
+     * Checks whether this effective type contains the named type.
+     *
+     * @param name node type name
+     * @return {@code true} if the named type is included,
+     *         {@code false} otherwise
+     */
+    boolean isNodeType(String name) {
+        for (NodeState type : types) {
+            if (name.equals(type.getName(JCR_NODETYPENAME))
+                    || contains(type.getNames(OAK_SUPERTYPES), name)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     @Nonnull
     Set<String> findMissingMandatoryItems(NodeState node) {
         ImmutableSet.Builder<String> builder = ImmutableSet.builder();

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java?rev=1483721&r1=1483720&r2=1483721&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/TypeEditor.java Fri May 17 10:12:28 2013
@@ -25,7 +25,6 @@ import com.google.common.base.Predicate;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
-import org.apache.jackrabbit.oak.core.IdentifierManager;
 import org.apache.jackrabbit.oak.plugins.value.ValueFactoryImpl;
 import org.apache.jackrabbit.oak.spi.commit.DefaultEditor;
 import org.apache.jackrabbit.oak.spi.commit.Editor;
@@ -34,14 +33,15 @@ import org.apache.jackrabbit.oak.spi.sta
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
-import static org.apache.jackrabbit.JcrConstants.JCR_NAME;
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.apache.jackrabbit.JcrConstants.JCR_REQUIREDTYPE;
 import static org.apache.jackrabbit.JcrConstants.JCR_UUID;
 import static org.apache.jackrabbit.JcrConstants.JCR_VALUECONSTRAINTS;
+import static org.apache.jackrabbit.JcrConstants.MIX_REFERENCEABLE;
 import static org.apache.jackrabbit.oak.api.Type.NAME;
 import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.api.Type.STRINGS;
+import static org.apache.jackrabbit.oak.core.IdentifierManager.isValidUUID;
 import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE;
 import static org.apache.jackrabbit.oak.plugins.nodetype.constraint.Constraints.valueConstraint;
 
@@ -113,14 +113,9 @@ class TypeEditor extends DefaultEditor {
     }
 
     @Override
-    public void propertyAdded(PropertyState after) throws CommitFailedException {
-        NodeState definition = effective.getDefinition(after);
-        if (definition == null) {
-            throw constraintViolation(
-                    3, "No matching property definition found for " + after);
-        }
-        checkUUIDCreation(definition, after);
-        checkValueConstraints(definition, after);
+    public void propertyAdded(PropertyState after)
+            throws CommitFailedException {
+        propertyChanged(null, after);
     }
 
     @Override
@@ -130,9 +125,17 @@ class TypeEditor extends DefaultEditor {
         if (definition == null) {
             throw constraintViolation(
                     4, "No matching property definition found for " + after);
+        } else if (JCR_UUID.equals(after.getName())
+                && effective.isNodeType(MIX_REFERENCEABLE)) {
+            // special handling for the jcr:uuid property of mix:referenceable
+            // TODO: this should be done in a pluggable extension
+            if (!isValidUUID(after.getValue(Type.STRING))) {
+                throw constraintViolation(
+                        12, "Invalid UUID value in the jcr:uuid property");
+            }
+        } else {
+            checkValueConstraints(definition, after);
         }
-        checkUUIDModification(definition, after);
-        checkValueConstraints(definition, after);
     }
 
     @Override
@@ -234,20 +237,4 @@ class TypeEditor extends DefaultEditor {
         throw constraintViolation(5, "Value constraint violation in " + property);
     }
 
-    private void checkUUIDCreation(NodeState definition, PropertyState property) throws CommitFailedException {
-        if (isJcrUuid(definition, property) && !IdentifierManager.isValidUUID(property.getValue(Type.STRING))) {
-            throw constraintViolation(12, "Invalid UUID in jcr:uuid property.");
-        }
-    }
-
-    private void checkUUIDModification(NodeState definition, PropertyState property) throws CommitFailedException {
-        if (isJcrUuid(definition, property)) {
-            throw constraintViolation(13, "jcr:uuid property cannot be modified.");
-        }
-    }
-
-    private static boolean isJcrUuid(NodeState definition, PropertyState property) {
-        return JCR_UUID.equals(property.getName()) && JCR_UUID.equals(definition.getName(JCR_NAME));
-    }
-
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java?rev=1483721&r1=1483720&r2=1483721&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/JcrUUIDTest.java Fri May 17 10:12:28 2013
@@ -174,7 +174,7 @@ public class JcrUUIDTest extends Abstrac
             fail("An attempt to change the jcr:uuid property must fail");
         } catch (CommitFailedException e) {
             assertEquals(CommitFailedException.CONSTRAINT, e.getType());
-            assertEquals(13, e.getCode());
+            assertEquals(12, e.getCode());
         }
     }