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 th...@apache.org on 2013/10/21 14:27:58 UTC
svn commit: r1534126 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/
Author: thomasm
Date: Mon Oct 21 12:27:58 2013
New Revision: 1534126
URL: http://svn.apache.org/r1534126
Log:
OAK-1083 Query with descendent node and access control fails to return result
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinConditionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeJoinConditionImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java Mon Oct 21 12:27:58 2013
@@ -93,4 +93,9 @@ public class ChildNodeJoinConditionImpl
// nothing to do
}
+ @Override
+ public boolean isParent(SourceImpl source) {
+ return source == parentSelector;
+ }
+
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java Mon Oct 21 12:27:58 2013
@@ -93,4 +93,9 @@ public class DescendantNodeJoinCondition
// nothing to do
}
+ @Override
+ public boolean isParent(SourceImpl source) {
+ return source == ancestorSelector;
+ }
+
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java Mon Oct 21 12:27:58 2013
@@ -142,5 +142,10 @@ public class EquiJoinConditionImpl exten
s.restrictSelector(ex);
}
}
+
+ @Override
+ public boolean isParent(SourceImpl source) {
+ return false;
+ }
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinConditionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinConditionImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinConditionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinConditionImpl.java Mon Oct 21 12:27:58 2013
@@ -51,4 +51,14 @@ public abstract class JoinConditionImpl
*/
public abstract void restrictPushDown(SelectorImpl s);
+ /**
+ * Check whether the given source is the parent of the join condition, as
+ * selector "[b]" is the parent of the join condition
+ * "isdescendantnode([a], [b])".
+ *
+ * @param source the source
+ * @return true if the source is the parent
+ */
+ public abstract boolean isParent(SourceImpl source);
+
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java Mon Oct 21 12:27:58 2013
@@ -106,9 +106,16 @@ public class JoinImpl extends SourceImpl
}
left.setQueryConstraint(queryConstraint);
right.setQueryConstraint(queryConstraint);
+ setParent(joinCondition);
right.init(query);
left.init(query);
}
+
+ @Override
+ protected void setParent(JoinConditionImpl joinCondition) {
+ left.setParent(joinCondition);
+ right.setParent(joinCondition);
+ }
@Override
public void prepare() {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeJoinConditionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeJoinConditionImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeJoinConditionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeJoinConditionImpl.java Mon Oct 21 12:27:58 2013
@@ -117,5 +117,10 @@ public class SameNodeJoinConditionImpl e
public void restrictPushDown(SelectorImpl s) {
// nothing to do
}
+
+ @Override
+ public boolean isParent(SourceImpl source) {
+ return false;
+ }
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java Mon Oct 21 12:27:58 2013
@@ -266,11 +266,32 @@ public class SelectorImpl extends Source
while (cursor != null && cursor.hasNext()) {
scanCount++;
currentRow = cursor.next();
- Tree tree = getTree(currentRow.getPath());
- if (tree == null || !tree.exists()) {
- continue;
+ if (isParent) {
+ // we must not check whether the _parent_ is readable
+ // for joins of type
+ // "select [b].[jcr:primaryType]
+ // from [nt:base] as [a]
+ // inner join [nt:base] as [b]
+ // on isdescendantnode([b], [a])
+ // where [b].[jcr:path] = $path"
+ // because if we did, we would filter out
+ // correct results
+ } else {
+ // we must check whether the _child_ is readable
+ // (even if no properties are read) for joins of type
+ // "select [a].[jcr:primaryType]
+ // from [nt:base] as [a]
+ // inner join [nt:base] as [b]
+ // on isdescendantnode([b], [a])
+ // where [a].[jcr:path] = $path"
+ // because not checking would reveal existence
+ // of the child node
+ Tree tree = getTree(currentRow.getPath());
+ if (tree == null || !tree.exists()) {
+ continue;
+ }
}
- if (!matchesAllTypes && !evaluateTypeMatch(tree)) {
+ if (!matchesAllTypes && !evaluateTypeMatch()) {
continue;
}
if (selectorCondition != null && !selectorCondition.evaluate()) {
@@ -286,7 +307,11 @@ public class SelectorImpl extends Source
return false;
}
- private boolean evaluateTypeMatch(Tree tree) {
+ private boolean evaluateTypeMatch() {
+ Tree tree = getTree(currentRow.getPath());
+ if (tree == null || !tree.exists()) {
+ return false;
+ }
PropertyState primary = tree.getProperty(JCR_PRIMARYTYPE);
if (primary != null && primary.getType() == NAME) {
String name = primary.getValue(NAME);
@@ -401,4 +426,11 @@ public class SelectorImpl extends Source
return selectorName.hashCode();
}
+ @Override
+ protected void setParent(JoinConditionImpl joinCondition) {
+ if (joinCondition.isParent(this)) {
+ isParent = true;
+ }
+ }
+
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java Mon Oct 21 12:27:58 2013
@@ -69,6 +69,13 @@ public abstract class SourceImpl extends
protected boolean outerJoinRightHandSide;
/**
+ * Whether this selector is the parent of a descendent or parent-child join.
+ * Access rights don't need to be checked in such selectors (unless there
+ * are conditions on the selector).
+ */
+ protected boolean isParent;
+
+ /**
* Set the complete constraint of the query (the WHERE ... condition).
*
* @param queryConstraint the constraint
@@ -161,4 +168,6 @@ public abstract class SourceImpl extends
*/
public abstract boolean next();
+ abstract void setParent(JoinConditionImpl joinCondition);
+
}
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java?rev=1534126&r1=1534125&r2=1534126&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/CompatibilityIssuesTest.java Mon Oct 21 12:27:58 2013
@@ -28,11 +28,14 @@ import java.io.IOException;
import java.io.ObjectOutputStream;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import javax.jcr.Credentials;
+import javax.jcr.GuestCredentials;
import javax.jcr.InvalidItemStateException;
import javax.jcr.ItemExistsException;
import javax.jcr.Node;
@@ -40,6 +43,8 @@ import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.SimpleCredentials;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
import javax.jcr.nodetype.ConstraintViolationException;
import javax.jcr.nodetype.NodeDefinition;
import javax.jcr.nodetype.NodeDefinitionTemplate;
@@ -51,11 +56,21 @@ import javax.jcr.observation.Event;
import javax.jcr.observation.EventIterator;
import javax.jcr.observation.EventListener;
import javax.jcr.observation.ObservationManager;
+import javax.jcr.query.Query;
+import javax.jcr.query.QueryManager;
+import javax.jcr.query.QueryResult;
+import javax.jcr.query.RowIterator;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.Privilege;
import junit.framework.Assert;
import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.api.JackrabbitRepository;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -327,4 +342,28 @@ public class CompatibilityIssuesTest ext
return out;
}
+ @Test
+ public void testSearchDescendentUsingXPath() throws Exception {
+ Session adminSession = getAdminSession();
+ String testNodePath = "/home/users/geometrixx-outdoors/emily.andrews@mailinator.com/social/relationships/following/aaron.mcdonald@mailinator.com";
+ Node testNode = JcrUtils.getOrCreateByPath(testNodePath, null, adminSession);
+ testNode.setProperty("id", "aaron.mcdonald@mailinator.com");
+
+ AccessControlManager acMgr = adminSession.getAccessControlManager();
+ JackrabbitAccessControlList tmpl = AccessControlUtils.getAccessControlList(acMgr, "/home/users/geometrixx-outdoors");
+ ValueFactory vf = adminSession.getValueFactory();
+ Map<String, Value> restrictions = new HashMap<String, Value>();
+ restrictions.put("rep:glob", vf.createValue("*/social/relationships/following/*"));
+ tmpl.addEntry(EveryonePrincipal.getInstance(), new Privilege[]{acMgr.privilegeFromName(Privilege.JCR_READ)}, true, restrictions);
+ acMgr.setPolicy(tmpl.getPath(), tmpl);
+ adminSession.save();
+
+ Session anonymousSession = getRepository().login(new GuestCredentials());
+ QueryManager qm = anonymousSession.getWorkspace().getQueryManager();
+ Query q = qm.createQuery("/jcr:root/home//social/relationships/following//*[id='aaron.mcdonald@mailinator.com']", Query.XPATH);
+ QueryResult r = q.execute();
+ RowIterator it = r.getRows();
+ Assert.assertTrue(it.hasNext());
+ }
+
}