You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by co...@apache.org on 2026/09/04 13:31:21 UTC
(cxf) branch coheigea/fiql-max-length created (now a896745e1db)
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a change to branch coheigea/fiql-max-length
in repository https://gitbox.apache.org/repos/asf/cxf.git
at a896745e1db Put a max length on the Fiql search term and fix performance parsing issue
This branch includes the following new commits:
new a896745e1db Put a max length on the Fiql search term and fix performance parsing issue
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(cxf) 01/01: Put a max length on the Fiql search term and fix performance parsing issue
Posted by co...@apache.org.
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 <co...@apache.org>
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