Index: oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java (revision )
+++ oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImplTest.java (revision )
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authorization.permission;
+
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nonnull;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Sets;
+import junit.framework.Assert;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+
+public class PermissionEntryProviderImplTest {
+
+ private final String GROUP_LONG_MAX = "groupLongMax";
+ private final String GROUP_LONG_MAX_MINUS_10 = "groupLongMaxMinus10";
+ private final String GROUP_50 = "group50";
+
+ /**
+ * @see OAK-2465
+ */
+ @Test
+ public void testInitLongOverflow() throws Exception {
+ MockPermissionStore store = new MockPermissionStore();
+ PermissionEntryCache cache = new MockPermissionEntryCache();
+ Set principalNames = ImmutableSet.of(GROUP_LONG_MAX);
+
+ /*
+ create a new PermissionEntryProviderImpl to have it's #init() method
+ called, which may trigger the cache to be pre-filled if the max number
+ if entries is not exceeded -> in case of PermissionStore#getNumEntries
+ return Long.MAX_VALUE the cache should not be filled (-> the mock-cache
+ implementation will fail.
+ */
+ PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, cache, principalNames, ConfigurationParameters.EMPTY);
+
+ // test that PermissionEntryProviderImpl.existingNames nevertheless is
+ // properly filled with all principal names for which permission entries exist
+ assertEquals(principalNames, store.existingNames);
+ assertNotSame(Iterators.emptyIterator(), provider.getEntryIterator(new EntryPredicate()));
+
+ }
+
+ /**
+ * @see OAK-2465
+ */
+ @Test
+ public void testInitLongOverflow2() throws Exception {
+ MockPermissionStore store = new MockPermissionStore();
+ PermissionEntryCache cache = new MockPermissionEntryCache();
+ Set principalNames = ImmutableSet.of(GROUP_LONG_MAX_MINUS_10, GROUP_50);
+
+ /*
+ create a new PermissionEntryProviderImpl to have it's #init() method
+ called, which may trigger the cache to be pre-filled if the max number
+ if entries is not exceeded -> still counting up the number of permission
+ entries must deal with the fact that the counter may become bigger that
+ Long.MAX_VALUE
+ */
+ PermissionEntryProviderImpl provider = new PermissionEntryProviderImpl(store, cache, principalNames, ConfigurationParameters.EMPTY);
+ assertEquals(principalNames, store.existingNames);
+ assertNotSame(Iterators.emptyIterator(), provider.getEntryIterator(new EntryPredicate()));
+ }
+
+ /**
+ * @see OAK-2465
+ */
+ @Test
+ public void testExistingNamesAndLongOverFlow() throws Exception {
+ MockPermissionStore store = new MockPermissionStore();
+ PermissionEntryCache cache = new MockPermissionEntryCache();
+ Set principalNames = Sets.newHashSet(GROUP_LONG_MAX_MINUS_10, GROUP_50, "noEntries");
+
+ /*
+ same as before but principal-set contains a name for which not entries
+ exist -> the 'existingNames' set must properly reflect that
+ */
+ new PermissionEntryProviderImpl(store, cache, principalNames, ConfigurationParameters.EMPTY);
+
+ assertFalse(principalNames.equals(store.existingNames));
+ assertEquals(2, store.existingNames.size());
+
+ principalNames.remove("noEntries");
+ assertEquals(principalNames, store.existingNames);
+ }
+
+ // Inner Classes
+ private class MockPermissionStore implements PermissionStore {
+
+ // corresponds to private set in PermissionEntryProviderImpl that contains
+ // those principal names for which actually any entries exist
+ private Set existingNames = new HashSet();
+
+ @Override
+ public Collection load(
+ Collection entries, String principalName,
+ String path) {
+ return null;
+ }
+
+ @Override
+ public void load(Map> entries,
+ String principalName) {
+ // ignore
+ }
+
+ @Override
+ public PrincipalPermissionEntries load(String principalName) {
+ return new PrincipalPermissionEntries(principalName);
+ }
+
+ @Override
+ public long getNumEntries(String principalName, long max) {
+ long cnt = 0;
+ if (GROUP_LONG_MAX_MINUS_10.equals(principalName)) {
+ cnt = Long.MAX_VALUE - 10;
+ } else if (GROUP_50.equals(principalName)) {
+ cnt = 50;
+ } else if (GROUP_LONG_MAX.equals(principalName)) {
+ cnt = Long.MAX_VALUE;
+ }
+ if (cnt > 0) {
+ existingNames.add(principalName);
+ }
+ return cnt;
+ }
+
+ }
+
+ private class MockPermissionEntryCache extends PermissionEntryCache {
+ @Override
+ public void load(@Nonnull PermissionStore store,
+ @Nonnull Map> pathEntryMap,
+ @Nonnull String principalName) {
+ Assert.fail("The number of entries exceeds the max cache size");
+ }
+ }
+}
Index: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (revision 1655361)
+++ oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java (revision )
@@ -39,18 +39,29 @@
private static final long DEFAULT_SIZE = 250;
+ /**
+ * The set of principal names for which this {@code PermissionEntryProvider}
+ * has been created.
+ */
private final Set principalNames;
+ /**
+ * The set of principal names for which the store contains any permission
+ * entries. This set is equals or just a subset of the {@code principalNames}
+ * defined above. The methods collecting the entries will shortcut in case
+ * this set is empty and thus no permission entries exist for the specified
+ * set of principal.
+ */
private final Set existingNames = new HashSet();
- private Map> pathEntryMap;
-
private final PermissionStore store;
private final PermissionEntryCache cache;
private final long maxSize;
+ private Map> pathEntryMap;
+
PermissionEntryProviderImpl(@Nonnull PermissionStore store, @Nonnull PermissionEntryCache cache,
@Nonnull Set principalNames, @Nonnull ConfigurationParameters options) {
this.store = store;
@@ -63,17 +74,32 @@
private void init() {
long cnt = 0;
existingNames.clear();
- for (String name: principalNames) {
+ for (String name : principalNames) {
+ /*
+ Calculate the total number of permission entries (cnt) defined for the
+ given set of principals in order to be able to determine if the cache
+ should be loaded upfront.
+ Note however that cache.getNumEntries (n) may return Long.MAX_VALUE
+ if the underlying implementation does not know the exact value, and
+ the child node count is higher than maxSize (see OAK-2465).
+ */
long n = cache.getNumEntries(store, name, maxSize);
+ if (Long.MAX_VALUE == n || Long.MAX_VALUE == cnt) {
+ cnt = Long.MAX_VALUE;
+ } else {
- cnt+= n;
+ cnt += n;
+ }
+ // if cache.getNumEntries (n) returns a number bigger than 0, we
+ // add this principal name to the 'existingNames'
if (n > 0) {
existingNames.add(name);
}
}
- if (cnt < maxSize) {
- // cache all entries of all principals
+ if (cnt > 0 && cnt < maxSize) {
+ // the total number of entries is smaller that maxSize, so we can
+ // cache all entries of all principals having any entries right away
pathEntryMap = new HashMap>();
- for (String name: principalNames) {
+ for (String name : existingNames) {
cache.load(store, pathEntryMap, name);
}
} else {
@@ -124,7 +150,7 @@
@Nonnull
private Collection loadEntries(@Nonnull String path) {
Collection ret = new TreeSet();
- for (String name: existingNames) {
+ for (String name : existingNames) {
cache.load(store, ret, name, path);
}
return ret;
\ No newline at end of file