You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2020/04/27 18:38:37 UTC

[struts] 01/04: WW-5065 Defines a new flag to control appending params

This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch WW-5065-append-or-not-cherry-pick
in repository https://gitbox.apache.org/repos/asf/struts.git

commit 9b9b3cd264f49ac51526a787a5db57c8c8e30efb
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Wed Apr 22 07:59:13 2020 +0200

    WW-5065 Defines a new flag to control appending params
---
 .../xwork2/config/impl/AbstractMatcher.java        | 39 +++++++++---
 .../xwork2/config/impl/ActionConfigMatcher.java    | 28 ++++++++-
 .../xwork2/config/impl/DefaultConfiguration.java   | 17 +++--
 .../xwork2/config/impl/NamespaceMatcher.java       | 20 +++++-
 .../providers/XWorkConfigurationProvider.java      |  2 +
 .../java/org/apache/struts2/StrutsConstants.java   |  3 +
 .../config/impl/ActionConfigMatcherTest.java       | 73 ++++++++++++++++++++++
 7 files changed, 164 insertions(+), 18 deletions(-)

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 3b3b74b..d170c03 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,6 +36,9 @@ import java.util.*;
  * @since 2.1
  */
 public abstract class AbstractMatcher<E> implements Serializable {
+
+    private static final Logger LOG = LogManager.getLogger(AbstractMatcher.class);
+
     /**
      * <p> The logging instance </p>
      */
@@ -50,10 +53,23 @@ public abstract class AbstractMatcher<E> implements Serializable {
      * <p> The compiled patterns and their associated target objects </p>
      */
     List<Mapping<E>> 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<Object>) helper;
+        this.appendNamedParameters = appendNamedParameters;
     }
 
     /**
@@ -152,20 +168,23 @@ public abstract class AbstractMatcher<E> implements Serializable {
      */
     protected Map<String,String> replaceParameters(Map<String, String> orig, Map<String,String> vars) {
         Map<String, String> map = new LinkedHashMap<>();
-        
+
         //this will set the group index references, like {1}
         for (Map.Entry<String,String> 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<String,String> 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<String,String> entry: vars.entrySet()) {
+                if (!NumberUtils.isCreatable(entry.getKey())) {
+                    map.put(entry.getKey(), entry.getValue());
+                }
             }
         }
-        
+
         return map;
     }
 
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 b94fff6..344339e 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<ActionConfig> implement
     public ActionConfigMatcher(PatternMatcher<?> patternMatcher,
             Map<String, ActionConfig> configs,
             boolean looseMatch) {
-        super(patternMatcher);
+        this(patternMatcher, configs, looseMatch, true);
+    }
+
+    /**
+     * <p> 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. </p>
+     *
+     * <p>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.</p>
+     *
+     * @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<String, ActionConfig> configs,
+            boolean looseMatch, boolean appendNamedParameters) {
+        super(patternMatcher, appendNamedParameters);
         for (Map.Entry<String, ActionConfig> 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 42cfabb..cf46680 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<int[]> 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<String, ResultConfig> results, PackageConfig packageContext) {
@@ -419,15 +425,18 @@ public class DefaultConfiguration implements Configuration {
 
         public RuntimeConfigurationImpl(Map<String, Map<String, ActionConfig>> namespaceActionConfigs,
                                         Map<String, String> namespaceConfigs,
-                                        PatternMatcher<int[]> matcher) {
+                                        PatternMatcher<int[]> 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<String, Map<String, ActionConfig>> 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 01decd3..b24fc75 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<NamespaceMatch> {
-     public NamespaceMatcher(PatternMatcher<?> patternMatcher,
-            Set<String> namespaces) {
-        super(patternMatcher);
+
+    public NamespaceMatcher(PatternMatcher<?> patternMatcher, Set<String> 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<String> 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 cffb451..ecd6d6e 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
@@ -107,6 +107,7 @@ import com.opensymphony.xwork2.validator.DefaultValidatorFactory;
 import com.opensymphony.xwork2.validator.DefaultValidatorFileParser;
 import com.opensymphony.xwork2.validator.ValidatorFactory;
 import com.opensymphony.xwork2.validator.ValidatorFileParser;
+import com.sun.org.apache.xpath.internal.operations.Bool;
 import ognl.MethodAccessor;
 import ognl.PropertyAccessor;
 import org.apache.struts2.StrutsConstants;
@@ -225,6 +226,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 a55ed62..6f71ce8 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 = "";
 }
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 7e0a60c..aa2d648 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
@@ -142,6 +142,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<String, ActionConfig> map = new HashMap<>();
+
+        HashMap<String, String> 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<String, ActionConfig> map = new HashMap<>();
+
+        HashMap<String, String> 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<String,ActionConfig> buildActionConfigMap() {
         Map<String, ActionConfig> map = new HashMap<>();