This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/fiql-max-length in repository https://gitbox.apache.org/repos/asf/cxf.git
commit a896745e1db3cff4fc8f796715f9094c226b90c0 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Sep 4 14:30:30 2026 +0100 Put a max length on the Fiql search term and fix performance parsing issue --- .../cxf/jaxrs/ext/search/SearchContextImpl.java | 4 +- .../cxf/jaxrs/ext/search/fiql/FiqlParser.java | 77 +++++++++++++++- .../jaxrs/ext/search/SearchContextImplTest.java | 8 ++ .../jaxrs/ext/search/fiql/FiqlParserReDoSTest.java | 102 +++++++++++++++++++++ 4 files changed, 185 insertions(+), 6 deletions(-) diff --git a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/SearchContextImpl.java b/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/SearchContextImpl.java index 00124ff69d1..14a831bfaea 100644 --- a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/SearchContextImpl.java +++ b/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/SearchContextImpl.java @@ -185,7 +185,7 @@ public class SearchContextImpl implements SearchContext { 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 @@ public class SearchContextImpl implements SearchContext { (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; } diff --git a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParser.java b/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParser.java index a71839402d0..0971e48a076 100644 --- a/rt/rs/extensions/search/src/main/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParser.java +++ b/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 class FiqlParser<T> extends AbstractSearchConditionParser<T> { 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 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> { 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 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> { */ @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 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> { } 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 @@ public class FiqlParser<T> extends AbstractSearchConditionParser<T> { 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); diff --git a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java index f30c9ea7b0c..b3c6b1c9e80 100644 --- a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java +++ b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/SearchContextImplTest.java @@ -76,6 +76,14 @@ public class SearchContextImplTest { 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(); diff --git a/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParserReDoSTest.java b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParserReDoSTest.java new file mode 100644 index 00000000000..30b92457fec --- /dev/null +++ b/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; + } + } +} \ No newline at end of file
