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 an...@apache.org on 2020/09/30 07:13:46 UTC
svn commit: r1882147 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/security/user/query/
test/java/org/apache/jackrabbit/oak/security/user/query/
Author: angela
Date: Wed Sep 30 07:13:46 2020
New Revision: 1882147
URL: http://svn.apache.org/viewvc?rev=1882147&view=rev
Log:
OAK-9243 : avoid potential NPE with Condition.Property
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/Condition.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ConditionVisitor.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitor.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilder.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilderTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/Condition.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/Condition.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/Condition.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/Condition.java Wed Sep 30 07:13:46 2020
@@ -16,6 +16,8 @@
*/
package org.apache.jackrabbit.oak.security.user.query;
+import org.jetbrains.annotations.NotNull;
+
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -25,72 +27,94 @@ import javax.jcr.Value;
interface Condition {
- void accept(ConditionVisitor visitor) throws RepositoryException;
+ void accept(@NotNull ConditionVisitor visitor) throws RepositoryException;
//-----------------------------------------------------< Node Condition >---
class Node implements Condition {
private final String pattern;
- public Node(String pattern) {
+ public Node(@NotNull String pattern) {
this.pattern = pattern;
}
+ @NotNull
public String getPattern() {
return pattern;
}
- public void accept(ConditionVisitor visitor) {
+ public void accept(@NotNull ConditionVisitor visitor) {
visitor.visit(this);
}
}
//-------------------------------------------------< Property Condition >---
- class Property implements Condition {
+ abstract class Property implements Condition {
private final String relPath;
private final RelationOp op;
- private final Value value;
- private final String pattern;
-
- public Property(String relPath, RelationOp op, Value value) {
- this.relPath = relPath;
- this.op = op;
- this.value = value;
- pattern = null;
- }
- public Property(String relPath, RelationOp op, String pattern) {
+ private Property(@NotNull String relPath, @NotNull RelationOp op) {
this.relPath = relPath;
this.op = op;
- value = null;
- this.pattern = pattern;
- }
-
- public Property(String relPath, RelationOp op) {
- this.relPath = relPath;
- this.op = op;
- value = null;
- pattern = null;
}
+ @NotNull
public String getRelPath() {
return relPath;
}
+ @NotNull
public RelationOp getOp() {
return op;
}
+ }
+
+ class PropertyValue extends Property {
+ private final Value value;
+
+ PropertyValue(@NotNull String relPath, @NotNull RelationOp op, @NotNull Value value) {
+ super(relPath, op);
+ this.value = value;
+ }
+
+ @NotNull
public Value getValue() {
return value;
}
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
+ visitor.visit(this);
+ }
+ }
+
+ class PropertyLike extends Property {
+
+ private final String pattern;
+
+ PropertyLike(@NotNull String relPath, @NotNull String pattern) {
+ super(relPath, RelationOp.LIKE);
+ this.pattern = pattern;
+ }
+
+ @NotNull
public String getPattern() {
return pattern;
}
- public void accept(ConditionVisitor visitor) throws RepositoryException {
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
+ visitor.visit(this);
+ }
+ }
+
+ class PropertyExists extends Property {
+
+ PropertyExists(@NotNull String relPath) {
+ super(relPath, RelationOp.EX);
+ }
+
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
visitor.visit(this);
}
}
@@ -101,20 +125,22 @@ interface Condition {
private final String relPath;
private final String searchExpr;
- public Contains(String relPath, String searchExpr) {
+ public Contains(@NotNull String relPath, @NotNull String searchExpr) {
this.relPath = relPath;
this.searchExpr = searchExpr;
}
+ @NotNull
public String getRelPath() {
return relPath;
}
+ @NotNull
public String getSearchExpr() {
return searchExpr;
}
- public void accept(ConditionVisitor visitor) {
+ public void accept(@NotNull ConditionVisitor visitor) {
visitor.visit(this);
}
}
@@ -124,15 +150,16 @@ interface Condition {
private final String name;
- public Impersonation(String name) {
+ public Impersonation(@NotNull String name) {
this.name = name;
}
+ @NotNull
public String getName() {
return name;
}
- public void accept(ConditionVisitor visitor) {
+ public void accept(@NotNull ConditionVisitor visitor) {
visitor.visit(this);
}
}
@@ -142,15 +169,16 @@ interface Condition {
private final Condition condition;
- public Not(Condition condition) {
+ public Not(@NotNull Condition condition) {
this.condition = condition;
}
+ @NotNull
public Condition getCondition() {
return condition;
}
- public void accept(ConditionVisitor visitor) throws RepositoryException {
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
visitor.visit(this);
}
}
@@ -160,11 +188,12 @@ interface Condition {
private final List<Condition> conditions = new ArrayList<>();
- Compound(Condition condition1, Condition condition2) {
+ Compound(@NotNull Condition condition1, @NotNull Condition condition2) {
conditions.add(condition1);
conditions.add(condition2);
}
+ @NotNull
public Iterator<Condition> iterator() {
return conditions.iterator();
}
@@ -173,11 +202,11 @@ interface Condition {
//------------------------------------------------------< And Condition >---
class And extends Compound {
- public And(Condition condition1, Condition condition2) {
+ public And(@NotNull Condition condition1, @NotNull Condition condition2) {
super(condition1, condition2);
}
- public void accept(ConditionVisitor visitor) throws RepositoryException {
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
visitor.visit(this);
}
}
@@ -185,11 +214,11 @@ interface Condition {
//-------------------------------------------------------< Or Condition >---
class Or extends Compound {
- public Or(Condition condition1, Condition condition2) {
+ public Or(@NotNull Condition condition1, @NotNull Condition condition2) {
super(condition1, condition2);
}
- public void accept(ConditionVisitor visitor) throws RepositoryException {
+ public void accept(@NotNull ConditionVisitor visitor) throws RepositoryException {
visitor.visit(this);
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ConditionVisitor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ConditionVisitor.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ConditionVisitor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ConditionVisitor.java Wed Sep 30 07:13:46 2020
@@ -16,21 +16,27 @@
*/
package org.apache.jackrabbit.oak.security.user.query;
+import org.jetbrains.annotations.NotNull;
+
import javax.jcr.RepositoryException;
interface ConditionVisitor {
- void visit(Condition.Node node);
+ void visit(@NotNull Condition.Node node);
+
+ void visit(@NotNull Condition.PropertyValue condition) throws RepositoryException;
+
+ void visit(@NotNull Condition.PropertyLike condition) throws RepositoryException;
- void visit(Condition.Property condition) throws RepositoryException;
+ void visit(@NotNull Condition.PropertyExists condition) throws RepositoryException;
- void visit(Condition.Contains condition);
+ void visit(@NotNull Condition.Contains condition);
- void visit(Condition.Impersonation condition);
+ void visit(@NotNull Condition.Impersonation condition);
- void visit(Condition.Not condition) throws RepositoryException;
+ void visit(@NotNull Condition.Not condition) throws RepositoryException;
- void visit(Condition.And condition) throws RepositoryException;
+ void visit(@NotNull Condition.And condition) throws RepositoryException;
- void visit(Condition.Or condition) throws RepositoryException;
+ void visit(@NotNull Condition.Or condition) throws RepositoryException;
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java Wed Sep 30 07:13:46 2020
@@ -16,20 +16,18 @@
*/
package org.apache.jackrabbit.oak.security.user.query;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Set;
-import javax.jcr.RepositoryException;
-
import com.google.common.base.Predicate;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+
+import javax.jcr.RepositoryException;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Set;
import static org.apache.jackrabbit.oak.security.user.query.QueryUtil.getID;
@@ -38,8 +36,6 @@ import static org.apache.jackrabbit.oak.
*/
class GroupPredicate implements Predicate<Authorizable> {
- private static final Logger log = LoggerFactory.getLogger(GroupPredicate.class);
-
private final Iterator<Authorizable> membersIterator;
private final Set<String> memberIds = new HashSet<>();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitor.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitor.java Wed Sep 30 07:13:46 2020
@@ -25,6 +25,7 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
+import org.jetbrains.annotations.NotNull;
/**
* XPATH based condition visitor.
@@ -35,8 +36,8 @@ class XPathConditionVisitor implements C
private final NamePathMapper namePathMapper;
private final UserManager userMgr;
- XPathConditionVisitor(StringBuilder statement, NamePathMapper namePathMapper,
- UserManager userMgr) {
+ XPathConditionVisitor(@NotNull StringBuilder statement, @NotNull NamePathMapper namePathMapper,
+ @NotNull UserManager userMgr) {
this.statement = statement;
this.namePathMapper = namePathMapper;
this.userMgr = userMgr;
@@ -44,7 +45,7 @@ class XPathConditionVisitor implements C
//---------------------------------------------------< ConditionVisitor >---
@Override
- public void visit(Condition.Node condition) {
+ public void visit(@NotNull Condition.Node condition) {
statement.append('(')
.append("jcr:like(@")
.append(QueryUtil.escapeForQuery(UserConstants.REP_AUTHORIZABLE_ID, namePathMapper))
@@ -65,25 +66,28 @@ class XPathConditionVisitor implements C
}
@Override
- public void visit(Condition.Property condition) throws RepositoryException {
- RelationOp relOp = condition.getOp();
- if (relOp == RelationOp.EX) {
- statement.append(QueryUtil.escapeForQuery(condition.getRelPath()));
- } else if (relOp == RelationOp.LIKE) {
- statement.append("jcr:like(")
- .append(QueryUtil.escapeForQuery(condition.getRelPath()))
- .append(",'")
- .append(QueryUtil.escapeForQuery(condition.getPattern()))
- .append("')");
- } else {
- statement.append(QueryUtil.escapeForQuery(condition.getRelPath()))
- .append(condition.getOp().getOp())
- .append(QueryUtil.format(condition.getValue()));
- }
+ public void visit(@NotNull Condition.PropertyValue condition) throws RepositoryException {
+ statement.append(QueryUtil.escapeForQuery(condition.getRelPath()))
+ .append(condition.getOp().getOp())
+ .append(QueryUtil.format(condition.getValue()));
+ }
+
+ @Override
+ public void visit(Condition.@NotNull PropertyLike condition) {
+ statement.append("jcr:like(")
+ .append(QueryUtil.escapeForQuery(condition.getRelPath()))
+ .append(",'")
+ .append(QueryUtil.escapeForQuery(condition.getPattern()))
+ .append("')");
+ }
+
+ @Override
+ public void visit(Condition.@NotNull PropertyExists condition) {
+ statement.append(QueryUtil.escapeForQuery(condition.getRelPath()));
}
@Override
- public void visit(Condition.Contains condition) {
+ public void visit(@NotNull Condition.Contains condition) {
statement.append("jcr:contains(")
.append(QueryUtil.escapeForQuery(condition.getRelPath()))
.append(",'")
@@ -92,7 +96,7 @@ class XPathConditionVisitor implements C
}
@Override
- public void visit(Condition.Impersonation condition) {
+ public void visit(@NotNull Condition.Impersonation condition) {
String principalName = condition.getName();
boolean isAdmin = false;
try {
@@ -117,14 +121,14 @@ class XPathConditionVisitor implements C
}
@Override
- public void visit(Condition.Not condition) throws RepositoryException {
+ public void visit(@NotNull Condition.Not condition) throws RepositoryException {
statement.append("not(");
condition.getCondition().accept(this);
statement.append(')');
}
@Override
- public void visit(Condition.And condition) throws RepositoryException {
+ public void visit(@NotNull Condition.And condition) throws RepositoryException {
int count = 0;
for (Condition c : condition) {
statement.append(count++ > 0 ? " and " : "");
@@ -133,7 +137,7 @@ class XPathConditionVisitor implements C
}
@Override
- public void visit(Condition.Or condition) throws RepositoryException {
+ public void visit(@NotNull Condition.Or condition) throws RepositoryException {
int pos = statement.length();
int count = 0;
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilder.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilder.java Wed Sep 30 07:13:46 2020
@@ -99,49 +99,49 @@ class XPathQueryBuilder implements Query
@NotNull
@Override
public Condition neq(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.NE, value);
+ return new Condition.PropertyValue(relPath, RelationOp.NE, value);
}
@NotNull
@Override
public Condition eq(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.EQ, value);
+ return new Condition.PropertyValue(relPath, RelationOp.EQ, value);
}
@NotNull
@Override
public Condition lt(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.LT, value);
+ return new Condition.PropertyValue(relPath, RelationOp.LT, value);
}
@NotNull
@Override
public Condition le(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.LE, value);
+ return new Condition.PropertyValue(relPath, RelationOp.LE, value);
}
@NotNull
@Override
public Condition gt(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.GT, value);
+ return new Condition.PropertyValue(relPath, RelationOp.GT, value);
}
@NotNull
@Override
public Condition ge(@NotNull String relPath, @NotNull Value value) {
- return new Condition.Property(relPath, RelationOp.GE, value);
+ return new Condition.PropertyValue(relPath, RelationOp.GE, value);
}
@NotNull
@Override
public Condition exists(@NotNull String relPath) {
- return new Condition.Property(relPath, RelationOp.EX);
+ return new Condition.PropertyExists(relPath);
}
@NotNull
@Override
public Condition like(@NotNull String relPath, @NotNull String pattern) {
- return new Condition.Property(relPath, RelationOp.LIKE, pattern);
+ return new Condition.PropertyLike(relPath, pattern);
}
@NotNull
@@ -175,8 +175,8 @@ class XPathQueryBuilder implements Query
}
//-----------------------------------------------------------< internal >---
- Condition property(String relPath, RelationOp op, Value value) {
- return new Condition.Property(relPath, op, value);
+ Condition property(@NotNull String relPath, @NotNull RelationOp op, @NotNull Value value) {
+ return new Condition.PropertyValue(relPath, op, value);
}
AuthorizableType getSelectorType() {
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java Wed Sep 30 07:13:46 2020
@@ -89,7 +89,7 @@ public class XPathConditionVisitorTest e
continue;
}
- visitor.visit(new Condition.Property(REL_PATH, op, v));
+ visitor.visit(new Condition.PropertyValue(REL_PATH, op, v));
String s = statement.toString();
String expected = QueryUtils.escapeForQuery(REL_PATH) + op.getOp() + QueryUtil.format(v);
@@ -102,15 +102,15 @@ public class XPathConditionVisitorTest e
}
@Test
- public void testVisitPropertyExists() throws Exception {
- visitor.visit((Condition.Property) new XPathQueryBuilder().exists(REL_PATH));
+ public void testVisitPropertyExists() {
+ visitor.visit((Condition.PropertyExists) new XPathQueryBuilder().exists(REL_PATH));
assertEquals(QueryUtils.escapeForQuery(REL_PATH), statement.toString());
}
@Test
- public void testVisitPropertyLike() throws Exception {
- visitor.visit((Condition.Property) new XPathQueryBuilder().like(REL_PATH, SERACH_EXPR));
+ public void testVisitPropertyLike() {
+ visitor.visit((Condition.PropertyLike) new XPathQueryBuilder().like(REL_PATH, SERACH_EXPR));
String s = statement.toString();
assertTrue(s.startsWith("jcr:like("));
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilderTest.java?rev=1882147&r1=1882146&r2=1882147&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathQueryBuilderTest.java Wed Sep 30 07:13:46 2020
@@ -44,14 +44,13 @@ public class XPathQueryBuilderTest exten
private final String relPath = "re/l/path";
private void assertPropertyCondition(@NotNull Condition condition, @NotNull RelationOp expectedOp) {
- assertTrue(condition instanceof Condition.Property);
- Condition.Property cp = (Condition.Property) condition;
+ assertTrue(condition instanceof Condition.PropertyValue);
+ Condition.PropertyValue cp = (Condition.PropertyValue) condition;
assertSame(expectedOp, cp.getOp());
assertEquals(relPath, cp.getRelPath());
assertEquals(v, cp.getValue());
- assertNull(cp.getPattern());
assertNull(builder.getCondition());
}
@@ -230,27 +229,24 @@ public class XPathQueryBuilderTest exten
public void testExists() {
Condition c = builder.exists(relPath);
- assertTrue(c instanceof Condition.Property);
- Condition.Property cp = (Condition.Property) c;
+ assertTrue(c instanceof Condition.PropertyExists);
+ Condition.PropertyExists cp = (Condition.PropertyExists) c;
assertSame(RelationOp.EX, cp.getOp());
assertEquals(relPath, cp.getRelPath());
- assertNull(cp.getValue());
- assertNull(cp.getPattern());
}
@Test
public void testLike() {
Condition c = builder.like(relPath, "pattern");
- assertTrue(c instanceof Condition.Property);
- Condition.Property cp = (Condition.Property) c;
+ assertTrue(c instanceof Condition.PropertyLike);
+ Condition.PropertyLike cp = (Condition.PropertyLike) c;
assertSame(RelationOp.LIKE, cp.getOp());
assertEquals(relPath, cp.getRelPath());
- assertNull(cp.getValue());
assertEquals("pattern", cp.getPattern());
}