WW-4669 Returns default action/method instead of throwing exception

This commit is contained in:
Lukasz Lenart
2016-07-29 10:09:24 +02:00
parent d19b9eaa82
commit 5acc71f7c3
4 changed files with 68 additions and 32 deletions
@@ -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";
@@ -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;
}
}
@@ -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}"));
}
}
@@ -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);
}