Put a max length on the Fiql search term and fix performance parsing …

rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/SearchContextImpl.java

@@ -185,7 +185,7 @@ private <T> SearchConditionParser<T> getParser(Class<T> cls,
 
         final Map<String, String> props;
         if (parserProperties == null) {
-            props = new HashMap<>(6);
+            props = new HashMap<>(7);
             props.put(SearchUtils.DATE_FORMAT_PROPERTY,
                       (String)message.getContextualProperty(SearchUtils.DATE_FORMAT_PROPERTY));
             props.put(SearchUtils.TIMEZONE_SUPPORT_PROPERTY,
@@ -199,6 +199,8 @@ private <T> SearchConditionParser<T> getParser(Class<T> cls,
                       (String)message.getContextualProperty(FiqlParser.SUPPORT_SINGLE_EQUALS));
             props.put(FiqlParser.MAX_PARENTHESIS_DEPTH,
                       (String)message.getContextualProperty(FiqlParser.MAX_PARENTHESIS_DEPTH));
+            props.put(FiqlParser.MAX_EXPRESSION_LENGTH,
+                      (String)message.getContextualProperty(FiqlParser.MAX_EXPRESSION_LENGTH));
         } else {
             props = parserProperties;
         }

rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParser.java

@@ -70,13 +70,20 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> {
      * the request thread. Must be a positive integer; the default is 64.
      */
     public static final String MAX_PARENTHESIS_DEPTH = "fiql.max.parenthesis.depth";
+    /**
+    * Context property limiting the length of a FIQL expression. The default is 8 KiB.
+     */
+    public static final String MAX_EXPRESSION_LENGTH = "fiql.max.expression.length";
     public static final String EXTENSION_COUNT = "count";
     protected static final String EXTENSION_COUNT_OPEN = EXTENSION_COUNT + "(";
 
     private static final int DEFAULT_MAX_PARENTHESIS_DEPTH = 64;
+    private static final int DEFAULT_MAX_EXPRESSION_LENGTH = 8 * 1024;
     private static final Map<String, ConditionType> OPERATORS_MAP;
     private static final Pattern COMPARATORS_PATTERN;
     private static final Pattern COMPARATORS_PATTERN_SINGLE_EQUALS;
+    private static final String[] COMPARATORS = {GT, GE, LT, LE, EQ, NEQ};
+    private static final String[] COMPARATORS_SINGLE_EQUALS = {GT, GE, LT, LE, EQ, NEQ, "="};
 
     static {
         // operatorsMap
@@ -110,6 +117,7 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> {
     protected Pattern comparatorsPattern = COMPARATORS_PATTERN;
 
     private final int maxParenthesisDepth;
+    private final int maxExpressionLength;
 
     /**
      * Creates FIQL parser.
@@ -146,6 +154,8 @@ public FiqlParser(Class<T> tclass,
         super(tclass, contextProperties, beanProperties);
 
         this.maxParenthesisDepth = parseMaxParenthesisDepth(this.contextProperties.get(MAX_PARENTHESIS_DEPTH));
+        this.maxExpressionLength = parseMaxExpressionLength(
+            this.contextProperties.get(MAX_EXPRESSION_LENGTH));
 
         if (PropertyUtils.isTrue(this.contextProperties.get(SUPPORT_SINGLE_EQUALS))) {
             operatorsMap = new HashMap<>(operatorsMap);
@@ -172,6 +182,24 @@ private static int parseMaxParenthesisDepth(String value) {
         return depth;
     }
 
+    private static int parseMaxExpressionLength(String value) {
+        if (value == null || value.trim().isEmpty()) {
+            return DEFAULT_MAX_EXPRESSION_LENGTH;
+        }
+        final int length;
+        try {
+            length = Integer.parseInt(value.trim());
+        } catch (NumberFormatException ex) {
+            throw new IllegalArgumentException(MAX_EXPRESSION_LENGTH
+                + " must be a positive integer, got: " + value, ex);
+        }
+        if (length < 1) {
+            throw new IllegalArgumentException(MAX_EXPRESSION_LENGTH
+                + " must be a positive integer, got: " + value);
+        }
+        return length;
+    }
+
     /**
      * Parses expression and builds search filter. Names used in FIQL expression are names of getters/setters
      * in type T.
@@ -196,6 +224,11 @@ private static int parseMaxParenthesisDepth(String value) {
      */
     @Override
     public SearchCondition<T> parse(String fiqlExpression) throws SearchParseException {
+        if (fiqlExpression.length() > maxExpressionLength) {
+            throw new SearchParseException("Exceeded the maximum FIQL expression length of "
+                + maxExpressionLength + "; the limit can be adjusted with the "
+                + MAX_EXPRESSION_LENGTH + " property");
+        }
         ASTNode<T> ast = parseAndsOrsBrackets(fiqlExpression, 0);
         return ast.build();
     }
@@ -284,11 +317,11 @@ private ASTNode<T> parseAndsOrsBrackets(String expr, int depth) throws SearchPar
     }
 
     protected ASTNode<T> parseComparison(String expr) throws SearchParseException {
-        Matcher m = comparatorsPattern.matcher(expr);
-        if (m.find()) {
-            String propertyName = expr.substring(0, m.start(1));
-            String operator = m.group(1);
-            String value = expr.substring(m.end(1));
+        int[] comparator = findComparator(expr);
+        if (comparator != null) {
+            String propertyName = expr.substring(0, comparator[0]);
+            String operator = expr.substring(comparator[0], comparator[1]);
+            String value = expr.substring(comparator[1]);
             if ("".equals(value)) {
                 throw new SearchParseException("Not a comparison expression: " + expr);
             }
@@ -305,6 +338,40 @@ protected ASTNode<T> parseComparison(String expr) throws SearchParseException {
         throw new SearchParseException("Not a comparison expression: " + expr);
     }
 
+    private int[] findComparator(String expr) {
+        // Preserve custom comparator patterns for subclasses.
+        if (comparatorsPattern != COMPARATORS_PATTERN
+            && comparatorsPattern != COMPARATORS_PATTERN_SINGLE_EQUALS) {
+            Matcher m = comparatorsPattern.matcher(expr);
+            return m.find() ? new int[] {m.start(1), m.end(1)} : null;
+        }
+
+        String[] comparators = comparatorsPattern == COMPARATORS_PATTERN
+            ? COMPARATORS : COMPARATORS_SINGLE_EQUALS;
+        int runStart = 0;
+        while (runStart < expr.length()) {
+            // The original pattern only allows the prefix and comparator to be ASCII.
+            while (runStart < expr.length() && expr.charAt(runStart) > 0x7F) {
+                runStart++;
+            }
+            int runEnd = runStart;
+            while (runEnd < expr.length() && expr.charAt(runEnd) <= 0x7F) {
+                runEnd++;
+            }
+            // Match the rightmost comparator, as the original greedy prefix did.
+            for (int index = runEnd - 1; index > runStart; index--) {
+                for (String comparator : comparators) {
+                    int comparatorEnd = index + comparator.length();
+                    if (comparatorEnd <= runEnd && expr.startsWith(comparator, index)) {
+                        return new int[] {index, comparatorEnd};
+                    }
+                }
+            }
+            runStart = runEnd;
+        }
+        return null;
+    }
+
 
     protected TypeInfoObject parseType(String originalName, String setter, String value) throws SearchParseException {
         TypeInfo typeInfo = getTypeInfo(setter, value);

rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java

@@ -76,6 +76,14 @@ public void testMaxParenthesisDepthPropertyAllowsWithinBound() {
         assertNotNull(new SearchContextImpl(m).getCondition(Book.class));
     }
 
+    @Test(expected = SearchParseException.class)
+    public void testMaxExpressionLengthPropertyIsHonoured() {
+        Message m = new MessageImpl();
+        m.put(FiqlParser.MAX_EXPRESSION_LENGTH, "10");
+        m.put(Message.QUERY_STRING, "_s=name==12345");
+        new SearchContextImpl(m).getCondition(Book.class);
+    }
+
     @Test
     public void testPlainQuery2() {
         Message m = new MessageImpl();

rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParserReDoSTest.java

@@ -0,0 +1,102 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cxf.jaxrs.ext.search.fiql;
+
+import java.util.Collections;
+
+import org.apache.cxf.jaxrs.ext.search.SearchParseException;
+
+import org.junit.Test;
+
+public class FiqlParserReDoSTest {
+    @Test(timeout = 2000)
+    public void testLongExpressionWithoutComparatorIsRejectedQuickly() {
+        StringBuilder expression = new StringBuilder(32768);
+        for (int i = 0; i < 32768; i++) {
+            expression.append('a');
+        }
+        try {
+            new FiqlParser<>(Bean.class).parse(expression.toString());
+        } catch (SearchParseException ex) {
+            return;
+        }
+        throw new AssertionError("An expression without a comparator must be rejected");
+    }
+
+    @Test
+    public void testComparatorsRemainSupported() throws SearchParseException {
+        FiqlParser<Bean> parser = new FiqlParser<>(Bean.class);
+        parser.parse("name==value");
+        parser = new FiqlParser<>(Bean.class,
+            Collections.singletonMap(FiqlParser.SUPPORT_SINGLE_EQUALS, "true"));
+        parser.parse("name=first");
+    }
+
+    @Test
+    public void testConfiguredExpressionLengthLimit() throws SearchParseException {
+        FiqlParser<Bean> parser = new FiqlParser<>(Bean.class,
+            Collections.singletonMap(FiqlParser.MAX_EXPRESSION_LENGTH, "10"));
+        parser.parse("name==1234");
+        try {
+            parser.parse("name==12345");
+        } catch (SearchParseException ex) {
+            return;
+        }
+        throw new AssertionError("An expression over the configured length must be rejected");
+    }
+
+    @Test
+    public void testDefaultExpressionLengthLimitIsEightKiB() throws SearchParseException {
+        FiqlParser<Bean> parser = new FiqlParser<>(Bean.class);
+        parser.parse(expressionOfLength(8192));
+        try {
+            parser.parse(expressionOfLength(8193));
+        } catch (SearchParseException ex) {
+            return;
+        }
+        throw new AssertionError("The default expression length limit must be 8 KiB");
+    }
+
+    private static String expressionOfLength(int length) {
+        StringBuilder expression = new StringBuilder(length);
+        expression.append("name==");
+        while (expression.length() < length) {
+            expression.append('a');
+        }
+        return expression.substring(0, length);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidExpressionLengthLimitRejected() {
+        new FiqlParser<>(Bean.class,
+            Collections.singletonMap(FiqlParser.MAX_EXPRESSION_LENGTH, "0"));
+    }
+
+    public static class Bean {
+        private String name;
+
+        public String getName() {
+            return name;
+        }
+
+        public void setName(String name) {
+            this.name = name;
+        }
+    }
+}