From 5d793012350e96184b01079eff9ccbd024d7fc8b Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 9 Jan 2024 17:21:02 +1100 Subject: [PATCH] WW-5352 Fix missing curved bracket --- .../DefaultAcceptedPatternsChecker.java | 16 +++++++++---- .../parameter/ParametersInterceptor.java | 7 +++--- .../StrutsParameterAnnotationTest.java | 24 +++++++++++++++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java index 0896fec82..4d2caa594 100644 --- a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java +++ b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java @@ -25,12 +25,13 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; -import java.util.Arrays; -import java.util.Collections; import java.util.HashSet; import java.util.Set; import java.util.regex.Pattern; +import static java.util.Arrays.asList; +import static java.util.Collections.unmodifiableSet; + public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { private static final Logger LOG = LogManager.getLogger(DefaultAcceptedPatternsChecker.class); @@ -39,6 +40,11 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { "\\w+((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w-?|[\\u4e00-\\u9fa5]-?)+'])|(\\('(\\w-?|[\\u4e00-\\u9fa5]-?)+'\\)))*" }; + /** + * Must match {@link #ACCEPTED_PATTERNS} RegEx. Signifies characters which result in a nested lookup via OGNL. + */ + public static final Set NESTING_CHARS = unmodifiableSet(new HashSet<>(asList('.', '[', '('))); + public static final String[] DMI_AWARE_ACCEPTED_PATTERNS = { "\\w+([:]?\\w+)?((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w-?|[\\u4e00-\\u9fa5]-?)+'])|(\\('(\\w-?|[\\u4e00-\\u9fa5]-?)+'\\)))*([!]?\\w+)?" }; @@ -74,7 +80,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { newAcceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - acceptedPatterns = Collections.unmodifiableSet(newAcceptedPatterns); + acceptedPatterns = unmodifiableSet(newAcceptedPatterns); } } @@ -85,7 +91,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { @Override public void setAcceptedPatterns(String[] additionalPatterns) { - setAcceptedPatterns(new HashSet<>(Arrays.asList(additionalPatterns))); + setAcceptedPatterns(new HashSet<>(asList(additionalPatterns))); } @Override @@ -97,7 +103,7 @@ public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { newAcceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); } } finally { - acceptedPatterns = Collections.unmodifiableSet(newAcceptedPatterns); + acceptedPatterns = unmodifiableSet(newAcceptedPatterns); } } diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java index f1651cd86..6833a796b 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java @@ -62,6 +62,7 @@ import java.util.Set; import java.util.TreeMap; import java.util.regex.Pattern; +import static com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker.NESTING_CHARS; import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.joining; import static org.apache.commons.lang3.StringUtils.indexOfAny; @@ -341,7 +342,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * Checks if the Action class member corresponding to a parameter is appropriately annotated with * {@link StrutsParameter} and OGNL allowlists any necessary classes. *

- * Note that this logic relies on the use of {@link DefaultAcceptedPatternsChecker#ACCEPTED_PATTERNS} and may also + * Note that this logic relies on the use of {@link DefaultAcceptedPatternsChecker#NESTING_CHARS} and may also * be adversely impacted by the use of custom OGNL property accessors. */ protected boolean isParameterAnnotatedAndAllowlist(String name, Object action) { @@ -349,9 +350,9 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return true; } - int nestingIndex = indexOfAny(name, ".["); + int nestingIndex = indexOfAny(name, NESTING_CHARS.stream().map(String::valueOf).collect(joining())); String rootProperty = nestingIndex == -1 ? name : name.substring(0, nestingIndex); - long paramDepth = name.chars().filter(ch -> ch == '.' || ch == '[').count(); + long paramDepth = name.codePoints().mapToObj(c -> (char) c).filter(NESTING_CHARS::contains).count(); return hasValidAnnotatedMember(rootProperty, action, paramDepth); } diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java index 7b6bbd260..839456e43 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java @@ -109,6 +109,18 @@ public class StrutsParameterAnnotationTest { assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); } + @Test + public void publicPojoDepthOne_sqrBracket() { + testParameter(new FieldAction(), "publicPojoDepthOne['key']", true); + assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); + } + + @Test + public void publicPojoDepthOne_bracket() { + testParameter(new FieldAction(), "publicPojoDepthOne('key')", true); + assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); + } + @Test public void publicNestedPojoDepthOne() { testParameter(new FieldAction(), "publicPojoDepthOne.key.key", false); @@ -126,6 +138,18 @@ public class StrutsParameterAnnotationTest { assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); } + @Test + public void publicNestedPojoDepthTwo_sqrBracket() { + testParameter(new FieldAction(), "publicPojoDepthTwo['key']['key']", true); + assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); + } + + @Test + public void publicNestedPojoDepthTwo_bracket() { + testParameter(new FieldAction(), "publicPojoDepthTwo('key')('key')", true); + assertThat(threadAllowlist.getAllowlist()).containsExactly(Pojo.class); + } + @Test public void privateStrAnnotatedMethod() { testParameter(new MethodAction(), "privateStr", false);