Skip to content

Commit

Permalink
CSW fix (#274)
Browse files Browse the repository at this point in the history 
* Revert #253

* CSW GetRecords doesn't escape query values when creating the Elasticsearch query. Fixes #7527

* CSW GetRecords doesn't escape query values when creating the Elasticsearch query / Escape Elasticsearch special chars in EQUAL / NOT EQUAL literal queries

* CSW GetRecords doesn't escape query values when creating the Elasticsearch query / Escape Elasticsearch special chars in IS LIKE literal queries

* Reset geotools usage and comments tests

---------

Co-authored-by: Jose García <[email protected]>
  • Loading branch information
@f-necas @josegar74
f-necas and josegar74 committed Jan 25, 2024
1 parent a15df19 commit c646022
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 67 deletions.
135 changes: 68 additions & 67 deletions csw-server/src/main/java/org/fao/geonet/kernel/csw/services/getrecords/es/CswFilter2Es.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2001-2016 Food and Agriculture Organization of the
* Copyright (C) 2001-2023 Food and Agriculture Organization of the
* United Nations (FAO-UN), United Nations World Food Programme (WFP)
* and United Nations Environment Programme (UNEP)
*
Expand All @@ -26,6 +26,7 @@
import org.apache.commons.lang.NotImplementedException;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.fao.geonet.constants.Geonet;
import org.fao.geonet.kernel.csw.services.getrecords.IFieldMapper;
import org.fao.geonet.utils.Log;
Expand Down Expand Up @@ -82,75 +83,77 @@
import org.opengis.filter.temporal.TOverlaps;
import org.opengis.geometry.BoundingBox;

import java.util.*;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
import java.util.List;
import java.util.regex.Pattern;

/**
* Manages the translation from CSW &lt;Filter&gt; into a ES query.
*/
public class CswFilter2Es extends AbstractFilterVisitor {
private final String BINARY_OPERATOR_AND = "AND";
private final String BINARY_OPERATOR_OR = "OR";
private static final String BINARY_OPERATOR_AND = "AND";
private static final String BINARY_OPERATOR_OR = "OR";

static final String SPECIAL_RE = "([" + Pattern.quote("+-&|!(){}[]^\\\"~*?:/") + "])";
static final String SPECIAL_LIKE_RE = "(?<!\\\\)([" + Pattern.quote("+-&|!(){}[]^\"~:/") + "])";
private static final String SPECIAL_RE = "([" + Pattern.quote("+-&|!(){}[]^\\\"~*?:/") + "])";
private static final String SPECIAL_LIKE_RE = "(?<!\\\\)([" + Pattern.quote("+-&|!(){}[]^\"~:/") + "])";
private final StringBuilder outQueryString = new StringBuilder();
private final Expression2CswVisitor expressionVisitor;

private boolean useFilter = true;

// Stack to build the Elasticsearch Query
Deque<String> stack = new ArrayDeque<String>();
Deque<String> stack = new ArrayDeque<>();

private final String templateNot = " {\"bool\": {\n" +
private static final String TEMPLATE_NOT = " {\"bool\": {\n" +
" \"must_not\": [\n" +
" %s\n" +
" ]\n" +
" }}";


private final String templateAnd = " {\"bool\": {\n" +
private static final String TEMPLATE_AND = " {\"bool\": {\n" +
" \"must\": [\n" +
" %s\n" +
" ]\n" +
" }}";

private final String templateAndWithFilter = " \"bool\": {\n" +
private static final String TEMPLATE_AND_WITH_FILTER = " \"bool\": {\n" +
" \"must\": [\n" +
" %s\n" +
" ]\n" +
" ,\"filter\":{\"query_string\":{\"query\":\"%s\"}}}"; //, "minimum_should_match" : 1

private final String templateOr = " {\"bool\": {\n" +
private static final String TEMPLATE_OR = " {\"bool\": {\n" +
" \"should\": [\n" +
" %s\n" +
" ]\n" +
" }}";

private final String templateOrWithFilter = " \"bool\": {\n" +
private static final String TEMPLATE_OR_WITH_FILTER = " \"bool\": {\n" +
" \"should\": [\n" +
" %s\n" +
" ]\n" +
" ,\"filter\":{\"query_string\":{\"query\":\"%s\"}}, \"minimum_should_match\" : 1}";

private final String templateMatch = "{\"query_string\": {\n" +
private static final String TEMPLATE_MATCH = "{\"query_string\": {\n" +
" \"fields\": [\"%s\"],\n" +
" \"query\": \"%s\"\n" +
" }}";

private final String templatePropertyIsNot = " {\"bool\": {\n" +
" \"must_not\": " + templateMatch +
private static final String TEMPLATE_PROPERTY_IS_NOT = " {\"bool\": {\n" +
" \"must_not\": " + TEMPLATE_MATCH +
" }}";

private final String templateRange = " {\n" +
private static final String TEMPLATE_RANGE = " {\n" +
" \"range\" : {\n" +
" \"%s\" : {\n" +
" \"%s\" : %s\n" +
" }\n" +
" }\n" +
" }";

private final String templateBetween = " {\n" +
private static final String TEMPLATE_BETWEEN = " {\n" +
" \"range\" : {\n" +
" \"%s\" : {\n" +
" \"gte\" : %s,\n" +
Expand All @@ -159,12 +162,12 @@ public class CswFilter2Es extends AbstractFilterVisitor {
" }\n" +
" }";

private final String templateIsLike = "{\"query_string\": {\n" +
private static final String TEMPLATE_IS_LIKE = "{\"query_string\": {\n" +
" \"fields\": [\"%s\"],\n" +
" \"query\": \"%s\"\n" +
" }}";

private final String templateSpatial = "{ \"geo_shape\": {\"geom\": {\n" +
private static final String TEMPLATE_SPATIAL = "{ \"geo_shape\": {\"geom\": {\n" +
" \t\"shape\": {\n" +
" \t\"type\": \"%s\",\n" +
" \t\"coordinates\" : %s\n" +
Expand Down Expand Up @@ -214,14 +217,16 @@ protected static String convertLikePattern(PropertyIsLike filter) {
: filter.getSingleChar();
result = result.replaceAll(singleCharRe, "?");
}

result = StringEscapeUtils.escapeJson(escapeLikeLiteral(result));
return result;
}

public String getFilter() {
String condition = stack.isEmpty()?"":stack.pop();
// Check for single condition (no binary operators to wrap the query
if (!condition.startsWith(" \"bool\":")) {
condition = String.format(templateAndWithFilter, condition, "%s");
condition = String.format(TEMPLATE_AND_WITH_FILTER, condition, "%s");
}

if (StringUtils.isEmpty(condition)) {
Expand All @@ -237,21 +242,6 @@ public String getFilter() {
return outQueryString.toString();
}

@Override
public Object visitNullFilter(Object extraData) {
return super.visitNullFilter(extraData);
}

@Override
public Object visit(ExcludeFilter filter, Object extraData) {
return super.visit(filter, extraData);
}

@Override
public Object visit(IncludeFilter filter, Object extraData) {
return super.visit(filter, extraData);
}

@Override
public Object visit(And filter, Object extraData) {
return visitBinaryLogic(filter, BINARY_OPERATOR_AND, extraData);
Expand All @@ -261,9 +251,9 @@ private Object visitBinaryLogic(BinaryLogicOperator filter, String operator, Obj
String filterCondition;

if (operator.equals(BINARY_OPERATOR_AND)) {
filterCondition = (useFilter?templateAndWithFilter:templateAnd);
filterCondition = (useFilter? TEMPLATE_AND_WITH_FILTER : TEMPLATE_AND);
} else if (operator.equals(BINARY_OPERATOR_OR)) {
filterCondition = (useFilter?templateOrWithFilter:templateOr);
filterCondition = (useFilter? TEMPLATE_OR_WITH_FILTER : TEMPLATE_OR);
} else {
throw new NotImplementedException();
}
Expand Down Expand Up @@ -307,7 +297,7 @@ public Object visit(Id filter, Object extraData) {

@Override
public Object visit(Not filter, Object extraData) {
String filterNot = templateNot;
String filterNot = TEMPLATE_NOT;

filter.getFilter().accept(this, extraData);

Expand All @@ -324,25 +314,32 @@ public Object visit(Or filter, Object extraData) {

@Override
public Object visit(PropertyIsBetween filter, Object extraData) {
String filterBetween = templateBetween;
String filterBetween = TEMPLATE_BETWEEN;

assert filter.getExpression() instanceof PropertyName;
filter.getExpression().accept(expressionVisitor, extraData);
if (!(filter.getExpression() instanceof PropertyName)) {
throw new IllegalArgumentException("Invalid expression property provided");
}

assert filter.getLowerBoundary() instanceof Literal;
filter.getLowerBoundary().accept(expressionVisitor, extraData);
if (!(filter.getLowerBoundary() instanceof Literal)) {
throw new IllegalArgumentException("Invalid expression lower boundary literal provided");
}

assert filter.getUpperBoundary() instanceof Literal;
if (!(filter.getUpperBoundary() instanceof Literal)) {
throw new IllegalArgumentException("Invalid expression upper boundary literal provided");
}

filter.getExpression().accept(expressionVisitor, extraData);
filter.getLowerBoundary().accept(expressionVisitor, extraData);
filter.getUpperBoundary().accept(expressionVisitor, extraData);

String dataPropertyUpperValue = stack.pop();
if (!NumberUtils.isNumber(dataPropertyUpperValue)) {
dataPropertyUpperValue = CswFilter2Es.quoteString(dataPropertyUpperValue);
dataPropertyUpperValue = StringEscapeUtils.escapeJson(CswFilter2Es.quoteString(dataPropertyUpperValue));
}

String dataPropertyLowerValue = stack.pop();
if (!NumberUtils.isNumber(dataPropertyLowerValue)) {
dataPropertyLowerValue = CswFilter2Es.quoteString(dataPropertyLowerValue);
dataPropertyLowerValue = StringEscapeUtils.escapeJson(CswFilter2Es.quoteString(dataPropertyLowerValue));
}

String dataPropertyName = stack.pop();
Expand All @@ -355,55 +352,51 @@ public Object visit(PropertyIsBetween filter, Object extraData) {

@Override
public Object visit(PropertyIsEqualTo filter, Object extraData) {
checkFilterExpressionsInBinaryComparisonOperator(filter);

assert filter.getExpression1() instanceof PropertyName;
filter.getExpression1().accept(expressionVisitor, extraData);

assert filter.getExpression2() instanceof Literal;
filter.getExpression2().accept(expressionVisitor, extraData);

String dataPropertyValue = stack.pop();
String dataPropertyName = stack.pop();

final String filterEqualTo = String.format(templateMatch, dataPropertyName, dataPropertyValue);
final String filterEqualTo = String.format(TEMPLATE_MATCH, dataPropertyName, StringEscapeUtils.escapeJson(escapeLiteral(dataPropertyValue)));
stack.push(filterEqualTo);

return this;
}

@Override
public Object visit(PropertyIsNotEqualTo filter, Object extraData) {
String filterPropertyIsNot = templatePropertyIsNot;
String filterPropertyIsNot = TEMPLATE_PROPERTY_IS_NOT;

assert filter.getExpression1() instanceof PropertyName;
filter.getExpression1().accept(expressionVisitor, extraData);
checkFilterExpressionsInBinaryComparisonOperator(filter);

assert filter.getExpression2() instanceof Literal;
filter.getExpression1().accept(expressionVisitor, extraData);
filter.getExpression2().accept(expressionVisitor, extraData);

String dataPropertyValue = stack.pop();
String dataPropertyName = stack.pop();

filterPropertyIsNot = String.format(filterPropertyIsNot, dataPropertyName, dataPropertyValue);
filterPropertyIsNot = String.format(filterPropertyIsNot, dataPropertyName, StringEscapeUtils.escapeJson(escapeLiteral(dataPropertyValue)));
stack.push(filterPropertyIsNot);

return this;
}

public Object visitRange(BinaryComparisonOperator filter, String operator, Object extraData) {
String filterRange = templateRange;
String filterRange = TEMPLATE_RANGE;

assert filter.getExpression1() instanceof PropertyName;
filter.getExpression1().accept(expressionVisitor, extraData);
checkFilterExpressionsInBinaryComparisonOperator(filter);

assert filter.getExpression2() instanceof Literal;
filter.getExpression1().accept(expressionVisitor, extraData);
filter.getExpression2().accept(expressionVisitor, extraData);

String dataPropertyValue = stack.pop();
String dataPropertyName = stack.pop();

if (!NumberUtils.isNumber(dataPropertyValue)) {
dataPropertyValue = CswFilter2Es.quoteString(dataPropertyValue);
dataPropertyValue = StringEscapeUtils.escapeJson(CswFilter2Es.quoteString(dataPropertyValue));
}

filterRange = String.format(filterRange, dataPropertyName, operator, dataPropertyValue);
Expand All @@ -423,7 +416,7 @@ public Object visit(PropertyIsGreaterThanOrEqualTo filter, Object extraData) {
}

@Override
public Object visit(PropertyIsLessThan filter, Object extraData) {
public Object visit(PropertyIsLessThan filter, Object extraData) {
return visitRange(filter, "lt", extraData);
}

Expand All @@ -434,7 +427,7 @@ public Object visit(PropertyIsLessThanOrEqualTo filter, Object extraData) {

@Override
public Object visit(PropertyIsLike filter, Object extraData) {
String filterIsLike = templateIsLike;
String filterIsLike = TEMPLATE_IS_LIKE;

String expression = convertLikePattern(filter);

Expand Down Expand Up @@ -472,7 +465,7 @@ public Object visit(PropertyIsNil filter, Object extraData) {
* @return
*/
private String fillTemplateSpatial(String shapeType, String coords, String relation) {
return String.format(templateSpatial, shapeType, coords, relation);
return String.format(TEMPLATE_SPATIAL, shapeType, coords, relation);
}

@Override
Expand All @@ -495,14 +488,12 @@ public Object visit(BBOX filter, Object extraData) {
}

private Object addGeomFilter(BinarySpatialOperator filter, String geoOperator, Object extraData) {

if (!(filter.getExpression2() == null || filter.getExpression1() == null)) {
filter.getExpression1().accept(expressionVisitor, extraData);
}

// out.append(":\"").append(geoOperator).append("(");
final Expression geoExpression = filter.getExpression2() == null ? filter.getExpression1()
: filter.getExpression2();
: filter.getExpression2();
geoExpression.accept(expressionVisitor, extraData);

String geom = stack.pop();
Expand Down Expand Up @@ -684,4 +675,14 @@ private String buildCoordinatesString(Coordinate[] coordinates) {

return String.join(" , ", coordinatesList);
}

private void checkFilterExpressionsInBinaryComparisonOperator(BinaryComparisonOperator filter) {
if (!(filter.getExpression1() instanceof PropertyName)) {
throw new IllegalArgumentException("Invalid expression property provided");
}

if (!(filter.getExpression2() instanceof Literal)) {
throw new IllegalArgumentException("Invalid expression literal provided");
}
}
}
Loading

0 comments on commit c646022

Please sign in to comment.