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/04/08 16:07:47 UTC

svn commit: r1465644 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: core/SecureNodeState.java spi/state/AbstractNodeState.java

Author: jukka
Date: Mon Apr  8 14:07:47 2013
New Revision: 1465644

URL: http://svn.apache.org/r1465644
Log:
OAK-708: SecureNodeState#getChildNodeCount and #getPropertyCount: don't respect read permissions

Streamline SecureNodeState.getPropertyCount() to use the minimum amount of work when possible.

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java?rev=1465644&r1=1465643&r2=1465644&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java Mon Apr  8 14:07:47 2013
@@ -24,6 +24,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.api.PropertyState;
@@ -87,11 +88,10 @@ public class SecureNodeState extends Abs
         return getReadStatus().includes(ReadStatus.ALLOW_THIS);
     }
 
-    @Override
-    @CheckForNull
+    @Override @CheckForNull
     public PropertyState getProperty(String name) {
         PropertyState property = state.getProperty(name);
-        if (canReadProperty(property)) {
+        if (property != null && canReadProperty(property)) {
             return property;
         } else {
             return null;
@@ -99,9 +99,17 @@ public class SecureNodeState extends Abs
     }
 
     @Override
-    public long getPropertyCount() {
-        if (propertyCount == -1){
-            propertyCount = super.getPropertyCount();
+    public synchronized long getPropertyCount() {
+        if (propertyCount == -1) {
+            ReadStatus rs = getReadStatus();
+            if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
+                propertyCount = state.getPropertyCount();
+            } else if (rs.includes(ReadStatus.DENY_PROPERTIES)) {
+                propertyCount = 0;
+            } else {
+                propertyCount = count(Iterables.filter(
+                        state.getProperties(), isPropertyReadable()));
+            }
         }
         return propertyCount;
     }
@@ -110,14 +118,13 @@ public class SecureNodeState extends Abs
     @Override
     public Iterable<? extends PropertyState> getProperties() {
         ReadStatus rs = getReadStatus();
-        Iterable<? extends PropertyState> properties = state.getProperties();
         if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
-            return properties;
+            return state.getProperties();
         } else if (rs.includes(ReadStatus.DENY_PROPERTIES)) {
             return Collections.emptySet();
         } else {
-            Iterable<PropertyState> readable = Iterables.transform(properties, new ReadableProperties());
-            return Iterables.filter(readable, Predicates.notNull());
+            return Iterables.filter(
+                    state.getProperties(), isPropertyReadable());
         }
     }
 
@@ -138,7 +145,7 @@ public class SecureNodeState extends Abs
     }
 
     @Override
-    public long getChildNodeCount() {
+    public synchronized long getChildNodeCount() {
         if (childNodeCount == -1) {
             childNodeCount = super.getChildNodeCount();
         }
@@ -192,10 +199,7 @@ public class SecureNodeState extends Abs
         return readStatus;
     }
 
-    private boolean canReadProperty(@Nullable PropertyState property) {
-        if (property == null) {
-            return false;
-        }
+    private boolean canReadProperty(@Nonnull PropertyState property) {
         if (readStatus == null || readStatus.appliesToThis()) {
             ReadStatus rs = permissionProvider.getReadStatus(this.base, property);
             if (rs.appliesToThis()) {
@@ -208,15 +212,18 @@ public class SecureNodeState extends Abs
         return readStatus.includes(ReadStatus.ALLOW_PROPERTIES);
     }
 
-    private class ReadableProperties implements Function<PropertyState, PropertyState> {
-        @Override
-        public PropertyState apply(PropertyState property) {
-            if (canReadProperty(property)) {
-                return property;
-            } else {
-                return null;
+    /**
+     * Returns a predicate for testing whether a given property is readable.
+     *
+     * @return predicate
+     */
+    private Predicate<PropertyState> isPropertyReadable() {
+        return new Predicate<PropertyState>() {
+            @Override
+            public boolean apply(@Nonnull PropertyState property) {
+                return canReadProperty(property);
             }
-        }
+        };
     }
 
     private class ReadableChildNodeEntries implements Function<ChildNodeEntry, ChildNodeEntry> {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java?rev=1465644&r1=1465643&r2=1465644&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeState.java Mon Apr  8 14:07:47 2013
@@ -234,7 +234,7 @@ public abstract class AbstractNodeState 
 
     //-----------------------------------------------------------< private >--
 
-    private static long count(Iterable<?> iterable) {
+    protected static long count(Iterable<?> iterable) {
         long n = 0;
         Iterator<?> iterator = iterable.iterator();
         while (iterator.hasNext()) {