This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cxf.git
The following commit(s) were added to refs/heads/main by this push:
new 8c2302b5dbd Put a configurable limit on deeply parenthesized
sub-expressions in the FiqlParser (#3442)
8c2302b5dbd is described below
commit 8c2302b5dbd602be91342109a0ad575b2e278882
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Fri Sep 4 12:55:37 2026 +0100
Put a configurable limit on deeply parenthesized sub-expressions in the
FiqlParser (#3442)
---
.../cxf/jaxrs/ext/search/SearchContextImpl.java | 4 +-
.../cxf/jaxrs/ext/search/fiql/FiqlParser.java | 42 +++++++-
.../jaxrs/ext/search/SearchContextImplTest.java | 16 +++
.../jaxrs/ext/search/fiql/FiqlParserDepthTest.java | 108 +++++++++++++++++++++
4 files changed, 166 insertions(+), 4 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 1e8f98f2047..00124ff69d1 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<>(5);
+ props = new HashMap<>(6);
props.put(SearchUtils.DATE_FORMAT_PROPERTY,
(String)message.getContextualProperty(SearchUtils.DATE_FORMAT_PROPERTY));
props.put(SearchUtils.TIMEZONE_SUPPORT_PROPERTY,
@@ -197,6 +197,8 @@ public class SearchContextImpl implements SearchContext {
// FIQL specific
props.put(FiqlParser.SUPPORT_SINGLE_EQUALS,
(String)message.getContextualProperty(FiqlParser.SUPPORT_SINGLE_EQUALS));
+ props.put(FiqlParser.MAX_PARENTHESIS_DEPTH,
+
(String)message.getContextualProperty(FiqlParser.MAX_PARENTHESIS_DEPTH));
} 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 aa1ac058048..a71839402d0 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
@@ -63,9 +63,17 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
public static final String SUPPORT_SINGLE_EQUALS =
"fiql.support.single.equals.operator";
+ /**
+ * Context property limiting how deeply parenthesized sub-expressions may
nest.
+ * The parser recurses once per nesting level, so without a bound a crafted
+ * expression of thousands of nested brackets triggers a
StackOverflowError on
+ * the request thread. Must be a positive integer; the default is 64.
+ */
+ public static final String MAX_PARENTHESIS_DEPTH =
"fiql.max.parenthesis.depth";
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 Map<String, ConditionType> OPERATORS_MAP;
private static final Pattern COMPARATORS_PATTERN;
private static final Pattern COMPARATORS_PATTERN_SINGLE_EQUALS;
@@ -100,6 +108,9 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
protected Map<String, ConditionType> operatorsMap = OPERATORS_MAP;
protected Pattern comparatorsPattern = COMPARATORS_PATTERN;
+
+ private final int maxParenthesisDepth;
+
/**
* Creates FIQL parser.
*
@@ -134,6 +145,8 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
Map<String, String> beanProperties) {
super(tclass, contextProperties, beanProperties);
+ this.maxParenthesisDepth =
parseMaxParenthesisDepth(this.contextProperties.get(MAX_PARENTHESIS_DEPTH));
+
if
(PropertyUtils.isTrue(this.contextProperties.get(SUPPORT_SINGLE_EQUALS))) {
operatorsMap = new HashMap<>(operatorsMap);
operatorsMap.put("=", ConditionType.EQUALS);
@@ -141,6 +154,24 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
}
}
+ private static int parseMaxParenthesisDepth(String value) {
+ if (value == null || value.trim().isEmpty()) {
+ return DEFAULT_MAX_PARENTHESIS_DEPTH;
+ }
+ final int depth;
+ try {
+ depth = Integer.parseInt(value.trim());
+ } catch (NumberFormatException ex) {
+ throw new IllegalArgumentException(MAX_PARENTHESIS_DEPTH + " must
be a positive integer, got: "
+ + value, ex);
+ }
+ if (depth < 1) {
+ throw new IllegalArgumentException(MAX_PARENTHESIS_DEPTH + " must
be a positive integer, got: "
+ + value);
+ }
+ return depth;
+ }
+
/**
* Parses expression and builds search filter. Names used in FIQL
expression are names of getters/setters
* in type T.
@@ -165,11 +196,16 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
*/
@Override
public SearchCondition<T> parse(String fiqlExpression) throws
SearchParseException {
- ASTNode<T> ast = parseAndsOrsBrackets(fiqlExpression);
+ ASTNode<T> ast = parseAndsOrsBrackets(fiqlExpression, 0);
return ast.build();
}
- private ASTNode<T> parseAndsOrsBrackets(String expr) throws
SearchParseException {
+ private ASTNode<T> parseAndsOrsBrackets(String expr, int depth) throws
SearchParseException {
+ if (depth > maxParenthesisDepth) {
+ throw new SearchParseException("Exceeded the maximum FIQL
expression nesting depth of "
+ + maxParenthesisDepth + "; the limit can be adjusted with the "
+ + MAX_PARENTHESIS_DEPTH + " property");
+ }
List<String> subexpressions = new ArrayList<>();
List<String> operators = new ArrayList<>();
int level = 0;
@@ -226,7 +262,7 @@ public class FiqlParser<T> extends
AbstractSearchConditionParser<T> {
String subex = subexpressions.get(from);
ASTNode<T> node;
if (subex.startsWith("(")) {
- node = parseAndsOrsBrackets(subex.substring(1,
subex.length() - 1));
+ node = parseAndsOrsBrackets(subex.substring(1,
subex.length() - 1), depth + 1);
} else {
node = parseComparison(subex);
}
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 32f73b29ca1..f30c9ea7b0c 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
@@ -60,6 +60,22 @@ public class SearchContextImplTest {
new SearchContextImpl(m).getCondition(Book.class);
}
+ @Test(expected = SearchParseException.class)
+ public void testMaxParenthesisDepthPropertyIsHonoured() {
+ Message m = new MessageImpl();
+ m.put(FiqlParser.MAX_PARENTHESIS_DEPTH, "1");
+ m.put(Message.QUERY_STRING, "_s=(((name==CXF)))");
+ new SearchContextImpl(m).getCondition(Book.class);
+ }
+
+ @Test
+ public void testMaxParenthesisDepthPropertyAllowsWithinBound() {
+ Message m = new MessageImpl();
+ m.put(FiqlParser.MAX_PARENTHESIS_DEPTH, "3");
+ m.put(Message.QUERY_STRING, "_s=(((name==CXF)))");
+ assertNotNull(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/FiqlParserDepthTest.java
b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParserDepthTest.java
new file mode 100644
index 00000000000..ace3303b025
--- /dev/null
+++
b/rt/rs/extensions/search/src/test/java/org/apache/cxf/jaxrs/ext/search/fiql/FiqlParserDepthTest.java
@@ -0,0 +1,108 @@
+/**
+ * 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.Assert;
+import org.junit.Test;
+
+/**
+ * The FIQL parser recurses once per parenthesis nesting level; nesting depth
must
+ * be bounded so a crafted expression cannot drive it into a
StackOverflowError.
+ */
+public class FiqlParserDepthTest extends Assert {
+
+ public static class Condition {
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+ }
+
+ private static String nested(int depth) {
+ StringBuilder sb = new StringBuilder();
+ for (int i = 0; i < depth; i++) {
+ sb.append('(');
+ }
+ sb.append("name==a");
+ for (int i = 0; i < depth; i++) {
+ sb.append(')');
+ }
+ return sb.toString();
+ }
+
+ @Test
+ public void testModerateNestingParses() throws Exception {
+ FiqlParser<Condition> parser = new FiqlParser<>(Condition.class);
+ assertNotNull(parser.parse(nested(10)));
+ }
+
+ @Test
+ public void testDefaultDepthBoundaryParses() throws Exception {
+ FiqlParser<Condition> parser = new FiqlParser<>(Condition.class);
+ assertNotNull(parser.parse(nested(64)));
+ }
+
+ @Test(expected = SearchParseException.class)
+ public void testDeepNestingRejectedNotStackOverflow() throws Exception {
+ // deep enough to overflow a default thread stack if recursion were
unbounded
+ new FiqlParser<>(Condition.class).parse(nested(10000));
+ }
+
+ @Test(expected = SearchParseException.class)
+ public void testConfiguredDepthLimitEnforced() throws Exception {
+ FiqlParser<Condition> parser = new FiqlParser<>(Condition.class,
+ Collections.singletonMap(FiqlParser.MAX_PARENTHESIS_DEPTH, "2"));
+ parser.parse(nested(3));
+ }
+
+ @Test
+ public void testConfiguredDepthLimitAllowsWithinBound() throws Exception {
+ FiqlParser<Condition> parser = new FiqlParser<>(Condition.class,
+ Collections.singletonMap(FiqlParser.MAX_PARENTHESIS_DEPTH, "8"));
+ assertNotNull(parser.parse(nested(8)));
+ }
+
+ @Test
+ public void testBlankDepthPropertyFallsBackToDefault() throws Exception {
+ FiqlParser<Condition> parser = new FiqlParser<>(Condition.class,
+ Collections.singletonMap(FiqlParser.MAX_PARENTHESIS_DEPTH, " "));
+ assertNotNull(parser.parse(nested(64)));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testNonNumericDepthPropertyRejected() throws Exception {
+ new FiqlParser<>(Condition.class,
+ Collections.singletonMap(FiqlParser.MAX_PARENTHESIS_DEPTH,
"lots"));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testNonPositiveDepthPropertyRejected() throws Exception {
+ new FiqlParser<>(Condition.class,
+ Collections.singletonMap(FiqlParser.MAX_PARENTHESIS_DEPTH, "0"));
+ }
+}
