diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java index 54f94f93d3..16fb7955cf 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java @@ -55,8 +55,7 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.properties.PropertyDescriptor; import com.google.common.base.Objects; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ListMultimap; +import com.google.common.collect.HashMultimap; import org.apache.commons.lang3.StringUtils; /** @@ -132,9 +131,9 @@ public class ApexCRUDViolationRule extends AbstractApexRule { }; private Map varToTypeMapping; - private ListMultimap typeToDMLOperationMapping; + private HashMultimap typeToDMLOperationMapping; private Map checkedTypeToDMLOperationViaESAPI; - private ListMultimap checkedTypeToDMLOperationsViaAuthPattern; + private HashMultimap checkedTypeToDMLOperationsViaAuthPattern; private Map classMethods; private String className; @@ -157,9 +156,9 @@ public class ApexCRUDViolationRule extends AbstractApexRule { // At the start of each rule execution, these member variables need to be fresh. So they're initialized in the // .start() method instead of the constructor, since .start() is called before every execution. varToTypeMapping = new HashMap<>(); - typeToDMLOperationMapping = ArrayListMultimap.create(); + typeToDMLOperationMapping = HashMultimap.create(); checkedTypeToDMLOperationViaESAPI = new HashMap<>(); - checkedTypeToDMLOperationsViaAuthPattern = ArrayListMultimap.create(); + checkedTypeToDMLOperationsViaAuthPattern = HashMultimap.create(); classMethods = new WeakHashMap<>(); className = null; super.start(ctx); @@ -637,7 +636,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } else { boolean properChecksHappened = false; - List dmlOperationsChecked = typeToDMLOperationMapping.get(typeCheck); + Set dmlOperationsChecked = typeToDMLOperationMapping.get(typeCheck); for (String dmlOp : dmlOperationsChecked) { if (dmlOp.equalsIgnoreCase(crudMethod)) { properChecksHappened = true; @@ -866,7 +865,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return true; } - final List dmlOperationsChecked = checkedTypeToDMLOperationsViaAuthPattern.get(typeToCheck); + final Set dmlOperationsChecked = checkedTypeToDMLOperationsViaAuthPattern.get(typeToCheck); return dmlOperationsChecked.contains(dmlOperation); } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml index b5e8066bbd..a36ef8b676 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml @@ -1226,6 +1226,43 @@ public class Foo { ]]> + + >#3576 - Verify use of readAuthMethodPattern, negative test + 2 + accounts = [SELECT Id FROM Account]; + } + + AuthorizationUtil.assertAccessible(Account.SObjectType); + // TODO: Evidently this rule doesn't check Database.query() yet + List accounts = [SELECT Id FROM Account]; + } +} + ]]> + + + + >#3576 - Verify use of readAuthMethodPattern, positive test + AuthorizationUtil\.(is|assert)Accessible + 0 + accounts = [SELECT Id FROM Account]; + } + + AuthorizationUtil.assertAccessible(Account.SObjectType); + // TODO: Evidently this rule doesn't check Database.query() yet + List accounts = [SELECT Id FROM Account]; + } +} + ]]> + + >#3576 - Verify use of updateAuthMethodPattern, negative test 2