> 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. > >
I'm fine with either way. Regards, Christoph > Regards > -- > Łukasz > + 48 606 323 122 http://www.lenart.org.pl/ > > > ---------- Forwarded message ---------- > From: <lukaszlen...@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: comm...@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 <lukaszlen...@apache.org> > Authored: Fri Jul 29 10:09:24 2016 +0200 > Committer: Lukasz Lenart <lukaszlen...@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-unsubscr...@struts.apache.org > For additional commands, e-mail: dev-h...@struts.apache.org > This Email was scanned by Sophos Anti Virus
