diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/AbstractMatcher.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/AbstractMatcher.java index 3b3b74b90..7fe4254c3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/AbstractMatcher.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/AbstractMatcher.java @@ -36,10 +36,8 @@ import java.util.*; * @since 2.1 */ public abstract class AbstractMatcher implements Serializable { - /** - *

The logging instance

- */ - private static final Logger log = LogManager.getLogger(AbstractMatcher.class); + + private static final Logger LOG = LogManager.getLogger(AbstractMatcher.class); /** *

Handles all wildcard pattern matching.

@@ -50,10 +48,34 @@ public abstract class AbstractMatcher implements Serializable { *

The compiled patterns and their associated target objects

*/ List> compiledPatterns = new ArrayList<>(); - ; + + /** + * This flag controls if passed named params should be appended + * to the map in {@link #replaceParameters(Map, Map)} + * and will be accessible in {@link com.opensymphony.xwork2.config.entities.ResultConfig}. + * If set to false, the named parameters won't be appended. + * + * This behaviour is controlled by {@link org.apache.struts2.StrutsConstants#STRUTS_MATCHER_APPEND_NAMED_PARAMETERS} + * + * @since 2.5.23 + * See WW-5065 + */ + private final boolean appendNamedParameters; - public AbstractMatcher(PatternMatcher helper) { + public AbstractMatcher(PatternMatcher helper, boolean appendNamedParameters) { this.wildcard = (PatternMatcher) helper; + this.appendNamedParameters = appendNamedParameters; + } + + /** + * Creates a matcher with {@link #appendNamedParameters} set to true to keep backward compatibility + * + * @param helper an instance of {@link PatternMatcher} + * @deprecated use @{link {@link AbstractMatcher(PatternMatcher, boolean)} instead + */ + @Deprecated + public AbstractMatcher(PatternMatcher helper) { + this(helper, true); } /** @@ -84,17 +106,17 @@ public abstract class AbstractMatcher implements Serializable { name = name.substring(1); } - log.debug("Compiling pattern '{}'", name); + LOG.debug("Compiling pattern '{}'", name); pattern = wildcard.compilePattern(name); - compiledPatterns.add(new Mapping(name, pattern, target)); + compiledPatterns.add(new Mapping<>(name, pattern, target)); if (looseMatch) { int lastStar = name.lastIndexOf('*'); if (lastStar > 1 && lastStar == name.length() - 1) { if (name.charAt(lastStar - 1) != '*') { pattern = wildcard.compilePattern(name.substring(0, lastStar - 1)); - compiledPatterns.add(new Mapping(name, pattern, target)); + compiledPatterns.add(new Mapping<>(name, pattern, target)); } } } @@ -115,12 +137,12 @@ public abstract class AbstractMatcher implements Serializable { E config = null; if (compiledPatterns.size() > 0) { - log.debug("Attempting to match '{}' to a wildcard pattern, {} available", potentialMatch, compiledPatterns.size()); + LOG.debug("Attempting to match '{}' to a wildcard pattern, {} available", potentialMatch, compiledPatterns.size()); - Map vars = new LinkedHashMap(); + Map vars = new LinkedHashMap<>(); for (Mapping m : compiledPatterns) { if (wildcard.match(vars, potentialMatch, m.getPattern())) { - log.debug("Value matches pattern '{}'", m.getOriginalPattern()); + LOG.debug("Value matches pattern '{}'", m.getOriginalPattern()); config = convert(potentialMatch, m.getTarget(), vars); break; } @@ -152,20 +174,23 @@ public abstract class AbstractMatcher implements Serializable { */ protected Map replaceParameters(Map orig, Map vars) { Map map = new LinkedHashMap<>(); - + //this will set the group index references, like {1} for (Map.Entry entry : orig.entrySet()) { map.put(entry.getKey(), convertParam(entry.getValue(), vars)); } - - //the values map will contain entries like name->"Lex Luthor" and 1->"Lex Luthor" - //now add the non-numeric values - for (Map.Entry entry: vars.entrySet()) { - if (!NumberUtils.isCreatable(entry.getKey())) { - map.put(entry.getKey(), entry.getValue()); + + if (appendNamedParameters) { + LOG.debug("Appending named parameters to the result map"); + //the values map will contain entries like name->"Lex Luthor" and 1->"Lex Luthor" + //now add the non-numeric values + for (Map.Entry entry: vars.entrySet()) { + if (!NumberUtils.isCreatable(entry.getKey())) { + map.put(entry.getKey(), entry.getValue()); + } } } - + return map; } @@ -192,7 +217,7 @@ public abstract class AbstractMatcher implements Serializable { c = val.charAt(x); if (x < len - 2 && c == '{' && '}' == val.charAt(x+2)) { - varVal = (String)vars.get(String.valueOf(val.charAt(x + 1))); + varVal = vars.get(String.valueOf(val.charAt(x + 1))); if (varVal != null) { ret.append(varVal); } @@ -213,18 +238,18 @@ public abstract class AbstractMatcher implements Serializable { /** *

The original pattern.

*/ - private String original; + private final String original; /** *

The compiled pattern.

*/ - private Object pattern; + private final Object pattern; /** *

The original object.

*/ - private E config; + private final E config; /** *

Contructs a read-only Mapping instance.

diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java index b94fff63a..344339e04 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java @@ -58,7 +58,33 @@ public class ActionConfigMatcher extends AbstractMatcher implement public ActionConfigMatcher(PatternMatcher patternMatcher, Map configs, boolean looseMatch) { - super(patternMatcher); + this(patternMatcher, configs, looseMatch, true); + } + + /** + *

Finds and precompiles the wildcard patterns from the ActionConfig + * "path" attributes. ActionConfig's will be evaluated in the order they + * exist in the config file. Only paths that actually contain a + * wildcard will be compiled.

+ * + *

Patterns can optionally be matched "loosely". When + * the end of the pattern matches \*[^*]\*$ (wildcard, no wildcard, + * wildcard), if the pattern fails, it is also matched as if the + * last two characters didn't exist. The goal is to support the + * legacy "*!*" syntax, where the "!*" is optional.

+ * + * @param patternMatcher pattern matcher + * @param configs An array of ActionConfig's to process + * @param looseMatch To loosely match wildcards or not + * @param appendNamedParameters To append named parameters or not + * + * @since 2.5.23 + * See WW-5065 + */ + public ActionConfigMatcher(PatternMatcher patternMatcher, + Map configs, + boolean looseMatch, boolean appendNamedParameters) { + super(patternMatcher, appendNamedParameters); for (Map.Entry entry : configs.entrySet()) { addPattern(entry.getKey(), entry.getValue(), looseMatch); } diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 42cfabbd8..cf46680fa 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -291,6 +291,8 @@ public class DefaultConfiguration implements Configuration { builder.constant(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, "false"); builder.constant(StrutsConstants.STRUTS_I18N_RELOAD, "false"); + builder.constant(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, "true"); + return builder.create(true); } @@ -340,8 +342,12 @@ public class DefaultConfiguration implements Configuration { } PatternMatcher matcher = container.getInstance(PatternMatcher.class); + boolean appendNamedParameters = Boolean.parseBoolean( + container.getInstance(String.class, StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS) + ); + return new RuntimeConfigurationImpl(Collections.unmodifiableMap(namespaceActionConfigs), - Collections.unmodifiableMap(namespaceConfigs), matcher); + Collections.unmodifiableMap(namespaceConfigs), matcher, appendNamedParameters); } private void setDefaultResults(Map results, PackageConfig packageContext) { @@ -419,15 +425,18 @@ public class DefaultConfiguration implements Configuration { public RuntimeConfigurationImpl(Map> namespaceActionConfigs, Map namespaceConfigs, - PatternMatcher matcher) { + PatternMatcher matcher, + boolean appendNamedParameters) + { this.namespaceActionConfigs = namespaceActionConfigs; this.namespaceConfigs = namespaceConfigs; this.namespaceActionConfigMatchers = new LinkedHashMap<>(); - this.namespaceMatcher = new NamespaceMatcher(matcher, namespaceActionConfigs.keySet()); + this.namespaceMatcher = new NamespaceMatcher(matcher, namespaceActionConfigs.keySet(), appendNamedParameters); for (Map.Entry> entry : namespaceActionConfigs.entrySet()) { - namespaceActionConfigMatchers.put(entry.getKey(), new ActionConfigMatcher(matcher, entry.getValue(), true)); + ActionConfigMatcher configMatcher = new ActionConfigMatcher(matcher, entry.getValue(), true, appendNamedParameters); + namespaceActionConfigMatchers.put(entry.getKey(), configMatcher); } } diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/NamespaceMatcher.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/NamespaceMatcher.java index 01decd34d..b24fc759a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/NamespaceMatcher.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/NamespaceMatcher.java @@ -29,9 +29,23 @@ import java.util.Set; * @since 2.1 */ public class NamespaceMatcher extends AbstractMatcher { - public NamespaceMatcher(PatternMatcher patternMatcher, - Set namespaces) { - super(patternMatcher); + + public NamespaceMatcher(PatternMatcher patternMatcher, Set namespaces) { + this(patternMatcher, namespaces, true); + } + + /** + * Matches namespace strings against a wildcard pattern matcher + * + * @param patternMatcher pattern matcher + * @param namespaces A set of namespaces to process + * @param appendNamedParameters To append named parameters or not + * + * @since 2.5.23 + * See WW-5065 + */ + public NamespaceMatcher(PatternMatcher patternMatcher, Set namespaces, boolean appendNamedParameters) { + super(patternMatcher, appendNamedParameters); for (String name : namespaces) { if (!patternMatcher.isLiteral(name)) { addPattern(name, new NamespaceMatch(name, null), false); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java index cffb451e8..63d8fab40 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java @@ -225,6 +225,7 @@ public class XWorkConfigurationProvider implements ConfigurationProvider { props.setProperty(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE.toString()); props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, Boolean.FALSE.toString()); props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Boolean.TRUE.toString()); + props.setProperty(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE.toString()); } } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index a55ed6282..b48b8bea9 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -346,4 +346,7 @@ public final class StrutsConstants { public static final String STRUTS_DISALLOW_PROXY_MEMBER_ACCESS = "struts.disallowProxyMemberAccess"; public static final String STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT = "struts.ognl.autoGrowthCollectionLimit"; + + /** See {@link com.opensymphony.xwork2.config.impl.AbstractMatcher#appendNamedParameters */ + public static final String STRUTS_MATCHER_APPEND_NAMED_PARAMETERS = "struts.matcher.appendNamedParameters"; } diff --git a/core/src/test/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcherTest.java b/core/src/test/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcherTest.java index 7e0a60c7a..ac522563c 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcherTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcherTest.java @@ -24,6 +24,7 @@ import com.opensymphony.xwork2.config.entities.ExceptionMappingConfig; import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.util.WildcardHelper; +import org.apache.struts2.util.RegexPatternMatcher; import java.util.HashMap; import java.util.Map; @@ -142,6 +143,79 @@ public class ActionConfigMatcherTest extends XWorkTestCase { } + /** + * Test to make sure the {@link AbstractMatcher#replaceParameters(Map, Map)} method isn't adding values to the + * return value. + */ + public void testReplaceParametersWithNoAppendingParams() { + Map map = new HashMap<>(); + + HashMap params = new HashMap<>(); + params.put("first", "{1}"); + + ActionConfig config = new ActionConfig.Builder("package", "foo/{one}/{two}/{three}", "foo.bar.Action") + .addParams(params) + .addExceptionMapping(new ExceptionMappingConfig.Builder("foo{1}", "java.lang.{2}Exception", "success{1}") + .addParams(new HashMap<>(params)) + .build()) + .addResultConfig(new ResultConfig.Builder("success{1}", "foo.{2}").addParams(params).build()) + .setStrictMethodInvocation(false) + .build(); + map.put("foo/{one}/{two}/{three}", config); + ActionConfigMatcher replaceMatcher = new ActionConfigMatcher(new RegexPatternMatcher(), map, false, false); + ActionConfig matched = replaceMatcher.match("foo/paramOne/paramTwo/paramThree"); + assertNotNull("ActionConfig should be matched", matched); + + // Verify all The ActionConfig, ExceptionConfig, and ResultConfig have the correct number of params + assertEquals("The ActionConfig should have the correct number of params", 1, matched.getParams().size()); + assertEquals("The ExceptionMappingConfigs should have the correct number of params", 1, matched.getExceptionMappings().get(0).getParams().size()); + assertEquals("The ResultConfigs should have the correct number of params", 1, matched.getResults().get("successparamOne").getParams().size()); + + // Verify the params are still getting their values replaced correctly + assertEquals("The ActionConfig params have replaced values", "paramOne", matched.getParams().get("first")); + assertEquals("The ActionConfig params have replaced values", "paramOne", matched.getExceptionMappings().get(0).getParams().get("first")); + assertEquals("The ActionConfig params have replaced values", "paramOne", matched.getResults().get("successparamOne").getParams().get("first")); + } + + /** + * Test to make sure the {@link AbstractMatcher#replaceParameters(Map, Map)} method is adding values to the + * return value. + */ + public void testReplaceParametersWithAppendingParams() { + Map map = new HashMap<>(); + + HashMap params = new HashMap<>(); + params.put("first", "{1}"); + + ActionConfig config = new ActionConfig.Builder("package", "foo/{one}/{two}/{three}", "foo.bar.Action") + .addParams(params) + .addExceptionMapping(new ExceptionMappingConfig.Builder("foo{1}", "java.lang.{2}Exception", "success{1}") + .addParams(new HashMap<>(params)) + .build()) + .addResultConfig(new ResultConfig.Builder("success{1}", "foo.{2}").addParams(params).build()) + .setStrictMethodInvocation(false) + .build(); + map.put("foo/{one}/{two}/{three}", config); + ActionConfigMatcher replaceMatcher = new ActionConfigMatcher(new RegexPatternMatcher(), map, false, true); + ActionConfig matched = replaceMatcher.match("foo/paramOne/paramTwo/paramThree"); + assertNotNull("ActionConfig should be matched", matched); + + assertEquals(4, matched.getParams().size()); + assertEquals(4, matched.getExceptionMappings().get(0).getParams().size()); + assertEquals(4, matched.getResults().get("successparamOne").getParams().size()); + + // Verify the params are still getting their values replaced correctly + assertEquals("paramOne", matched.getParams().get("first")); + assertEquals("paramOne", matched.getParams().get("one")); + assertEquals("paramTwo", matched.getParams().get("two")); + assertEquals("paramThree", matched.getParams().get("three")); + assertEquals("paramOne", matched.getExceptionMappings().get(0).getParams().get("first")); + assertEquals("paramOne", matched.getExceptionMappings().get(0).getParams().get("one")); + assertEquals("paramTwo", matched.getExceptionMappings().get(0).getParams().get("two")); + assertEquals("paramThree", matched.getExceptionMappings().get(0).getParams().get("three")); + assertEquals("paramOne", matched.getResults().get("successparamOne").getParams().get("first")); + } + private Map buildActionConfigMap() { Map map = new HashMap<>();