This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/fiql in repository https://gitbox.apache.org/repos/asf/cxf.git
commit e7f3d1072678a999974b09445301e9b3af05af16 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Sep 4 07:54:50 2026 +0100 Put a configurable limit on deeply parenthesized sub-expressions in the FiqlParser --- .../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")); + } +}
