struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Huber <gregh3...@gmail.com>
Subject Re: struts git commit: WW-4669 Returns default action/method instead of throwing exception
Date Fri, 29 Jul 2016 08:28:51 GMT
Have you an example of what the default action/name would actually be?


<action name="messageDelete!*" method="{1}" class="admin.MessageDelete">
            <result name="input" type="tiles">.MessageDelete</result>
            <result name="confirm" type="tiles">.MessageDelete</result>
            <result name="success" type="chain">messages</result>
            <result name="cancel" type="redirectAction">
                <param name="actionName">messages</param>
            </result>
            <result name="error" type="chain">messages</result>
        </action>






On 29 July 2016 at 09:11, Lukasz Lenart <lukaszlenart@apache.org> wrote:

> Hi,
>
> I have changed logic for cleaning up action/method names - instead of
> throwing exception a default action/method name will be returned,
> wdyt?
> I must updated docs but will wait for your opinion.
>
>
> Regards
> --
> Ɓukasz
> + 48 606 323 122 http://www.lenart.org.pl/
>
>
> ---------- Forwarded message ----------
> From:  <lukaszlenart@apache.org>
> Date: 2016-07-29 10:09 GMT+02:00
> Subject: struts git commit: WW-4669 Returns default action/method
> instead of throwing exception
> To: commits@struts.apache.org
>
>
> Repository: struts
> Updated Branches:
>   refs/heads/master d19b9eaa8 -> 5acc71f7c
>
>
> WW-4669 Returns default action/method instead of throwing exception
>
>
> Project: http://git-wip-us.apache.org/repos/asf/struts/repo
> Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/5acc71f7
> Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/5acc71f7
> Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/5acc71f7
>
> Branch: refs/heads/master
> Commit: 5acc71f7c30e16807912bf99b4c32cc4b25f586e
> Parents: d19b9ea
> Author: Lukasz Lenart <lukaszlenart@apache.org>
> Authored: Fri Jul 29 10:09:24 2016 +0200
> Committer: Lukasz Lenart <lukaszlenart@apache.org>
> Committed: Fri Jul 29 10:09:24 2016 +0200
>
> ----------------------------------------------------------------------
>  .../org/apache/struts2/StrutsConstants.java     |  7 ++++
>  .../dispatcher/mapper/DefaultActionMapper.java  | 44 ++++++++++++++++++--
>  .../mapper/DefaultActionMapperTest.java         | 44 ++++++++------------
>  .../apache/struts2/rest/RestActionMapper.java   |  5 ++-
>  4 files changed, 68 insertions(+), 32 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/main/java/org/apache/struts2/StrutsConstants.java
> ----------------------------------------------------------------------
> diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
> b/core/src/main/java/org/apache/struts2/StrutsConstants.java
> index 186e880..c41d542 100644
> --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
> +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
> @@ -268,6 +268,13 @@ public final class StrutsConstants {
>
>      /** actions names' whitelist **/
>      public static final String STRUTS_ALLOWED_ACTION_NAMES =
> "struts.allowed.action.names";
> +    /** default action name to use when action didn't match the whitelist
> **/
> +    public static final String STRUTS_DEFAULT_ACTION_NAME =
> "struts.default.action.name";
> +
> +    /** methods names' whitelist **/
> +    public static final String STRUTS_ALLOWED_METHOD_NAMES =
> "struts.allowed.method.names";
> +    /** default method name to use when method didn't match the whitelist
> **/
> +    public static final String STRUTS_DEFAULT_METHOD_NAME =
> "struts.default.method.name";
>
>      /** enables action: prefix **/
>      public static final String STRUTS_MAPPER_ACTION_PREFIX_ENABLED =
> "struts.mapper.action.prefix.enabled";
>
>
> http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
> ----------------------------------------------------------------------
> diff --git
> a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
>
> b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
> index d0e89be..f1fcfee 100644
> ---
> a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
> +++
> b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
> @@ -121,6 +121,11 @@ public class DefaultActionMapper implements
> ActionMapper {
>      protected boolean alwaysSelectFullNamespace = false;
>      protected PrefixTrie prefixTrie = null;
>      protected Pattern allowedActionNames =
> Pattern.compile("[a-zA-Z0-9._!/\\-]*");
> +    protected String defaultActionName = "index";
> +
> +    protected Pattern allowedMethodNames =
> Pattern.compile("[a-zA-Z_]*[0-9]*");
> +    protected String defaultMethodName = "execute";
> +
>      private boolean allowActionPrefix = false;
>      private boolean allowActionCrossNamespaceAccess = false;
>
> @@ -137,7 +142,7 @@ public class DefaultActionMapper implements
> ActionMapper {
>                  put(METHOD_PREFIX, new ParameterAction() {
>                      public void execute(String key, ActionMapping
> mapping) {
>                          if (allowDynamicMethodCalls) {
> -
>
> mapping.setMethod(cleanupActionName(key.substring(METHOD_PREFIX.length())));
> +
>
> mapping.setMethod(cleanupMethodName(key.substring(METHOD_PREFIX.length())));
>                          }
>                      }
>                  });
> @@ -149,7 +154,7 @@ public class DefaultActionMapper implements
> ActionMapper {
>                              if (allowDynamicMethodCalls) {
>                                  int bang = name.indexOf('!');
>                                  if (bang != -1) {
> -                                    String method =
> cleanupActionName(name.substring(bang + 1));
> +                                    String method =
> cleanupMethodName(name.substring(bang + 1));
>                                      mapping.setMethod(method);
>                                      name = name.substring(0, bang);
>                                  }
> @@ -205,6 +210,21 @@ public class DefaultActionMapper implements
> ActionMapper {
>          this.allowedActionNames = Pattern.compile(allowedActionNames);
>      }
>
> +    @Inject(value = StrutsConstants.STRUTS_DEFAULT_ACTION_NAME,
> required = false)
> +    public void setDefaultActionName(String defaultActionName) {
> +        this.defaultActionName = defaultActionName;
> +    }
> +
> +    @Inject(value = StrutsConstants.STRUTS_ALLOWED_METHOD_NAMES,
> required = false)
> +    public void setAllowedMethodNames(String allowedMethodNames) {
> +        this.allowedMethodNames = Pattern.compile(allowedMethodNames);
> +    }
> +
> +    @Inject(value = StrutsConstants.STRUTS_DEFAULT_METHOD_NAME,
> required = false)
> +    public void setDefaultMethodName(String defaultMethodName) {
> +        this.defaultMethodName = defaultMethodName;
> +    }
> +
>      @Inject(value = StrutsConstants.STRUTS_MAPPER_ACTION_PREFIX_ENABLED)
>      public void setAllowActionPrefix(String allowActionPrefix) {
>          this.allowActionPrefix =
> BooleanUtils.toBoolean(allowActionPrefix);
> @@ -377,7 +397,7 @@ public class DefaultActionMapper implements
> ActionMapper {
>      }
>
>      /**
> -     * Cleans up action name from suspicious characters
> +     * Checks action name against allowed pattern if not matched
> returns default action name
>       *
>       * @param rawActionName action name extracted from URI
>       * @return safe action name
> @@ -386,7 +406,23 @@ public class DefaultActionMapper implements
> ActionMapper {
>          if (allowedActionNames.matcher(rawActionName).matches()) {
>              return rawActionName;
>          } else {
> -            throw new StrutsException("Action [" + rawActionName + "]
> does not match allowed action names pattern [" + allowedActionNames +
> "]!");
> +            LOG.warn("{} did not match allowed action names {} -
> default action {} will be used!", rawActionName, allowedActionNames,
> defaultActionName);
> +            return defaultActionName;
> +        }
> +    }
> +
> +    /**
> +     * Checks method name (when DMI is enabled) against allowed
> pattern if not matched returns default action name
> +     *
> +     * @param rawMethodName method name extracted from URI
> +     * @return safe method name
> +     */
> +    protected String cleanupMethodName(final String rawMethodName) {
> +        if (allowedMethodNames.matcher(rawMethodName).matches()) {
> +            return rawMethodName;
> +        } else {
> +            LOG.warn("{} did not match allowed method names {} -
> default method {} will be used!", rawMethodName, allowedMethodNames,
> defaultMethodName);
> +            return defaultMethodName;
>          }
>      }
>
>
>
> http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
> ----------------------------------------------------------------------
> diff --git
> a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
>
> b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
> index b51f569..2008e38 100644
> ---
> a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
> +++
> b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
> @@ -845,37 +845,14 @@ public class DefaultActionMapperTest extends
> StrutsInternalTestCase {
>          String actionName = "action";
>          assertEquals(actionName, mapper.cleanupActionName(actionName));
>
> -        Throwable expected = null;
> -
>          actionName = "${action}";
> -        try {
> -            mapper.cleanupActionName(actionName);
> -            fail();
> -        } catch (Throwable t) {
> -            expected = t;
> -        }
> -        assertTrue(expected instanceof StrutsException);
> -        assertEquals("Action [${action}] does not match allowed
> action names pattern [" + mapper.allowedActionNames.pattern() + "]!",
> expected.getMessage());
> +        assertEquals(mapper.defaultActionName,
> mapper.cleanupActionName(actionName));
>
>          actionName = "${${%{action}}}";
> -        try {
> -            mapper.cleanupActionName(actionName);
> -            fail();
> -        } catch (Throwable t) {
> -            expected = t;
> -        }
> -        assertTrue(expected instanceof StrutsException);
> -        assertEquals("Action [${${%{action}}}] does not match allowed
> action names pattern [" + mapper.allowedActionNames.pattern() + "]!",
> expected.getMessage());
> +        assertEquals(mapper.defaultActionName,
> mapper.cleanupActionName(actionName));
>
>          actionName = "${#foo='action',#foo}";
> -        try {
> -            mapper.cleanupActionName(actionName);
> -            fail();
> -        } catch (Throwable t) {
> -            expected = t;
> -        }
> -        assertTrue(expected instanceof StrutsException);
> -        assertEquals("Action [${#foo='action',#foo}] does not match
> allowed action names pattern [" + mapper.allowedActionNames.pattern()
> + "]!", expected.getMessage());
> +        assertEquals(mapper.defaultActionName,
> mapper.cleanupActionName(actionName));
>
>          actionName = "test-action";
>          assertEquals("test-action", mapper.cleanupActionName(actionName));
> @@ -887,4 +864,19 @@ public class DefaultActionMapperTest extends
> StrutsInternalTestCase {
>          assertEquals("test!bar.action",
> mapper.cleanupActionName(actionName));
>      }
>
> +    public void testAllowedMethodNames() throws Exception {
> +        DefaultActionMapper mapper = new DefaultActionMapper();
> +
> +        assertEquals("", mapper.cleanupMethodName(""));
> +        assertEquals("test", mapper.cleanupMethodName("test"));
> +        assertEquals("test_method",
> mapper.cleanupMethodName("test_method"));
> +        assertEquals("_test", mapper.cleanupMethodName("_test"));
> +        assertEquals("test1", mapper.cleanupMethodName("test1"));
> +
> +        assertEquals(mapper.defaultMethodName,
> mapper.cleanupMethodName("2test"));
> +        assertEquals(mapper.defaultMethodName,
> mapper.cleanupMethodName("%{exp}"));
> +        assertEquals(mapper.defaultMethodName,
> mapper.cleanupMethodName("${%{foo}}"));
> +        assertEquals(mapper.defaultMethodName,
> mapper.cleanupMethodName("${#foo='method',#foo}"));
> +    }
> +
>  }
>
>
> http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
> ----------------------------------------------------------------------
> diff --git
> a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
> b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
> index 678a6f3..a56c8e1 100644
> ---
> a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
> +++
> b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
> @@ -121,6 +121,7 @@ public class RestActionMapper extends
> DefaultActionMapper {
>      private boolean allowDynamicMethodCalls = false;
>
>      public RestActionMapper() {
> +        this.defaultMethodName = indexMethodName;
>      }
>
>      public String getIdParameterName() {
> @@ -299,7 +300,7 @@ public class RestActionMapper extends
> DefaultActionMapper {
>                  fullName = fullName.substring(0, lastSlashPos);
>              }
>
> -            mapping.setName(fullName);
> +            mapping.setName(cleanupActionName(fullName));
>          }
>          return mapping;
>      }
> @@ -320,7 +321,7 @@ public class RestActionMapper extends
> DefaultActionMapper {
>
>              mapping.setName(actionName);
>              if (allowDynamicMethodCalls) {
> -                mapping.setMethod(cleanupActionName(actionMethod));
> +                mapping.setMethod(cleanupMethodName(actionMethod));
>              } else {
>                  mapping.setMethod(null);
>              }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message