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 <[email protected]> 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: <[email protected]>
> Date: 2016-07-29 10:09 GMT+02:00
> Subject: struts git commit: WW-4669 Returns default action/method
> instead of throwing exception
> To: [email protected]
>
>
> 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 <[email protected]>
> Authored: Fri Jul 29 10:09:24 2016 +0200
> Committer: Lukasz Lenart <[email protected]>
> 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: [email protected]
> For additional commands, e-mail: [email protected]
>
>
