Skip to content

Commit

Permalink
Better recursion detection.
Browse files Browse the repository at this point in the history
  • Loading branch information
CoreyD97 committed Feb 24, 2023
1 parent 756f4a2 commit d46c85c
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ public class FilterExpression {
protected HashSet<FieldGroup> requiredContexts;

public FilterExpression(String filterString) throws ParseException {
this(null, filterString);
}

public FilterExpression(String alias, String filterString) throws ParseException {
this.ast = FilterParser.parseFilter(filterString);
HashMap<String, Object> filterInfo = FilterParser.validateFilterDependencies(LoggerPlusPlus.instance.getLibraryController(), this.ast);
HashMap<String, Object> filterInfo = FilterParser.validateFilterDependencies(LoggerPlusPlus.instance.getLibraryController(), alias, this.ast);
snippetDependencies = (HashSet<String>) filterInfo.get("dependencies");
requiredContexts = (HashSet<FieldGroup>) filterInfo.get("contexts");
}
Expand All @@ -43,7 +47,7 @@ public void addConditionToFilter(LogicalOperator logicalOperator, LogEntryField
}

this.ast = FilterParser.parseFilter(String.format("%s %s %s %s %s", existing, logicalOperator.toString(), field.toString(), booleanOperator, value));
HashMap<String, Object> filterInfo = FilterParser.validateFilterDependencies(LoggerPlusPlus.instance.getLibraryController(), this.ast);
HashMap<String, Object> filterInfo = FilterParser.validateFilterDependencies(LoggerPlusPlus.instance.getLibraryController(), null, this.ast);
snippetDependencies = (HashSet<String>) filterInfo.get("dependencies");
requiredContexts = (HashSet<FieldGroup>) filterInfo.get("contexts");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public boolean trySetFilter(String filterString){

public void parseAndSetFilter(String filterString) throws ParseException {
this.filterString = filterString;
this.filterExpression = new FilterExpression(filterString);
try {
this.filterExpression = new FilterExpression(name, filterString);
}catch (ParseException e){
this.filterExpression = null;
throw e;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
import com.nccgroup.loggerplusplus.filterlibrary.FilterLibraryController;
import com.nccgroup.loggerplusplus.logentry.FieldGroup;
import com.nccgroup.loggerplusplus.logentry.LogEntryField;
import lombok.extern.log4j.Log4j2;

import java.util.HashSet;
import java.util.LinkedList;
import java.util.*;
import java.util.stream.Collectors;

@Log4j2
public class AliasCheckVisitor implements FilterParserVisitor{

private FilterLibraryController filterLibraryController;
Expand All @@ -21,20 +23,31 @@ public VisitorData defaultVisit(SimpleNode node, VisitorData data){
node.childrenAccept(this, data);
return data;
}

@Override
public VisitorData visit(SimpleNode node, VisitorData data){
return defaultVisit(node, data);
}

public VisitorData visit(SimpleNode node){
public VisitorData visit(String alias, SimpleNode node){
log.debug("Starting sanity check on expression (%s) alias (%s) ".formatted(node.toString(), alias));
VisitorData visitorData = new VisitorData();
visitorData.setData("dependencies", new HashSet<String>());
visitorData.setData("contexts", new HashSet<FieldGroup>());
visitorData.setData("aliasVisitList", new LinkedList<String>());
Stack<String> visitStack = new Stack<String>();
visitorData.setData("aliasVisitList", visitStack);
if (alias != null) {
visitStack.push(alias.toUpperCase());
}
return visit(node, visitorData);
}

@Override
public VisitorData visit(ASTExpression node, VisitorData data){
return defaultVisit(node, data);
}

@Override
public VisitorData visit(ASTComparison node, VisitorData visitorData){
HashSet<FieldGroup> contexts = (HashSet<FieldGroup>) visitorData.getData().get("contexts");
if(node.left instanceof LogEntryField) contexts.add(((LogEntryField) node.left).getFieldGroup());
Expand All @@ -43,41 +56,41 @@ public VisitorData visit(ASTComparison node, VisitorData visitorData){
return visitorData;
}

private static String RECURSION_CHECK = "RECURSION_CHECK";
@Override
public VisitorData visit(ASTAlias node, VisitorData data) {
//Add this alias to our dependencies
((HashSet<String>) data.getData().get("dependencies")).add(node.identifier);
if(filterLibraryController == null){
data.addError("Cannot use aliases in this context. Filter library controller is not set.");
return data;
}
Stack<String> aliasVisitList = (Stack<String>) data.getData().get("aliasVisitList");
try {
log.debug("Visiting " + node.identifier);
if (aliasVisitList.contains(node.identifier.toUpperCase())) {
//We're recursing, don't continue!
aliasVisitList.push(node.identifier.toUpperCase());
String visitOrder = aliasVisitList.stream().collect(Collectors.joining("->"));
data.addError("Recursion detected in filter. Alias trace: " + visitOrder);
return data;
}

aliasVisitList.push(node.identifier.toUpperCase());
log.debug("Current Visit Queue " + aliasVisitList.toString());

LinkedList<String> aliasVisitList = (LinkedList<String>) data.getData().get("aliasVisitList");
if(aliasVisitList.contains(node.identifier)){
//We're recursing, don't continue!
data.addError("Recursion detected in filter. Alias identifier: " + node.identifier);
return data;
}else{
aliasVisitList.push(node.identifier);
}

//Now sanity check on the aliased filter with our existing data
boolean foundAliasedFilter = false;
for (SavedFilter savedFilter : filterLibraryController.getFilterSnippets()) {
if(savedFilter.getName().equalsIgnoreCase(node.identifier) && savedFilter.getFilterExpression() != null){
visit(savedFilter.getFilterExpression().getAst(), data);
foundAliasedFilter = true;
aliasVisitList.pop();
break;
((HashSet<String>) data.getData().get("dependencies")).add(node.identifier.toUpperCase());

//Now sanity check on the aliased filter with our existing data
Optional<SavedFilter> aliasedFilter = filterLibraryController.getFilterSnippets().stream().filter(savedFilter -> savedFilter.getName().equalsIgnoreCase(node.identifier)).findFirst();
if (aliasedFilter.isPresent()) {
visit(aliasedFilter.get().getFilterExpression().getAst(), data);
} else {
data.addError("Could not find a filter in the library for alias: " + node.identifier);
}
}

if(!foundAliasedFilter){
data.addError("Could not find a filter in the library for alias: " + node.identifier);
}

return data;
return data;
}finally {
log.debug("Leaving " + node.identifier);
aliasVisitList.pop();
log.debug("Current Visit Queue " + aliasVisitList.toString());
}
}
}
/* JavaCC - OriginalChecksum=b30458d637879c9662107beea18204f0 (do not edit this line) */
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ public class FilterParser/*@bgen(jjtree)*/implements FilterParserTreeConstants/*
return node;
}

public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(filter);
public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, String alias, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(alias, filter);
if(!result.isSuccess()) throw new ParseException(result.getErrorString());
return result.getData();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public class FilterParser {
return node;
}

public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(filter);
public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, String alias, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(alias, filter);
if(!result.isSuccess()) throw new ParseException(result.getErrorString());
return result.getData();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ public static ASTExpression parseFilter(String string) throws ParseException {
return node;
}

public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(filter);
public static HashMap<String, Object> validateFilterDependencies(FilterLibraryController libraryController, String alias, ASTExpression filter) throws ParseException {
VisitorData result = new AliasCheckVisitor(libraryController).visit(alias, filter);
if(!result.isSuccess()) throw new ParseException(result.getErrorString());
return result.getData();
}
Expand Down

0 comments on commit d46c85c

Please sign in to comment.