From 065b5b79ae068ab7891a4232a0769290fd21bb17 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:31:59 +0200 Subject: [PATCH 01/32] Drops wildcard as a valid action method --- .../xwork2/config/entities/ActionConfig.java | 40 ++++++------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index b32f2cf2df..b947ed96b7 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -42,7 +42,6 @@ public class ActionConfig extends Located implements Serializable { public static final String DEFAULT_METHOD = "execute"; - public static final String WILDCARD = "*"; protected List interceptors; // a list of interceptorMapping Objects eg. List protected Map params; @@ -58,11 +57,11 @@ protected ActionConfig(String packageName, String name, String className) { this.packageName = packageName; this.name = name; this.className = className; - params = new LinkedHashMap(); - results = new LinkedHashMap(); - interceptors = new ArrayList(); - exceptionMappings = new ArrayList(); - allowedMethods = new HashSet(); + params = new LinkedHashMap<>(); + results = new LinkedHashMap<>(); + interceptors = new ArrayList<>(); + exceptionMappings = new ArrayList<>(); + allowedMethods = new HashSet<>(Collections.singletonList(DEFAULT_METHOD)); } /** @@ -128,11 +127,7 @@ public Map getResults() { } public boolean isAllowedMethod(String method) { - if (allowedMethods.size() == 1 && WILDCARD.equals(allowedMethods.iterator().next())) { - return true; - } else { - return method.equals(methodName != null ? methodName : DEFAULT_METHOD) || allowedMethods.contains(method); - } + return method.equals(methodName != null ? methodName : DEFAULT_METHOD) || allowedMethods.contains(method); } @Override public boolean equals(Object o) { @@ -214,7 +209,6 @@ public boolean isAllowedMethod(String method) { public static class Builder implements InterceptorListHolder{ protected ActionConfig target; - private boolean gotMethods; public Builder(ActionConfig toClone) { target = new ActionConfig(toClone); @@ -249,6 +243,7 @@ public Builder defaultClassName(String name) { public Builder methodName(String method) { target.methodName = method; + addAllowedMethod(method); return this; } @@ -317,10 +312,7 @@ public Builder addAllowedMethod(String methodName) { } public Builder addAllowedMethod(Collection methods) { - if (methods != null) { - gotMethods = true; - target.allowedMethods.addAll(methods); - } + target.allowedMethods.addAll(methods); return this; } @@ -330,22 +322,16 @@ public Builder location(Location loc) { } public ActionConfig build() { - embalmTarget(); - ActionConfig result = target; - target = new ActionConfig(target); - return result; - } - - protected void embalmTarget() { - if (!gotMethods && target.allowedMethods.isEmpty()) { - target.allowedMethods.add(WILDCARD); - } - target.params = Collections.unmodifiableMap(target.params); target.results = Collections.unmodifiableMap(target.results); target.interceptors = Collections.unmodifiableList(target.interceptors); target.exceptionMappings = Collections.unmodifiableList(target.exceptionMappings); target.allowedMethods = Collections.unmodifiableSet(target.allowedMethods); + + ActionConfig result = target; + target = new ActionConfig(target); + return result; } + } } From ce884e92a15ef601b0e119963d3c521fa68d8bb1 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:33:31 +0200 Subject: [PATCH 02/32] Defines global-allowed-methods --- .../xwork2/config/entities/ActionConfig.java | 1 + .../providers/XmlConfigurationProvider.java | 48 +++++- .../StrutsXmlConfigurationProvider.java | 1 + core/src/main/resources/struts-2.5.dtd | 156 ++++++++++++++++++ core/src/main/resources/struts-default.xml | 6 +- core/src/main/resources/xwork-2.5.dtd | 135 +++++++++++++++ 6 files changed, 336 insertions(+), 11 deletions(-) create mode 100644 core/src/main/resources/struts-2.5.dtd create mode 100644 core/src/main/resources/xwork-2.5.dtd diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index b947ed96b7..fd61ad95ec 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -42,6 +42,7 @@ public class ActionConfig extends Located implements Serializable { public static final String DEFAULT_METHOD = "execute"; + public static final String WILDCARD = "*"; protected List interceptors; // a list of interceptorMapping Objects eg. List protected Map params; diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 12a71c7421..449d254714 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -15,12 +15,24 @@ */ package com.opensymphony.xwork2.config.providers; -import com.opensymphony.xwork2.*; +import com.opensymphony.xwork2.Action; +import com.opensymphony.xwork2.FileManager; +import com.opensymphony.xwork2.FileManagerFactory; +import com.opensymphony.xwork2.ObjectFactory; +import com.opensymphony.xwork2.XWorkException; import com.opensymphony.xwork2.config.Configuration; import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.ConfigurationProvider; import com.opensymphony.xwork2.config.ConfigurationUtil; -import com.opensymphony.xwork2.config.entities.*; +import com.opensymphony.xwork2.config.entities.ActionConfig; +import com.opensymphony.xwork2.config.entities.ExceptionMappingConfig; +import com.opensymphony.xwork2.config.entities.InterceptorConfig; +import com.opensymphony.xwork2.config.entities.InterceptorMapping; +import com.opensymphony.xwork2.config.entities.InterceptorStackConfig; +import com.opensymphony.xwork2.config.entities.PackageConfig; +import com.opensymphony.xwork2.config.entities.ResultConfig; +import com.opensymphony.xwork2.config.entities.ResultTypeConfig; +import com.opensymphony.xwork2.config.entities.UnknownHandlerConfig; import com.opensymphony.xwork2.config.impl.LocatableFactory; import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.ContainerBuilder; @@ -47,7 +59,17 @@ import java.io.InputStream; import java.lang.reflect.Modifier; import java.net.URL; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.Vector; /** @@ -90,6 +112,7 @@ public XmlConfigurationProvider(String filename, boolean errorIfMissing) { this.errorIfMissing = errorIfMissing; Map mappings = new HashMap<>(); + mappings.put("-//Apache Struts//XWork 2.5//EN", "xwork-2.5.dtd"); mappings.put("-//Apache Struts//XWork 2.3//EN", "xwork-2.3.dtd"); mappings.put("-//Apache Struts//XWork 2.1.3//EN", "xwork-2.1.3.dtd"); mappings.put("-//Apache Struts//XWork 2.1//EN", "xwork-2.1.dtd"); @@ -522,6 +545,8 @@ protected PackageConfig addPackage(Element packageElement) throws ConfigurationE // load the global result list for this package loadGlobalResults(newPackage, packageElement); + loadGlobalAllowedMethods(newPackage, packageElement); + // load the global exception handler list for this package loadGobalExceptionMappings(newPackage, packageElement); @@ -623,8 +648,6 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { boolean isAbstract = Boolean.parseBoolean(abstractVal); String name = StringUtils.defaultString(packageElement.getAttribute("name")); String namespace = StringUtils.defaultString(packageElement.getAttribute("namespace")); - String strictDMIVal = StringUtils.defaultString(packageElement.getAttribute("strict-method-invocation")); - boolean strictDMI = Boolean.parseBoolean(strictDMIVal); if (StringUtils.isNotEmpty(packageElement.getAttribute("externalReferenceResolver"))) { throw new ConfigurationException("The 'externalReferenceResolver' attribute has been removed. Please use " + @@ -634,7 +657,6 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { PackageConfig.Builder cfg = new PackageConfig.Builder(name) .namespace(namespace) .isAbstract(isAbstract) - .strictMethodInvocation(strictDMI) .location(DomHelper.getLocationObject(packageElement)); if (StringUtils.isNotEmpty(StringUtils.defaultString(parent))) { // has parents, let's look it up @@ -825,7 +847,7 @@ protected List buildExceptionMappings(Element element, P protected Set buildAllowedMethods(Element element, PackageConfig.Builder packageContext) { NodeList allowedMethodsEls = element.getElementsByTagName("allowed-methods"); - Set allowedMethods = null; + Set allowedMethods = packageContext.getGlobalAllowedMethods(); if (allowedMethodsEls.getLength() > 0) { allowedMethods = new HashSet<>(); @@ -836,8 +858,6 @@ protected Set buildAllowedMethods(Element element, PackageConfig.Builder allowedMethods = TextParseUtil.commaDelimitedStringToSet(s); } } - } else if (packageContext.isStrictMethodInvocation()) { - allowedMethods = new HashSet<>(); } return allowedMethods; @@ -877,6 +897,16 @@ protected void loadGlobalResults(PackageConfig.Builder packageContext, Element p } } + protected void loadGlobalAllowedMethods(PackageConfig.Builder packageContext, Element packageElement) { + NodeList globalAllowedMethods = packageElement.getElementsByTagName("global-allowed-methods"); + + if (globalAllowedMethods.getLength() > 0) { + Element globalAllowedMethodsElement = (Element) globalAllowedMethods.item(0); + Set results = TextParseUtil.commaDelimitedStringToSet(globalAllowedMethodsElement.getAttribute("methods")); + packageContext.addGlobalAllowedMethods(results); + } + } + protected void loadDefaultClassRef(PackageConfig.Builder packageContext, Element element) { NodeList defaultClassRefList = element.getElementsByTagName("default-class-ref"); if (defaultClassRefList.getLength() > 0) { diff --git a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java index cca80b7822..e39e475490 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java @@ -75,6 +75,7 @@ public StrutsXmlConfigurationProvider(String filename, boolean errorIfMissing, S dtdMappings.put("-//Apache Software Foundation//DTD Struts Configuration 2.1//EN", "struts-2.1.dtd"); dtdMappings.put("-//Apache Software Foundation//DTD Struts Configuration 2.1.7//EN", "struts-2.1.7.dtd"); dtdMappings.put("-//Apache Software Foundation//DTD Struts Configuration 2.3//EN", "struts-2.3.dtd"); + dtdMappings.put("-//Apache Software Foundation//DTD Struts Configuration 2.5//EN", "struts-2.5.dtd"); setDtdMappings(dtdMappings); File file = new File(filename); if (file.getParent() != null) { diff --git a/core/src/main/resources/struts-2.5.dtd b/core/src/main/resources/struts-2.5.dtd new file mode 100644 index 0000000000..86afbc029a --- /dev/null +++ b/core/src/main/resources/struts-2.5.dtd @@ -0,0 +1,156 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index ca2a6544f3..9fc1c21068 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -33,8 +33,8 @@ and {@link com.opensymphony.xwork2.inject.Inject} --> + "-//Apache Software Foundation//DTD Struts Configuration 2.5//EN" + "http://struts.apache.org/dtds/struts-2.5.dtd"> @@ -358,6 +358,8 @@ + + diff --git a/core/src/main/resources/xwork-2.5.dtd b/core/src/main/resources/xwork-2.5.dtd new file mode 100644 index 0000000000..efbd7652d1 --- /dev/null +++ b/core/src/main/resources/xwork-2.5.dtd @@ -0,0 +1,135 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From fd22e3a16c88ef0528c1e26e0d6bdfdf1c02c755 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:35:16 +0200 Subject: [PATCH 03/32] Uses global-allowed-methods config para --- .../xwork2/config/entities/PackageConfig.java | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index c9ef12d016..19ce36fc8d 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -40,6 +40,7 @@ public class PackageConfig extends Located implements Comparable, Serializable, protected Map actionConfigs; protected Map globalResultConfigs; + protected Set globalAllowedMethods; protected Map interceptorConfigs; protected Map resultTypeConfigs; protected List globalExceptionMappingConfigs; @@ -57,6 +58,7 @@ protected PackageConfig(String name) { this.name = name; actionConfigs = new LinkedHashMap<>(); globalResultConfigs = new LinkedHashMap<>(); + globalAllowedMethods = new HashSet<>(); interceptorConfigs = new LinkedHashMap<>(); resultTypeConfigs = new LinkedHashMap<>(); globalExceptionMappingConfigs = new ArrayList<>(); @@ -74,6 +76,7 @@ protected PackageConfig(PackageConfig orig) { this.needsRefresh = orig.needsRefresh; this.actionConfigs = new LinkedHashMap<>(orig.actionConfigs); this.globalResultConfigs = new LinkedHashMap<>(orig.globalResultConfigs); + this.globalAllowedMethods = new LinkedHashSet<>(orig.globalAllowedMethods); this.interceptorConfigs = new LinkedHashMap<>(orig.interceptorConfigs); this.resultTypeConfigs = new LinkedHashMap<>(orig.resultTypeConfigs); this.globalExceptionMappingConfigs = new ArrayList<>(orig.globalExceptionMappingConfigs); @@ -445,7 +448,6 @@ public Object getInterceptorConfig(String name) { public static class Builder implements InterceptorLocator { protected PackageConfig target; - private boolean strictDMI; public Builder(String name) { target = new PackageConfig(name); @@ -528,6 +530,15 @@ public Builder addGlobalResultConfigs(Map resultConfigs) { return this; } + public Set getGlobalAllowedMethods() { + return target.globalAllowedMethods; + } + + public Builder addGlobalAllowedMethods(Set allowedMethods) { + target.globalAllowedMethods.addAll(allowedMethods); + return this; + } + public Builder addExceptionMappingConfig(ExceptionMappingConfig exceptionMappingConfig) { target.globalExceptionMappingConfigs.add(exceptionMappingConfig); return this; @@ -591,15 +602,6 @@ public Object getInterceptorConfig(String name) { return target.getAllInterceptorConfigs().get(name); } - public Builder strictMethodInvocation(boolean strict) { - strictDMI = strict; - return this; - } - - public boolean isStrictMethodInvocation() { - return strictDMI; - } - public PackageConfig build() { embalmTarget(); PackageConfig result = target; From 55b8070048cbec0a6e08b1781f81b1bfdb3354f2 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:41:57 +0200 Subject: [PATCH 04/32] Drops strict DMI --- core/src/main/resources/xwork-2.5.dtd | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/resources/xwork-2.5.dtd b/core/src/main/resources/xwork-2.5.dtd index efbd7652d1..4d3e1ca845 100644 --- a/core/src/main/resources/xwork-2.5.dtd +++ b/core/src/main/resources/xwork-2.5.dtd @@ -22,7 +22,6 @@ extends CDATA #IMPLIED namespace CDATA #IMPLIED abstract CDATA #IMPLIED - strict-method-invocation CDATA #IMPLIED > From fb0c4a58507c7fb1af135bb376af5b475f43d7ee Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:42:44 +0200 Subject: [PATCH 05/32] Drops outdated attribute --- .../xwork2/config/providers/XmlConfigurationProvider.java | 5 ----- core/src/main/resources/struts-2.5.dtd | 1 - 2 files changed, 6 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 449d254714..fd9f92a921 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -649,11 +649,6 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { String name = StringUtils.defaultString(packageElement.getAttribute("name")); String namespace = StringUtils.defaultString(packageElement.getAttribute("namespace")); - if (StringUtils.isNotEmpty(packageElement.getAttribute("externalReferenceResolver"))) { - throw new ConfigurationException("The 'externalReferenceResolver' attribute has been removed. Please use " + - "a custom ObjectFactory or Interceptor.", packageElement); - } - PackageConfig.Builder cfg = new PackageConfig.Builder(name) .namespace(namespace) .isAbstract(isAbstract) diff --git a/core/src/main/resources/struts-2.5.dtd b/core/src/main/resources/struts-2.5.dtd index 86afbc029a..25ca800278 100644 --- a/core/src/main/resources/struts-2.5.dtd +++ b/core/src/main/resources/struts-2.5.dtd @@ -43,7 +43,6 @@ extends CDATA #IMPLIED namespace CDATA #IMPLIED abstract CDATA #IMPLIED - externalReferenceResolver NMTOKEN #IMPLIED > From 4565993463f660e9be90b9fe9c3597ce54b58917 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:43:21 +0200 Subject: [PATCH 06/32] Extends Unknown Handler to allowed check if method is allowed --- .../xwork2/DefaultUnknownHandlerManager.java | 10 ++++++++++ .../java/com/opensymphony/xwork2/UnknownHandler.java | 10 ++++++++++ .../com/opensymphony/xwork2/UnknownHandlerManager.java | 3 +++ 3 files changed, 23 insertions(+) diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java b/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java index 5cafe9a265..04ac430736 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java @@ -145,4 +145,14 @@ public boolean hasUnknownHandlers() { public List getUnknownHandlers() { return unknownHandlers; } + + @Override + public boolean isAllowedMethod(String allowedMethod, ActionConfig config) { + for (UnknownHandler unknownHandler : unknownHandlers) { + if (unknownHandler.isAllowedMethod(allowedMethod, config)) { + return true; + } + } + return false; + } } diff --git a/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java b/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java index 51556d6a44..1e6f5aa556 100644 --- a/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java +++ b/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java @@ -60,4 +60,14 @@ public interface UnknownHandler { */ Object handleUnknownActionMethod(Object action, String methodName); + /** + * Handles the case when an action method is unknown and cannot be validated if it is allowed to be executed. + * + * @since 2.5 + * @param allowedMethod The method's name + * @param actionConfig The config of actions + * @return true if allowedMethod can be executed, false otherwise + */ + boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig); + } diff --git a/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java b/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java index f5e254299c..caf63b33dc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java @@ -43,4 +43,7 @@ public interface UnknownHandlerManager { boolean hasUnknownHandlers(); List getUnknownHandlers(); + + boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig); + } From c3f4457b8b8ad6bd0e89646d825f2ef5f9f91118 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 14:43:31 +0200 Subject: [PATCH 07/32] Fixes tests --- .../xwork2/DefaultActionProxy.java | 11 ++++++----- .../xwork2/ActionInvocationTest.java | 18 ++++++++++++++---- .../config/providers/SomeUnknownHandler.java | 5 +++++ core/src/test/resources/xwork-sample.xml | 6 ++++-- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java index d1bfd71385..6b8749ebcd 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java @@ -194,12 +194,13 @@ protected void prepare() { resolveMethod(); - if (!config.isAllowedMethod(method)) { - throw new ConfigurationException("Invalid method: " + method + " for action " + actionName); + if (config.isAllowedMethod(method) || + (unknownHandlerManager.hasUnknownHandlers() && unknownHandlerManager.isAllowedMethod(method, config)) + ) { + invocation.init(this); + } else { + throw new ConfigurationException("This method: " + method + " for action " + actionName + " is not allowed!"); } - - invocation.init(this); - } finally { UtilTimerStack.pop(profileKey); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java b/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java index 05c09b23df..37aa2a534d 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java @@ -17,6 +17,7 @@ import com.opensymphony.xwork2.config.entities.ActionConfig; import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; +import com.opensymphony.xwork2.mock.MockResult; import java.util.HashMap; @@ -45,12 +46,12 @@ public void testCommandInvocationDoMethod() throws Exception { public void testCommandInvocationUnknownHandler() throws Exception { - DefaultActionProxy baseActionProxy = (DefaultActionProxy) actionProxyFactory.createActionProxy( - "baz", "unknownMethodTest", "unknownmethod", null); UnknownHandler unknownHandler = new UnknownHandler() { - public ActionConfig handleUnknownAction(String namespace, String actionName) throws XWorkException { return null;} + public ActionConfig handleUnknownAction(String namespace, String actionName) throws XWorkException { + return new ActionConfig.Builder("test", actionName, ActionSupport.class.getName()).build(); + } public Result handleUnknownResult(ActionContext actionContext, String actionName, ActionConfig actionConfig, String resultCode) throws XWorkException { - return null; + return new MockResult(); } public Object handleUnknownActionMethod(Object action, String methodName) { if (methodName.equals("unknownmethod")) { @@ -59,10 +60,19 @@ public Object handleUnknownActionMethod(Object action, String methodName) { return null; } } + public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { + return "unknownmethod".equals(allowedMethod); + } }; UnknownHandlerManagerMock uhm = new UnknownHandlerManagerMock(); uhm.addUnknownHandler(unknownHandler); + + loadButAdd(UnknownHandlerManager.class, uhm); + + DefaultActionProxy baseActionProxy = (DefaultActionProxy) actionProxyFactory.createActionProxy( + "baz", "unknownMethodTest", "unknownmethod", null); + ((DefaultActionInvocation)baseActionProxy.getInvocation()).setUnknownHandlerManager(uhm); assertEquals("found", baseActionProxy.execute()); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java index 312f4c9a48..5e922e0fc2 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java @@ -38,6 +38,11 @@ public Result handleUnknownResult(ActionContext actionContext, String actionName return null; } + @Override + public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { + return false; + } + public void setActionConfig(ActionConfig actionConfig) { this.actionConfig = actionConfig; } diff --git a/core/src/test/resources/xwork-sample.xml b/core/src/test/resources/xwork-sample.xml index 3705298654..cd2ddcb9c7 100644 --- a/core/src/test/resources/xwork-sample.xml +++ b/core/src/test/resources/xwork-sample.xml @@ -1,6 +1,6 @@ @@ -14,6 +14,8 @@ + + 17 23 From c1928ad06bdfbe245b1ed7d5bfeb07ed9bface37 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 18:36:04 +0200 Subject: [PATCH 08/32] Fixes tests --- ...XmlConfigurationProviderAllowedMethodsTest.java | 14 +++++++------- .../providers/xwork-test-allowed-methods.xml | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java index 6a55e75a88..11f779ac6b 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java @@ -27,12 +27,12 @@ public void testDefaultAllowedMethods() throws ConfigurationException { assertEquals(5, actionConfigs.size()); ActionConfig action = (ActionConfig) actionConfigs.get("Default"); - assertEquals(1, action.getAllowedMethods().size()); + assertEquals(0, action.getAllowedMethods().size()); assertTrue(action.isAllowedMethod("execute")); - assertTrue(action.isAllowedMethod("foo")); - assertTrue(action.isAllowedMethod("bar")); - assertTrue(action.isAllowedMethod("baz")); - assertTrue(action.isAllowedMethod("xyz")); + assertFalse(action.isAllowedMethod("foo")); + assertFalse(action.isAllowedMethod("bar")); + assertFalse(action.isAllowedMethod("baz")); + assertFalse(action.isAllowedMethod("xyz")); action = (ActionConfig) actionConfigs.get("Boring"); assertEquals(0, action.getAllowedMethods().size()); @@ -59,7 +59,7 @@ public void testDefaultAllowedMethods() throws ConfigurationException { assertFalse(action.isAllowedMethod("xyz")); action = (ActionConfig) actionConfigs.get("Baz"); - assertEquals(2, action.getAllowedMethods().size()); + assertEquals(3, action.getAllowedMethods().size()); assertFalse(action.isAllowedMethod("execute")); assertTrue(action.isAllowedMethod("foo")); assertTrue(action.isAllowedMethod("bar")); @@ -114,7 +114,7 @@ public void testStrictAllowedMethods() throws ConfigurationException { assertFalse(action.isAllowedMethod("xyz")); action = (ActionConfig) actionConfigs.get("Baz"); - assertEquals(2, action.getAllowedMethods().size()); + assertEquals(3, action.getAllowedMethods().size()); assertFalse(action.isAllowedMethod("execute")); assertTrue(action.isAllowedMethod("foo")); assertTrue(action.isAllowedMethod("bar")); diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml index 7851408343..4059d6252b 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml @@ -25,7 +25,7 @@ - + From 3b31c428856766389ad6df4ba1edc3d60ecf5e24 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 18:36:29 +0200 Subject: [PATCH 09/32] Adds support for wildcards --- .../xwork2/config/entities/ActionConfig.java | 22 +-- .../config/entities/AllowedMethods.java | 152 ++++++++++++++++++ 2 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index fd61ad95ec..5303e83954 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -52,7 +52,7 @@ public class ActionConfig extends Located implements Serializable { protected String methodName; protected String packageName; protected String name; - protected Set allowedMethods; + protected AllowedMethods allowedMethods; protected ActionConfig(String packageName, String name, String className) { this.packageName = packageName; @@ -62,7 +62,7 @@ protected ActionConfig(String packageName, String name, String className) { results = new LinkedHashMap<>(); interceptors = new ArrayList<>(); exceptionMappings = new ArrayList<>(); - allowedMethods = new HashSet<>(Collections.singletonList(DEFAULT_METHOD)); + allowedMethods = new AllowedMethods(new HashSet<>(Collections.singletonList(DEFAULT_METHOD))); } /** @@ -79,7 +79,7 @@ protected ActionConfig(ActionConfig orig) { this.interceptors = new ArrayList<>(orig.interceptors); this.results = new LinkedHashMap<>(orig.results); this.exceptionMappings = new ArrayList<>(orig.exceptionMappings); - this.allowedMethods = new HashSet<>(orig.allowedMethods); + this.allowedMethods = new AllowedMethods(orig.allowedMethods.list()); this.location = orig.location; } @@ -100,7 +100,7 @@ public List getInterceptors() { } public Set getAllowedMethods() { - return allowedMethods; + return allowedMethods.list(); } /** @@ -128,7 +128,7 @@ public Map getResults() { } public boolean isAllowedMethod(String method) { - return method.equals(methodName != null ? methodName : DEFAULT_METHOD) || allowedMethods.contains(method); + return method.equals(methodName != null ? methodName : DEFAULT_METHOD) || allowedMethods.isAllowed(method); } @Override public boolean equals(Object o) { @@ -210,14 +210,16 @@ public boolean isAllowedMethod(String method) { public static class Builder implements InterceptorListHolder{ protected ActionConfig target; + protected Set allowedMethods; public Builder(ActionConfig toClone) { target = new ActionConfig(toClone); - addAllowedMethod(toClone.getAllowedMethods()); + allowedMethods = toClone.getAllowedMethods(); } public Builder(String packageName, String name, String className) { target = new ActionConfig(packageName, name, className); + allowedMethods = new HashSet<>(); } public Builder packageName(String name) { @@ -308,12 +310,14 @@ public Builder addResultConfigs(Map configs) { } public Builder addAllowedMethod(String methodName) { - target.allowedMethods.add(methodName); + if (methodName != null) { + allowedMethods.add(methodName); + } return this; } public Builder addAllowedMethod(Collection methods) { - target.allowedMethods.addAll(methods); + allowedMethods.addAll(methods); return this; } @@ -327,7 +331,7 @@ public ActionConfig build() { target.results = Collections.unmodifiableMap(target.results); target.interceptors = Collections.unmodifiableList(target.interceptors); target.exceptionMappings = Collections.unmodifiableList(target.exceptionMappings); - target.allowedMethods = Collections.unmodifiableSet(target.allowedMethods); + target.allowedMethods = new AllowedMethods(allowedMethods); ActionConfig result = target; target = new ActionConfig(target); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java new file mode 100644 index 0000000000..7a4fec1b87 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java @@ -0,0 +1,152 @@ +package com.opensymphony.xwork2.config.entities; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import java.util.regex.Pattern; + +public class AllowedMethods { + + private Set allowedMethods; + + public AllowedMethods(Set methods) { + Set allowedMethods = new HashSet<>(); + for (String method : methods) { + allowedMethods.add(build(method)); + } + this.allowedMethods = Collections.unmodifiableSet(allowedMethods); + } + + private AllowedMethod build(String method) { + boolean isPattern = false; + int len = method.length(); + StringBuilder ret = new StringBuilder(); + char c; + for (int x = 0; x < len; x++) { + c = method.charAt(x); + if (x < len - 2 && c == '{' && '}' == method.charAt(x + 2)) { + ret.append("(.*)"); + isPattern = true; + x += 2; + } else { + ret.append(c); + } + } + if (isPattern) { + return new PatternAllowedMethod(ret.toString(), method); + } else { + return new LiteralAllowedMethod(ret.toString()); + } + } + + public boolean isAllowed(String method) { + for (AllowedMethod allowedMethod : allowedMethods) { + if (allowedMethod.isAllowed(method)) { + return true; + } + } + return false; + } + + public Set list() { + Set result = new HashSet<>(); + for (AllowedMethod allowedMethod : allowedMethods) { + result.add(allowedMethod.original()); + } + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + AllowedMethods that = (AllowedMethods) o; + + return allowedMethods.equals(that.allowedMethods); + } + + @Override + public int hashCode() { + return allowedMethods.hashCode(); + } + + private interface AllowedMethod { + boolean isAllowed(String methodName); + + String original(); + } + + private static class PatternAllowedMethod implements AllowedMethod { + + private final Pattern allowedMethodPattern; + private String original; + + public PatternAllowedMethod(String pattern, String original) { + this.original = original; + allowedMethodPattern = Pattern.compile(pattern); + } + + @Override + public boolean isAllowed(String methodName) { + return allowedMethodPattern.matcher(methodName).matches(); + } + + @Override + public String original() { + return original; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + PatternAllowedMethod that = (PatternAllowedMethod) o; + + return original.equals(that.original); + + } + + @Override + public int hashCode() { + return original.hashCode(); + } + } + + private static class LiteralAllowedMethod implements AllowedMethod { + + private String allowedMethod; + + public LiteralAllowedMethod(String allowedMethod) { + this.allowedMethod = allowedMethod; + } + + @Override + public boolean isAllowed(String methodName) { + return methodName.equals(allowedMethod); + } + + @Override + public String original() { + return allowedMethod; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + LiteralAllowedMethod that = (LiteralAllowedMethod) o; + + return allowedMethod.equals(that.allowedMethod); + + } + + @Override + public int hashCode() { + return allowedMethod.hashCode(); + } + } + +} From 185530464b838b3aac9681b5ff5b16401ccef56d Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 18:36:41 +0200 Subject: [PATCH 10/32] Simplifies implementation --- .../providers/XmlConfigurationProvider.java | 16 +++++++++++----- core/src/main/resources/struts-2.5.dtd | 3 --- core/src/main/resources/struts-default.xml | 2 +- core/src/main/resources/xwork-2.5.dtd | 3 --- core/src/test/resources/xwork-sample.xml | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index fd9f92a921..0d64ad71d7 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -893,12 +893,18 @@ protected void loadGlobalResults(PackageConfig.Builder packageContext, Element p } protected void loadGlobalAllowedMethods(PackageConfig.Builder packageContext, Element packageElement) { - NodeList globalAllowedMethods = packageElement.getElementsByTagName("global-allowed-methods"); + NodeList globalAllowedMethodsElms = packageElement.getElementsByTagName("global-allowed-methods"); - if (globalAllowedMethods.getLength() > 0) { - Element globalAllowedMethodsElement = (Element) globalAllowedMethods.item(0); - Set results = TextParseUtil.commaDelimitedStringToSet(globalAllowedMethodsElement.getAttribute("methods")); - packageContext.addGlobalAllowedMethods(results); + if (globalAllowedMethodsElms.getLength() > 0) { + Set globalAllowedMethods = new HashSet<>(); + Node n = globalAllowedMethodsElms.item(0).getFirstChild(); + if (n != null) { + String s = n.getNodeValue().trim(); + if (s.length() > 0) { + globalAllowedMethods = TextParseUtil.commaDelimitedStringToSet(s); + } + } + packageContext.addGlobalAllowedMethods(globalAllowedMethods); } } diff --git a/core/src/main/resources/struts-2.5.dtd b/core/src/main/resources/struts-2.5.dtd index 25ca800278..6591ddbf6a 100644 --- a/core/src/main/resources/struts-2.5.dtd +++ b/core/src/main/resources/struts-2.5.dtd @@ -90,9 +90,6 @@ - diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 9fc1c21068..d7a0ee042f 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -358,7 +358,7 @@ - + execute,input,back,cancel,browse diff --git a/core/src/main/resources/xwork-2.5.dtd b/core/src/main/resources/xwork-2.5.dtd index 4d3e1ca845..197d0c1b57 100644 --- a/core/src/main/resources/xwork-2.5.dtd +++ b/core/src/main/resources/xwork-2.5.dtd @@ -69,9 +69,6 @@ - diff --git a/core/src/test/resources/xwork-sample.xml b/core/src/test/resources/xwork-sample.xml index cd2ddcb9c7..7f5e5475e7 100644 --- a/core/src/test/resources/xwork-sample.xml +++ b/core/src/test/resources/xwork-sample.xml @@ -14,7 +14,7 @@ - + execute,input,back,cancel,browse 17 From 47a01eab10d940fdc134cb666d3d2db0280d8ca8 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 20:28:45 +0200 Subject: [PATCH 11/32] Fixes typo --- .../xwork2/config/providers/XmlConfigurationProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 0d64ad71d7..0302a688ee 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -548,7 +548,7 @@ protected PackageConfig addPackage(Element packageElement) throws ConfigurationE loadGlobalAllowedMethods(newPackage, packageElement); // load the global exception handler list for this package - loadGobalExceptionMappings(newPackage, packageElement); + loadGlobalExceptionMappings(newPackage, packageElement); // get actions NodeList actionList = packageElement.getElementsByTagName("action"); @@ -922,7 +922,7 @@ protected void loadDefaultClassRef(PackageConfig.Builder packageContext, Element * @param packageContext the package context * @param packageElement the given XML element */ - protected void loadGobalExceptionMappings(PackageConfig.Builder packageContext, Element packageElement) { + protected void loadGlobalExceptionMappings(PackageConfig.Builder packageContext, Element packageElement) { NodeList globalExceptionMappingList = packageElement.getElementsByTagName("global-exception-mappings"); if (globalExceptionMappingList.getLength() > 0) { From 63bb6e30e75facf5382608857494cf971f0378dd Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 21:06:06 +0200 Subject: [PATCH 12/32] Adds missing comma --- core/src/main/resources/struts-2.5.dtd | 2 +- core/src/main/resources/struts-default.xml | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/resources/struts-2.5.dtd b/core/src/main/resources/struts-2.5.dtd index 6591ddbf6a..0845eeee99 100644 --- a/core/src/main/resources/struts-2.5.dtd +++ b/core/src/main/resources/struts-2.5.dtd @@ -37,7 +37,7 @@ order CDATA #IMPLIED > - + - execute,input,back,cancel,browse - + + execute,input,back,cancel,browse + From 4c7a7dd6c02457cf006318ed4621b7c432cc478c Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 21:46:16 +0200 Subject: [PATCH 13/32] Adds null-safety --- .../opensymphony/xwork2/config/impl/ActionConfigMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bb97ce8850..7f17e41c06 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 @@ -77,7 +77,7 @@ public ActionConfigMatcher(PatternMatcher patternMatcher, Map vars) { String methodName = convertParam(orig.getMethodName(), vars); - if (!orig.isAllowedMethod(methodName)) { + if (methodName != null && !orig.isAllowedMethod(methodName)) { return null; } From 77691563b9b8d2ad01c078a66d1ed207bf3611b3 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 21:46:47 +0200 Subject: [PATCH 14/32] Implements required method --- .../struts2/convention/ConventionUnknownHandler.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java b/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java index 85ce714094..78c195637d 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java @@ -28,6 +28,7 @@ import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.util.ClassLoaderUtil; +import com.opensymphony.xwork2.util.TextParseUtil; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -67,6 +68,8 @@ public class ConventionUnknownHandler implements UnknownHandler { private ConventionsService conventionsService; private String nameSeparator; + protected Set allowedMethods = new HashSet<>(); + /** * Constructs the unknown handler. * @@ -102,6 +105,8 @@ public ConventionUnknownHandler(Configuration configuration, ObjectFactory objec } this.redirectToSlash = Boolean.parseBoolean(redirectToSlash); + + allowedMethods = TextParseUtil.commaDelimitedStringToSet("execute,input,back,cancel,browse"); } public ActionConfig handleUnknownAction(String namespace, String actionName) @@ -398,4 +403,9 @@ public Resource(String path, String ext) { this.ext = ext; } } + + @Override + public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { + return false; + } } From dd406fbb04e755c0545c318c3ea099674fb78363 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 31 Aug 2015 21:46:55 +0200 Subject: [PATCH 15/32] Fixes test --- .../test/java/org/apache/struts2/TestConfigurationProvider.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java index aee8473fba..0eebd85c0f 100644 --- a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java +++ b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java @@ -127,6 +127,7 @@ public void loadPackages() { .addActionConfig("testActionTagAction", new ActionConfig.Builder("", "", TestAction.class.getName()) .addResultConfig(new ResultConfig.Builder(Action.SUCCESS, TestActionTagResult.class.getName()).build()) .addResultConfig(new ResultConfig.Builder(Action.INPUT, TestActionTagResult.class.getName()).build()) + .addAllowedMethod("input") .build()) .build(); From ab5fb27d368f8857e137bc3a319c21fc85b519ee Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Thu, 3 Sep 2015 13:03:55 +0200 Subject: [PATCH 16/32] Adds annotation to support AllowedMethods --- .../xwork2/config/entities/PackageConfig.java | 13 +++- .../PackageBasedActionConfigBuilder.java | 30 ++++++++-- .../convention/annotation/AllowedMethods.java | 30 ++++++++++ .../PackageBasedActionConfigBuilderTest.java | 60 +++++++++++++++++-- .../ClassLevelAllowedMethodsAction.java | 10 ++++ .../PackageLevelAllowedMethodsAction.java | 7 +++ .../actions/allowedmethods/package-info.java | 23 +++++++ ...PackageLevelAllowedMethodsChildAction.java | 9 +++ 8 files changed, 171 insertions(+), 11 deletions(-) create mode 100644 plugins/convention/src/main/java/org/apache/struts2/convention/annotation/AllowedMethods.java create mode 100644 plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/ClassLevelAllowedMethodsAction.java create mode 100644 plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/PackageLevelAllowedMethodsAction.java create mode 100644 plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/package-info.java create mode 100644 plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/sub/PackageLevelAllowedMethodsChildAction.java diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index 19ce36fc8d..093985c545 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -531,7 +531,18 @@ public Builder addGlobalResultConfigs(Map resultConfigs) { } public Set getGlobalAllowedMethods() { - return target.globalAllowedMethods; + Set allowedMethods = target.globalAllowedMethods; + allowedMethods.addAll(getParentsAllowedMethods(target.parents)); + return allowedMethods; + } + + public Set getParentsAllowedMethods(List parents) { + Set allowedMethods = new HashSet<>(); + for (PackageConfig parent : parents) { + allowedMethods.addAll(parent.globalAllowedMethods); + allowedMethods.addAll(getParentsAllowedMethods(parent.getParents())); + } + return allowedMethods; } public Builder addGlobalAllowedMethods(Set allowedMethods) { diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java index b4a8be9e60..64a0e98ddb 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java @@ -42,6 +42,7 @@ import org.apache.struts2.StrutsConstants; import org.apache.struts2.StrutsException; import org.apache.struts2.convention.annotation.*; +import org.apache.struts2.convention.annotation.AllowedMethods; import java.io.IOException; import java.lang.reflect.Method; @@ -652,6 +653,8 @@ protected void buildConfiguration(Set classes) { String actionPackage = actionClass.getPackage().getName(); LOG.debug("Processing class [{}] in package [{}]", actionClass.getName(), actionPackage); + Set allowedMethods = getAllowedMethods(actionClass); + // Determine the default namespace and action name List namespaces = determineActionNamespace(actionClass); for (String namespace : namespaces) { @@ -692,7 +695,7 @@ protected void buildConfiguration(Set classes) { // Build the default if (!found) { - createActionConfig(defaultPackageConfig, actionClass, defaultActionName, DEFAULT_METHOD, null); + createActionConfig(defaultPackageConfig, actionClass, defaultActionName, DEFAULT_METHOD, null, allowedMethods); } } @@ -706,14 +709,14 @@ protected void buildConfiguration(Set classes) { actionClass, action); } - createActionConfig(pkgCfg, actionClass, defaultActionName, method, action); + createActionConfig(pkgCfg, actionClass, defaultActionName, method, action, allowedMethods); } } // some actions will not have any @Action or a default method, like the rest actions // where the action mapper is the one that finds the right method at runtime if (map.isEmpty() && mapAllMatches && actionAnnotation == null && actionsAnnotation == null) { - createActionConfig(defaultPackageConfig, actionClass, defaultActionName, null, actionAnnotation); + createActionConfig(defaultPackageConfig, actionClass, defaultActionName, null, actionAnnotation, allowedMethods); } //if there are @Actions or @Action at the class level, create the mappings for them @@ -721,9 +724,9 @@ protected void buildConfiguration(Set classes) { if (actionsAnnotation != null) { List actionAnnotations = checkActionsAnnotation(actionsAnnotation); for (Action actionAnnotation2 : actionAnnotations) - createActionConfig(defaultPackageConfig, actionClass, defaultActionName, methodName, actionAnnotation2); + createActionConfig(defaultPackageConfig, actionClass, defaultActionName, methodName, actionAnnotation2, allowedMethods); } else if (actionAnnotation != null) - createActionConfig(defaultPackageConfig, actionClass, defaultActionName, methodName, actionAnnotation); + createActionConfig(defaultPackageConfig, actionClass, defaultActionName, methodName, actionAnnotation, allowedMethods); } } @@ -736,6 +739,15 @@ protected void buildConfiguration(Set classes) { } } + private Set getAllowedMethods(Class actionClass) { + AllowedMethods annotation = AnnotationUtils.findAnnotation(actionClass, AllowedMethods.class); + if (annotation == null) { + return Collections.emptySet(); + } else { + return TextParseUtil.commaDelimitedStringToSet(annotation.value()); + } + } + /** * Interfaces, enums, annotations, and abstract classes cannot be instantiated. * @param actionClass class to check @@ -896,7 +908,7 @@ protected List checkActionsAnnotation(Actions actionsAnnotation) { * @param annotation The ActionName annotation that might override the action name and possibly */ protected void createActionConfig(PackageConfig.Builder pkgCfg, Class actionClass, String actionName, - String actionMethod, Action annotation) { + String actionMethod, Action annotation, Set allowedMethods) { String className = actionClass.getName(); if (annotation != null) { actionName = annotation.value() != null && annotation.value().equals(Action.DEFAULT_VALUE) ? actionName : annotation.value(); @@ -909,6 +921,12 @@ protected void createActionConfig(PackageConfig.Builder pkgCfg, Class actionC ActionConfig.Builder actionConfig = new ActionConfig.Builder(pkgCfg.getName(), actionName, className); actionConfig.methodName(actionMethod); + if (!allowedMethods.contains(actionMethod)) { + actionConfig.addAllowedMethod(actionMethod); + } + actionConfig.addAllowedMethod(allowedMethods); + actionConfig.addAllowedMethod(pkgCfg.getGlobalAllowedMethods()); + if (LOG.isDebugEnabled()) { LOG.debug("Creating action config for class [{}], name [{}] and package name [{}] in namespace [{}]", actionClass.toString(), actionName, pkgCfg.getName(), pkgCfg.getNamespace()); diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/annotation/AllowedMethods.java b/plugins/convention/src/main/java/org/apache/struts2/convention/annotation/AllowedMethods.java new file mode 100644 index 0000000000..8e3c9abcd2 --- /dev/null +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/annotation/AllowedMethods.java @@ -0,0 +1,30 @@ +package org.apache.struts2.convention.annotation; + +import com.opensymphony.xwork2.config.entities.ActionConfig; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * + *

+ * This annotation allows actions to specify allowed action methods + * to limit access to any other public action's methods + *

+ * + *

+ * This annotation can be used directly on Action classes or + * in the package-info.java class in order + * to specify global allowed methods for all sub-packages. + *

+ * + */ +@Target({ElementType.TYPE, ElementType.PACKAGE}) +@Retention(value = RetentionPolicy.RUNTIME) +public @interface AllowedMethods { + + String value() default ActionConfig.DEFAULT_METHOD; + +} diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java index 38403a0354..61ed3eb282 100644 --- a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java @@ -29,6 +29,7 @@ import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Scope.Strategy; import com.opensymphony.xwork2.ognl.OgnlReflectionProvider; +import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.fs.DefaultFileManager; import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory; import com.opensymphony.xwork2.util.reflection.ReflectionException; @@ -37,6 +38,9 @@ import org.apache.struts2.convention.actions.NoAnnotationAction; import org.apache.struts2.convention.actions.Skip; import org.apache.struts2.convention.actions.action.*; +import org.apache.struts2.convention.actions.allowedmethods.ClassLevelAllowedMethodsAction; +import org.apache.struts2.convention.actions.allowedmethods.PackageLevelAllowedMethodsAction; +import org.apache.struts2.convention.actions.allowedmethods.sub.PackageLevelAllowedMethodsChildAction; import org.apache.struts2.convention.actions.chain.ChainedAction; import org.apache.struts2.convention.actions.defaultinterceptor.SingleActionNameAction2; import org.apache.struts2.convention.actions.exception.ExceptionsActionLevelAction; @@ -125,8 +129,10 @@ private void run(String actionPackages, String packageLocators, String excludePa new ResultTypeConfig.Builder("chain", ActionChainResult.class.getName()).defaultResultParam("actionName").build()}; + Set globalAllowedMethods = TextParseUtil.commaDelimitedStringToSet("execute,browse,cancel,input"); + PackageConfig strutsDefault = makePackageConfig("struts-default", null, null, "dispatcher", - defaultResults, defaultInterceptors, defaultInterceptorStacks); + defaultResults, defaultInterceptors, defaultInterceptorStacks, globalAllowedMethods); PackageConfig packageLevelParentPkg = makePackageConfig("package-level", null, null, null); PackageConfig classLevelParentPkg = makePackageConfig("class-level", null, null, null); @@ -151,6 +157,16 @@ private void run(String actionPackages, String packageLocators, String excludePa "/parentpackage", packageLevelParentPkg, null); PackageConfig packageLevelSubPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage.sub#package-level#/parentpackage/sub", "/parentpackage/sub", packageLevelParentPkg, null); + + // Unexpected method call build(class org.apache.struts2.convention.actions.allowedmethods.PackageLevelAllowedMethodsAction, null, "package-level-allowed-methods", PackageConfig: [org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods] for namespace [/allowedmethods] with parents [[PackageConfig: [struts-default] for namespace [] with parents [[]]]]): + PackageConfig packageLevelAllowedMethodsPkg = makePackageConfig("org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods", + "/allowedmethods", strutsDefault, null); + PackageConfig packageLevelAllowedMethodsSubPkg = makePackageConfig("org.apache.struts2.convention.actions.allowedmethods.sub#struts-default#/allowedmethods/sub", + "/allowedmethods/sub", strutsDefault, null); + + PackageConfig classLevelAllowedMethodsPkg = makePackageConfig("org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods", + "/allowedmethods", strutsDefault, null); + PackageConfig differentPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage#class-level#/parentpackage", "/parentpackage", classLevelParentPkg, null); PackageConfig differentSubPkg = makePackageConfig("org.apache.struts2.convention.actions.parentpackage.sub#class-level#/parentpackage/sub", @@ -261,6 +277,11 @@ private void run(String actionPackages, String packageLocators, String excludePa expect(resultMapBuilder.build(ClassLevelParentPackageAction.class, null, "class-level-parent-package", differentPkg)).andReturn(results); expect(resultMapBuilder.build(ClassLevelParentPackageChildAction.class, null, "class-level-parent-package-child", differentSubPkg)).andReturn(results); + /* org.apache.struts2.convention.actions.allowedmethods */ + expect(resultMapBuilder.build(ClassLevelAllowedMethodsAction.class, null, "class-level-allowed-methods", classLevelAllowedMethodsPkg)).andReturn(results); + expect(resultMapBuilder.build(PackageLevelAllowedMethodsAction.class, null, "package-level-allowed-methods", packageLevelAllowedMethodsPkg)).andReturn(results); + expect(resultMapBuilder.build(PackageLevelAllowedMethodsChildAction.class, null, "package-level-allowed-methods-child", packageLevelAllowedMethodsSubPkg)).andReturn(results); + /* org.apache.struts2.convention.actions.result */ expect(resultMapBuilder.build(ClassLevelResultAction.class, null, "class-level-result", resultPkg)).andReturn(results); expect(resultMapBuilder.build(ClassLevelResultsAction.class, null, "class-level-results", resultPkg)).andReturn(results); @@ -450,7 +471,7 @@ public Container getContainer() { verifyActionConfig(pkgConfig, "", org.apache.struts2.convention.actions.idx.Index.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "index", org.apache.struts2.convention.actions.idx.Index.class, "execute", pkgConfig.getName()); verifyActionConfig(pkgConfig, "idx2", org.apache.struts2.convention.actions.idx.idx2.Index.class, "execute", - "org.apache.struts2.convention.actions.idx.idx2#struts-default#/idx/idx2"); + "org.apache.struts2.convention.actions.idx.idx2#struts-default#/idx/idx2"); /* org.apache.struts2.convention.actions.defaultinterceptor */ pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.defaultinterceptor#struts-default#/defaultinterceptor"); @@ -514,6 +535,33 @@ public Container getContainer() { verifyActionConfig(pkgConfig, "package-level-parent-package-child", PackageLevelParentPackageChildAction.class, "execute", pkgConfig.getName()); assertEquals("package-level", pkgConfig.getParents().get(0).getName()); + /* org.apache.struts2.convention.actions.allowedmethods class level */ + pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.allowedmethods#struts-default#/allowedmethods"); + assertNotNull(pkgConfig); + assertEquals(2, pkgConfig.getActionConfigs().size()); + verifyActionConfig(pkgConfig, "class-level-allowed-methods", ClassLevelAllowedMethodsAction.class, "execute", pkgConfig.getName()); + assertEquals("struts-default", pkgConfig.getParents().get(0).getName()); + + ActionConfig actionConfig = pkgConfig.getActionConfigs().get("class-level-allowed-methods"); + assertEquals(actionConfig.getAllowedMethods().size(), 5); + assertTrue(actionConfig.getAllowedMethods().contains("execute")); + assertTrue(actionConfig.getAllowedMethods().contains("end")); + assertTrue(actionConfig.getAllowedMethods().contains("input")); + + /* org.apache.struts2.convention.actions.allowedmethods.sub package level */ + pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.allowedmethods.sub#struts-default#/allowedmethods/sub"); + assertNotNull(pkgConfig); + assertEquals(1, pkgConfig.getActionConfigs().size()); + verifyActionConfig(pkgConfig, "package-level-allowed-methods-child", PackageLevelAllowedMethodsChildAction.class, "execute", pkgConfig.getName()); + assertEquals("struts-default", pkgConfig.getParents().get(0).getName()); + + actionConfig = pkgConfig.getActionConfigs().get("package-level-allowed-methods-child"); + assertEquals(actionConfig.getAllowedMethods().size(), 6); + assertTrue(actionConfig.getAllowedMethods().contains("execute")); + assertTrue(actionConfig.getAllowedMethods().contains("home")); + assertTrue(actionConfig.getAllowedMethods().contains("start")); + assertTrue(actionConfig.getAllowedMethods().contains("input")); + /* org.apache.struts2.convention.actions.result */ pkgConfig = configuration.getPackageConfig("org.apache.struts2.convention.actions.result#struts-default#/result"); assertNotNull(pkgConfig); @@ -631,12 +679,12 @@ private void verifyActionConfigInterceptors(PackageConfig pkgConfig, String acti private PackageConfig makePackageConfig(String name, String namespace, PackageConfig parent, String defaultResultType, ResultTypeConfig... results) { - return makePackageConfig(name, namespace, parent, defaultResultType, results, null, null); + return makePackageConfig(name, namespace, parent, defaultResultType, results, null, null, null); } private PackageConfig makePackageConfig(String name, String namespace, PackageConfig parent, String defaultResultType, ResultTypeConfig[] results, List interceptors, - List interceptorStacks) { + List interceptorStacks, Set globalAllowedMethods) { PackageConfig.Builder builder = new PackageConfig.Builder(name); if (namespace != null) { builder.namespace(namespace); @@ -663,6 +711,10 @@ private PackageConfig makePackageConfig(String name, String namespace, PackageCo } } + if (globalAllowedMethods != null) { + builder.addGlobalAllowedMethods(globalAllowedMethods); + } + return new MyPackageConfig(builder.build()); } diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/ClassLevelAllowedMethodsAction.java b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/ClassLevelAllowedMethodsAction.java new file mode 100644 index 0000000000..24ea48950a --- /dev/null +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/ClassLevelAllowedMethodsAction.java @@ -0,0 +1,10 @@ +package org.apache.struts2.convention.actions.allowedmethods; + +import org.apache.struts2.convention.annotation.AllowedMethods; + +@AllowedMethods("end") +public class ClassLevelAllowedMethodsAction { + + public String execute() { return null; } + +} diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/PackageLevelAllowedMethodsAction.java b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/PackageLevelAllowedMethodsAction.java new file mode 100644 index 0000000000..eab0895071 --- /dev/null +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/PackageLevelAllowedMethodsAction.java @@ -0,0 +1,7 @@ +package org.apache.struts2.convention.actions.allowedmethods; + +public class PackageLevelAllowedMethodsAction { + + public String execute() { return null; } + +} diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/package-info.java b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/package-info.java new file mode 100644 index 0000000000..461131f7c3 --- /dev/null +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/package-info.java @@ -0,0 +1,23 @@ +/* + * $Id$ + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +@org.apache.struts2.convention.annotation.AllowedMethods("home,start") +package org.apache.struts2.convention.actions.allowedmethods; + diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/sub/PackageLevelAllowedMethodsChildAction.java b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/sub/PackageLevelAllowedMethodsChildAction.java new file mode 100644 index 0000000000..4a6223d0d5 --- /dev/null +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/allowedmethods/sub/PackageLevelAllowedMethodsChildAction.java @@ -0,0 +1,9 @@ +package org.apache.struts2.convention.actions.allowedmethods.sub; + +import org.apache.struts2.convention.actions.allowedmethods.PackageLevelAllowedMethodsAction; + +public class PackageLevelAllowedMethodsChildAction extends PackageLevelAllowedMethodsAction { + + public String execute() { return null; } + +} From e4fc8528b874d5d4d454ec8d8976b696a8375f82 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 08:55:14 +0200 Subject: [PATCH 17/32] Puts back option to disable strict DMI --- .../xwork2/config/entities/PackageConfig.java | 10 ++++++++++ .../config/providers/XmlConfigurationProvider.java | 3 +++ core/src/main/resources/struts-2.5.dtd | 1 + core/src/main/resources/xwork-2.5.dtd | 1 + 4 files changed, 15 insertions(+) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index 093985c545..aa53a1267f 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -448,6 +448,7 @@ public Object getInterceptorConfig(String name) { public static class Builder implements InterceptorLocator { protected PackageConfig target; + private boolean strictDMI; public Builder(String name) { target = new PackageConfig(name); @@ -613,6 +614,15 @@ public Object getInterceptorConfig(String name) { return target.getAllInterceptorConfigs().get(name); } + public Builder strictMethodInvocation(boolean strict) { + strictDMI = strict; + return this; + } + + public boolean isStrictMethodInvocation() { + return strictDMI; + } + public PackageConfig build() { embalmTarget(); PackageConfig result = target; diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 0302a688ee..3009dcbca7 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -649,9 +649,12 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { String name = StringUtils.defaultString(packageElement.getAttribute("name")); String namespace = StringUtils.defaultString(packageElement.getAttribute("namespace")); + boolean strictDMI = Boolean.parseBoolean(packageElement.getAttribute("strict-method-invocation")); + PackageConfig.Builder cfg = new PackageConfig.Builder(name) .namespace(namespace) .isAbstract(isAbstract) + .strictMethodInvocation(strictDMI) .location(DomHelper.getLocationObject(packageElement)); if (StringUtils.isNotEmpty(StringUtils.defaultString(parent))) { // has parents, let's look it up diff --git a/core/src/main/resources/struts-2.5.dtd b/core/src/main/resources/struts-2.5.dtd index 0845eeee99..b426d29adc 100644 --- a/core/src/main/resources/struts-2.5.dtd +++ b/core/src/main/resources/struts-2.5.dtd @@ -43,6 +43,7 @@ extends CDATA #IMPLIED namespace CDATA #IMPLIED abstract CDATA #IMPLIED + strict-method-invocation (true|false) "true" > diff --git a/core/src/main/resources/xwork-2.5.dtd b/core/src/main/resources/xwork-2.5.dtd index 197d0c1b57..31c1167c56 100644 --- a/core/src/main/resources/xwork-2.5.dtd +++ b/core/src/main/resources/xwork-2.5.dtd @@ -22,6 +22,7 @@ extends CDATA #IMPLIED namespace CDATA #IMPLIED abstract CDATA #IMPLIED + strict-method-invocation (true|false) "true" > From e38d4657c6a973b30b9ece360f7ea2c97fb69555 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 09:04:37 +0200 Subject: [PATCH 18/32] Adds support to define allowed methods as regex --- .../xwork2/config/entities/ActionConfig.java | 1 + .../config/entities/AllowedMethods.java | 5 +++- .../providers/XmlConfigurationProvider.java | 24 ++++++++++++------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index 5303e83954..7c10c9099e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -43,6 +43,7 @@ public class ActionConfig extends Located implements Serializable { public static final String DEFAULT_METHOD = "execute"; public static final String WILDCARD = "*"; + public static final String REGEX_WILDCARD = "regex:.*"; protected List interceptors; // a list of interceptorMapping Objects eg. List protected Map params; diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java index 7a4fec1b87..f619d890e4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java @@ -32,8 +32,11 @@ private AllowedMethod build(String method) { ret.append(c); } } - if (isPattern) { + if (isPattern && !method.startsWith("regex:")) { return new PatternAllowedMethod(ret.toString(), method); + } else if (method.startsWith("regex:")) { + String pattern = method.substring(method.indexOf(":") + 1); + return new PatternAllowedMethod(pattern, method); } else { return new LiteralAllowedMethod(ret.toString()); } diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 3009dcbca7..950d4a12b5 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -845,17 +845,23 @@ protected List buildExceptionMappings(Element element, P protected Set buildAllowedMethods(Element element, PackageConfig.Builder packageContext) { NodeList allowedMethodsEls = element.getElementsByTagName("allowed-methods"); - Set allowedMethods = packageContext.getGlobalAllowedMethods(); - - if (allowedMethodsEls.getLength() > 0) { - allowedMethods = new HashSet<>(); - Node n = allowedMethodsEls.item(0).getFirstChild(); - if (n != null) { - String s = n.getNodeValue().trim(); - if (s.length() > 0) { - allowedMethods = TextParseUtil.commaDelimitedStringToSet(s); + Set allowedMethods; + if (packageContext.isStrictMethodInvocation()) { + allowedMethods = packageContext.getGlobalAllowedMethods(); + + if (allowedMethodsEls.getLength() > 0) { + allowedMethods = new HashSet<>(); + Node n = allowedMethodsEls.item(0).getFirstChild(); + if (n != null) { + String s = n.getNodeValue().trim(); + if (s.length() > 0) { + allowedMethods = TextParseUtil.commaDelimitedStringToSet(s); + } } } + } else { + allowedMethods = new HashSet<>(); + allowedMethods.add(ActionConfig.REGEX_WILDCARD); } return allowedMethods; From cb06d7d2580e37ab8aba2170c433c1674932c4de Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 09:08:32 +0200 Subject: [PATCH 19/32] Uses factory method pattern --- .../xwork2/config/entities/ActionConfig.java | 6 +-- .../config/entities/AllowedMethods.java | 54 ++++++++++--------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index 7c10c9099e..a821c93a89 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -63,7 +63,7 @@ protected ActionConfig(String packageName, String name, String className) { results = new LinkedHashMap<>(); interceptors = new ArrayList<>(); exceptionMappings = new ArrayList<>(); - allowedMethods = new AllowedMethods(new HashSet<>(Collections.singletonList(DEFAULT_METHOD))); + allowedMethods = AllowedMethods.build(new HashSet<>(Collections.singletonList(DEFAULT_METHOD))); } /** @@ -80,7 +80,7 @@ protected ActionConfig(ActionConfig orig) { this.interceptors = new ArrayList<>(orig.interceptors); this.results = new LinkedHashMap<>(orig.results); this.exceptionMappings = new ArrayList<>(orig.exceptionMappings); - this.allowedMethods = new AllowedMethods(orig.allowedMethods.list()); + this.allowedMethods = AllowedMethods.build(orig.allowedMethods.list()); this.location = orig.location; } @@ -332,7 +332,7 @@ public ActionConfig build() { target.results = Collections.unmodifiableMap(target.results); target.interceptors = Collections.unmodifiableList(target.interceptors); target.exceptionMappings = Collections.unmodifiableList(target.exceptionMappings); - target.allowedMethods = new AllowedMethods(allowedMethods); + target.allowedMethods = AllowedMethods.build(allowedMethods); ActionConfig result = target; target = new ActionConfig(target); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java index f619d890e4..75826966d2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java @@ -9,37 +9,39 @@ public class AllowedMethods { private Set allowedMethods; - public AllowedMethods(Set methods) { + public static AllowedMethods build(Set methods) { + Set allowedMethods = new HashSet<>(); for (String method : methods) { - allowedMethods.add(build(method)); - } - this.allowedMethods = Collections.unmodifiableSet(allowedMethods); - } - - private AllowedMethod build(String method) { - boolean isPattern = false; - int len = method.length(); - StringBuilder ret = new StringBuilder(); - char c; - for (int x = 0; x < len; x++) { - c = method.charAt(x); - if (x < len - 2 && c == '{' && '}' == method.charAt(x + 2)) { - ret.append("(.*)"); - isPattern = true; - x += 2; + boolean isPattern = false; + int len = method.length(); + StringBuilder ret = new StringBuilder(); + char c; + for (int x = 0; x < len; x++) { + c = method.charAt(x); + if (x < len - 2 && c == '{' && '}' == method.charAt(x + 2)) { + ret.append("(.*)"); + isPattern = true; + x += 2; + } else { + ret.append(c); + } + } + if (isPattern && !method.startsWith("regex:")) { + allowedMethods.add(new PatternAllowedMethod(ret.toString(), method)); + } else if (method.startsWith("regex:")) { + String pattern = method.substring(method.indexOf(":") + 1); + allowedMethods.add(new PatternAllowedMethod(pattern, method)); } else { - ret.append(c); + allowedMethods.add(new LiteralAllowedMethod(ret.toString())); } } - if (isPattern && !method.startsWith("regex:")) { - return new PatternAllowedMethod(ret.toString(), method); - } else if (method.startsWith("regex:")) { - String pattern = method.substring(method.indexOf(":") + 1); - return new PatternAllowedMethod(pattern, method); - } else { - return new LiteralAllowedMethod(ret.toString()); - } + + return new AllowedMethods(allowedMethods); + } + + private AllowedMethods(Set methods) { + this.allowedMethods = Collections.unmodifiableSet(methods); } public boolean isAllowed(String method) { From f1dbc5a71914f4159d9aa0f0a8b6c8bfbd37f864 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 09:24:11 +0200 Subject: [PATCH 20/32] Adds toString implementations --- .../xwork2/config/entities/AllowedMethods.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java index 75826966d2..e8796b1d42 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java @@ -117,6 +117,14 @@ public boolean equals(Object o) { public int hashCode() { return original.hashCode(); } + + @Override + public String toString() { + return "PatternAllowedMethod{" + + "allowedMethodPattern=" + allowedMethodPattern + + ", original='" + original + '\'' + + '}'; + } } private static class LiteralAllowedMethod implements AllowedMethod { @@ -152,6 +160,13 @@ public boolean equals(Object o) { public int hashCode() { return allowedMethod.hashCode(); } + + @Override + public String toString() { + return "LiteralAllowedMethod{" + + "allowedMethod='" + allowedMethod + '\'' + + '}'; + } } } From 7db5e76133e22e86e0af0687fe1b3f1b80bfc09a Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 09:24:18 +0200 Subject: [PATCH 21/32] Adds basic test case --- .../config/entities/AllowedMethodsTest.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java diff --git a/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java new file mode 100644 index 0000000000..78f309453c --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java @@ -0,0 +1,55 @@ +package com.opensymphony.xwork2.config.entities; + +import junit.framework.TestCase; + +import java.util.HashSet; +import java.util.Set; + +public class AllowedMethodsTest extends TestCase { + + public void testLiteralMethods() throws Exception { + // given + String method = "myMethod"; + Set literals = new HashSet<>(); + literals.add(method); + + // when + AllowedMethods allowedMethods = AllowedMethods.build(literals); + + // then + assertEquals(1, allowedMethods.list().size()); + assertTrue(allowedMethods.isAllowed(method)); + assertFalse(allowedMethods.isAllowed("someOtherMethod")); + } + + public void testWidlcardMethods() throws Exception { + // given + String method = "my{1}"; + Set literals = new HashSet<>(); + literals.add(method); + + // when + AllowedMethods allowedMethods = AllowedMethods.build(literals); + + // then + assertEquals(1, allowedMethods.list().size()); + assertTrue(allowedMethods.isAllowed("myMethod")); + assertFalse(allowedMethods.isAllowed("someOtherMethod")); + } + + public void testRegexMethods() throws Exception { + // given + String method = "regex:my([a-zA-Z].*)"; + Set literals = new HashSet<>(); + literals.add(method); + + // when + AllowedMethods allowedMethods = AllowedMethods.build(literals); + + // then + assertEquals(1, allowedMethods.list().size()); + assertTrue(allowedMethods.isAllowed("myMethod")); + assertFalse(allowedMethods.isAllowed("someOtherMethod")); + } + +} From 45af87655d1904bc55934512b5f0790dd04afa2d Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 11:35:07 +0200 Subject: [PATCH 22/32] Drops unneeded if clause --- .../struts2/convention/PackageBasedActionConfigBuilder.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java index 64a0e98ddb..0fc238cccf 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java @@ -921,9 +921,7 @@ protected void createActionConfig(PackageConfig.Builder pkgCfg, Class actionC ActionConfig.Builder actionConfig = new ActionConfig.Builder(pkgCfg.getName(), actionName, className); actionConfig.methodName(actionMethod); - if (!allowedMethods.contains(actionMethod)) { - actionConfig.addAllowedMethod(actionMethod); - } + actionConfig.addAllowedMethod(actionMethod); actionConfig.addAllowedMethod(allowedMethods); actionConfig.addAllowedMethod(pkgCfg.getGlobalAllowedMethods()); From 0aaade64cd332a7a5f45ae5068b8f7be1abb3817 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 11:35:23 +0200 Subject: [PATCH 23/32] Adds more strict DMI logic --- .../xwork2/config/providers/XmlConfigurationProvider.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 950d4a12b5..7d2e065d50 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -846,7 +846,8 @@ protected Set buildAllowedMethods(Element element, PackageConfig.Builder NodeList allowedMethodsEls = element.getElementsByTagName("allowed-methods"); Set allowedMethods; - if (packageContext.isStrictMethodInvocation()) { + if (allowedMethodsEls.getLength() > 0) { + // user defined 'allowed-methods' so used them whatever Strict DMI was enabled or not allowedMethods = packageContext.getGlobalAllowedMethods(); if (allowedMethodsEls.getLength() > 0) { @@ -859,7 +860,11 @@ protected Set buildAllowedMethods(Element element, PackageConfig.Builder } } } + } else if (packageContext.isStrictMethodInvocation()) { + // user enabled Strict DMI but didn't defined action specific 'allowed-methods' so we use 'global-allowed-methods' only + allowedMethods = packageContext.getGlobalAllowedMethods(); } else { + // Strict DMI is disabled to any method can be called allowedMethods = new HashSet<>(); allowedMethods.add(ActionConfig.REGEX_WILDCARD); } From 3ccd7307f9af04d1f45a9ddd2360c390d83e13b0 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 11:35:31 +0200 Subject: [PATCH 24/32] Adjusts tests --- .../XmlConfigurationProviderActionsTest.java | 16 ++++++++++++++-- ...lConfigurationProviderAllowedMethodsTest.java | 10 +++++----- ...nfigurationProviderExceptionMappingsTest.java | 7 ++++--- ...XmlConfigurationProviderInterceptorsTest.java | 6 ++++++ .../XmlConfigurationProviderResultsTest.java | 1 + .../providers/xwork-test-allowed-methods.xml | 4 ++-- 6 files changed, 32 insertions(+), 12 deletions(-) diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java index 2181181633..cde4c6272d 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java @@ -56,6 +56,7 @@ public void testActions() throws Exception { ActionConfig barAction = new ActionConfig.Builder("", "Bar", SimpleAction.class.getName()) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .addParams(params).build(); // foo action is a little more complex, two params, a result and an interceptor stack @@ -72,6 +73,7 @@ public void testActions() throws Exception { .addParams(params) .addResultConfigs(results) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // wildcard action is simple wildcard example @@ -81,6 +83,7 @@ public void testActions() throws Exception { ActionConfig wildcardAction = new ActionConfig.Builder("", "WildCard", SimpleAction.class.getName()) .addResultConfigs(results) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // fooBar action is a little more complex, two params, a result and an interceptor stack @@ -99,6 +102,7 @@ public void testActions() throws Exception { .addResultConfigs(results) .addInterceptors(interceptors) .addExceptionMappings(exceptionMappings) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParam action tests that an interceptor worked @@ -112,6 +116,7 @@ public void testActions() throws Exception { ActionConfig intAction = new ActionConfig.Builder("", "TestInterceptorParam", SimpleAction.class.getName()) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParamOverride action tests that an interceptor with a param override worked @@ -123,6 +128,7 @@ public void testActions() throws Exception { ActionConfig intOverAction = new ActionConfig.Builder("", "TestInterceptorParamOverride", SimpleAction.class.getName()) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration @@ -164,7 +170,10 @@ public void testPackageDefaultClassRef() throws Exception { params.put("bar", "23"); ActionConfig barWithPackageDefaultClassRefConfig = - new ActionConfig.Builder("", "Bar", "").addParams(params).build(); + new ActionConfig.Builder("", "Bar", "") + .addParams(params) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) + .build(); // execute the configuration provider.init(configuration); @@ -188,7 +197,10 @@ public void testDefaultActionClass() throws Exception { params.put("bar", "23"); ActionConfig barWithoutClassNameConfig = - new ActionConfig.Builder("", "BarWithoutClassName", "").addParams(params).build(); + new ActionConfig.Builder("", "BarWithoutClassName", "") + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) + .addParams(params) + .build(); // execute the configuration provider.init(configuration); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java index 11f779ac6b..4b4460ae7e 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderAllowedMethodsTest.java @@ -27,12 +27,12 @@ public void testDefaultAllowedMethods() throws ConfigurationException { assertEquals(5, actionConfigs.size()); ActionConfig action = (ActionConfig) actionConfigs.get("Default"); - assertEquals(0, action.getAllowedMethods().size()); + assertEquals(1, action.getAllowedMethods().size()); assertTrue(action.isAllowedMethod("execute")); - assertFalse(action.isAllowedMethod("foo")); - assertFalse(action.isAllowedMethod("bar")); - assertFalse(action.isAllowedMethod("baz")); - assertFalse(action.isAllowedMethod("xyz")); + assertTrue(action.isAllowedMethod("foo")); + assertTrue(action.isAllowedMethod("bar")); + assertTrue(action.isAllowedMethod("baz")); + assertTrue(action.isAllowedMethod("xyz")); action = (ActionConfig) actionConfigs.get("Boring"); assertEquals(0, action.getAllowedMethods().size()); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java index 232efd3db6..d02a312eb4 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java @@ -32,20 +32,21 @@ public void testActions() throws ConfigurationException { exceptionMappings.add( new ExceptionMappingConfig.Builder("spooky-result", "com.opensymphony.xwork2.SpookyException", "spooky-result") - .build()); + .build()); results.put("spooky-result", new ResultConfig.Builder("spooky-result", MockResult.class.getName()).build()); Map resultParams = new HashMap<>(); resultParams.put("actionName", "bar.vm"); results.put("specificLocationResult", new ResultConfig.Builder("specificLocationResult", ActionChainResult.class.getName()) - .addParams(resultParams) - .build()); + .addParams(resultParams) + .build()); ActionConfig expectedAction = new ActionConfig.Builder("default", "Bar", SimpleAction.class.getName()) .addParams(parameters) .addResultConfigs(results) .addExceptionMappings(exceptionMappings) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java index 4f26b5f8ff..b10eb49516 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java @@ -108,16 +108,19 @@ public void testInterceptorDefaultRefs() throws ConfigurationException { ActionConfig actionWithOwnRef = new ActionConfig.Builder("", "ActionWithOwnRef", SimpleAction.class.getName()) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); ActionConfig actionWithDefaultRef = new ActionConfig.Builder("", "ActionWithDefaultRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // sub package // this should inherit ActionConfig actionWithNoRef = new ActionConfig.Builder("", "ActionWithNoRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); interceptors = new ArrayList<>(); @@ -125,6 +128,7 @@ public void testInterceptorDefaultRefs() throws ConfigurationException { ActionConfig anotherActionWithOwnRef = new ActionConfig.Builder("", "AnotherActionWithOwnRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("logging", objectFactory.buildInterceptor(loggingInterceptor, new HashMap()))) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); RuntimeConfiguration runtimeConfig = configurationManager.getConfiguration().getRuntimeConfiguration(); @@ -184,6 +188,7 @@ public void testInterceptorParamOverriding() throws Exception { ActionConfig intAction = new ActionConfig.Builder("", "TestInterceptorParam", SimpleAction.class.getName()) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParamOverride action tests that an interceptor with a param override worked @@ -202,6 +207,7 @@ public void testInterceptorParamOverriding() throws Exception { ActionConfig intOverAction = new ActionConfig.Builder("", "TestInterceptorParamOverride", SimpleAction.class.getName()) .addInterceptors(interceptors) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); ConfigurationProvider provider = buildConfigurationProvider("com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml"); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java index caa9da0d62..d096c14dc7 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java @@ -67,6 +67,7 @@ public void testActions() throws ConfigurationException { ActionConfig expectedAction = new ActionConfig.Builder("default", "Bar", SimpleAction.class.getName()) .addParams(parameters) .addResultConfigs(results) + .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml index 4059d6252b..6741e668cf 100644 --- a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-allowed-methods.xml @@ -4,7 +4,7 @@ > - + @@ -25,7 +25,7 @@ - + From e9e6ae90e6d7efc931b1738c55fa052045b7bef5 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 4 Sep 2015 11:44:44 +0200 Subject: [PATCH 25/32] Enables Strict DMI by default --- core/src/main/resources/struts-default.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index f1684e5111..2dd6b756d3 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -165,7 +165,7 @@ - + From a61a3692241d0da833139b9e4eab10a4a3e496aa Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 6 Sep 2015 10:36:51 +0200 Subject: [PATCH 26/32] Drops support for handleUnknownAllowedMethod as it was added to satisfy test --- .../com/opensymphony/xwork2/DefaultActionProxy.java | 4 +--- .../xwork2/DefaultUnknownHandlerManager.java | 9 --------- .../java/com/opensymphony/xwork2/UnknownHandler.java | 10 ---------- .../com/opensymphony/xwork2/UnknownHandlerManager.java | 2 -- .../com/opensymphony/xwork2/ActionInvocationTest.java | 7 +++---- .../xwork2/config/providers/SomeUnknownHandler.java | 5 ----- .../struts2/convention/ConventionUnknownHandler.java | 4 ---- 7 files changed, 4 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java index 6b8749ebcd..72c6e9e025 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultActionProxy.java @@ -194,9 +194,7 @@ protected void prepare() { resolveMethod(); - if (config.isAllowedMethod(method) || - (unknownHandlerManager.hasUnknownHandlers() && unknownHandlerManager.isAllowedMethod(method, config)) - ) { + if (config.isAllowedMethod(method)) { invocation.init(this); } else { throw new ConfigurationException("This method: " + method + " for action " + actionName + " is not allowed!"); diff --git a/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java b/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java index 04ac430736..46af876629 100644 --- a/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/DefaultUnknownHandlerManager.java @@ -146,13 +146,4 @@ public List getUnknownHandlers() { return unknownHandlers; } - @Override - public boolean isAllowedMethod(String allowedMethod, ActionConfig config) { - for (UnknownHandler unknownHandler : unknownHandlers) { - if (unknownHandler.isAllowedMethod(allowedMethod, config)) { - return true; - } - } - return false; - } } diff --git a/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java b/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java index 1e6f5aa556..51556d6a44 100644 --- a/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java +++ b/core/src/main/java/com/opensymphony/xwork2/UnknownHandler.java @@ -60,14 +60,4 @@ public interface UnknownHandler { */ Object handleUnknownActionMethod(Object action, String methodName); - /** - * Handles the case when an action method is unknown and cannot be validated if it is allowed to be executed. - * - * @since 2.5 - * @param allowedMethod The method's name - * @param actionConfig The config of actions - * @return true if allowedMethod can be executed, false otherwise - */ - boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig); - } diff --git a/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java b/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java index caf63b33dc..d8cce9f78b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/UnknownHandlerManager.java @@ -44,6 +44,4 @@ public interface UnknownHandlerManager { List getUnknownHandlers(); - boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig); - } diff --git a/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java b/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java index 37aa2a534d..a1182ccfd5 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ActionInvocationTest.java @@ -48,7 +48,9 @@ public void testCommandInvocationUnknownHandler() throws Exception { UnknownHandler unknownHandler = new UnknownHandler() { public ActionConfig handleUnknownAction(String namespace, String actionName) throws XWorkException { - return new ActionConfig.Builder("test", actionName, ActionSupport.class.getName()).build(); + return new ActionConfig.Builder("test", actionName, ActionSupport.class.getName()) + .addAllowedMethod("unknownmethod") + .build(); } public Result handleUnknownResult(ActionContext actionContext, String actionName, ActionConfig actionConfig, String resultCode) throws XWorkException { return new MockResult(); @@ -60,9 +62,6 @@ public Object handleUnknownActionMethod(Object action, String methodName) { return null; } } - public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { - return "unknownmethod".equals(allowedMethod); - } }; UnknownHandlerManagerMock uhm = new UnknownHandlerManagerMock(); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java index 5e922e0fc2..312f4c9a48 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/SomeUnknownHandler.java @@ -38,11 +38,6 @@ public Result handleUnknownResult(ActionContext actionContext, String actionName return null; } - @Override - public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { - return false; - } - public void setActionConfig(ActionConfig actionConfig) { this.actionConfig = actionConfig; } diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java b/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java index 78c195637d..b7c7acff7f 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/ConventionUnknownHandler.java @@ -404,8 +404,4 @@ public Resource(String path, String ext) { } } - @Override - public boolean isAllowedMethod(String allowedMethod, ActionConfig actionConfig) { - return false; - } } From 294076145f153e291eb7c38bb58a9b04bf3fb0e5 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 8 Sep 2015 08:24:28 +0200 Subject: [PATCH 27/32] Inherits parents' strict DMI --- .../xwork2/config/entities/PackageConfig.java | 102 ++++++++---------- ...ConfigurationProviderInterceptorsTest.java | 2 - 2 files changed, 46 insertions(+), 58 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index aa53a1267f..10286c42b4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -53,6 +53,7 @@ public class PackageConfig extends Located implements Comparable, Serializable, protected String namespace = ""; protected boolean isAbstract = false; protected boolean needsRefresh; + protected boolean strictMethodInvocation = true; protected PackageConfig(String name) { this.name = name; @@ -82,6 +83,7 @@ protected PackageConfig(PackageConfig orig) { this.globalExceptionMappingConfigs = new ArrayList<>(orig.globalExceptionMappingConfigs); this.parents = new ArrayList<>(orig.parents); this.location = orig.location; + this.strictMethodInvocation = orig.strictMethodInvocation; } public boolean isAbstract() { @@ -330,7 +332,6 @@ public Map getResultTypeConfigs() { return resultTypeConfigs; } - public boolean isNeedsRefresh() { return needsRefresh; } @@ -345,80 +346,64 @@ public List getGlobalExceptionMappingConfigs() { return globalExceptionMappingConfigs; } + public boolean isStrictMethodInvocation() { + return strictMethodInvocation; + } + @Override public boolean equals(Object o) { - if (this == o) { - return true; - } - - if (!(o instanceof PackageConfig)) { - return false; - } - - final PackageConfig packageConfig = (PackageConfig) o; + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; - if (isAbstract != packageConfig.isAbstract) { - return false; - } + PackageConfig that = (PackageConfig) o; - if ((actionConfigs != null) ? (!actionConfigs.equals(packageConfig.actionConfigs)) : (packageConfig.actionConfigs != null)) { + if (isAbstract != that.isAbstract) return false; + if (needsRefresh != that.needsRefresh) return false; + if (strictMethodInvocation != that.strictMethodInvocation) return false; + if (actionConfigs != null ? !actionConfigs.equals(that.actionConfigs) : that.actionConfigs != null) return false; - } - - if ((defaultResultType != null) ? (!defaultResultType.equals(packageConfig.defaultResultType)) : (packageConfig.defaultResultType != null)) { + if (globalResultConfigs != null ? !globalResultConfigs.equals(that.globalResultConfigs) : that.globalResultConfigs != null) return false; - } - - if ((defaultClassRef != null) ? (!defaultClassRef.equals(packageConfig.defaultClassRef)) : (packageConfig.defaultClassRef != null)) { + if (globalAllowedMethods != null ? !globalAllowedMethods.equals(that.globalAllowedMethods) : that.globalAllowedMethods != null) return false; - } - - if ((globalResultConfigs != null) ? (!globalResultConfigs.equals(packageConfig.globalResultConfigs)) : (packageConfig.globalResultConfigs != null)) { + if (interceptorConfigs != null ? !interceptorConfigs.equals(that.interceptorConfigs) : that.interceptorConfigs != null) return false; - } - - if ((interceptorConfigs != null) ? (!interceptorConfigs.equals(packageConfig.interceptorConfigs)) : (packageConfig.interceptorConfigs != null)) { + if (resultTypeConfigs != null ? !resultTypeConfigs.equals(that.resultTypeConfigs) : that.resultTypeConfigs != null) return false; - } - - if ((name != null) ? (!name.equals(packageConfig.name)) : (packageConfig.name != null)) { + if (globalExceptionMappingConfigs != null ? !globalExceptionMappingConfigs.equals(that.globalExceptionMappingConfigs) : that.globalExceptionMappingConfigs != null) return false; - } - - if ((namespace != null) ? (!namespace.equals(packageConfig.namespace)) : (packageConfig.namespace != null)) { + if (parents != null ? !parents.equals(that.parents) : that.parents != null) return false; + if (defaultInterceptorRef != null ? !defaultInterceptorRef.equals(that.defaultInterceptorRef) : that.defaultInterceptorRef != null) return false; - } - - if ((parents != null) ? (!parents.equals(packageConfig.parents)) : (packageConfig.parents != null)) { + if (defaultActionRef != null ? !defaultActionRef.equals(that.defaultActionRef) : that.defaultActionRef != null) return false; - } - - if ((resultTypeConfigs != null) ? (!resultTypeConfigs.equals(packageConfig.resultTypeConfigs)) : (packageConfig.resultTypeConfigs != null)) { + if (defaultResultType != null ? !defaultResultType.equals(that.defaultResultType) : that.defaultResultType != null) return false; - } - - if ((globalExceptionMappingConfigs != null) ? (!globalExceptionMappingConfigs.equals(packageConfig.globalExceptionMappingConfigs)) : (packageConfig.globalExceptionMappingConfigs != null)) { + if (defaultClassRef != null ? !defaultClassRef.equals(that.defaultClassRef) : that.defaultClassRef != null) return false; - } + if (!name.equals(that.name)) return false; + return !(namespace != null ? !namespace.equals(that.namespace) : that.namespace != null); - return true; } @Override public int hashCode() { - int result; - result = ((name != null) ? name.hashCode() : 0); - result = (29 * result) + ((parents != null) ? parents.hashCode() : 0); - result = (29 * result) + ((actionConfigs != null) ? actionConfigs.hashCode() : 0); - result = (29 * result) + ((globalResultConfigs != null) ? globalResultConfigs.hashCode() : 0); - result = (29 * result) + ((interceptorConfigs != null) ? interceptorConfigs.hashCode() : 0); - result = (29 * result) + ((resultTypeConfigs != null) ? resultTypeConfigs.hashCode() : 0); - result = (29 * result) + ((globalExceptionMappingConfigs != null) ? globalExceptionMappingConfigs.hashCode() : 0); - result = (29 * result) + ((defaultResultType != null) ? defaultResultType.hashCode() : 0); - result = (29 * result) + ((defaultClassRef != null) ? defaultClassRef.hashCode() : 0); - result = (29 * result) + ((namespace != null) ? namespace.hashCode() : 0); - result = (29 * result) + (isAbstract ? 1 : 0); - + int result = actionConfigs != null ? actionConfigs.hashCode() : 0; + result = 31 * result + (globalResultConfigs != null ? globalResultConfigs.hashCode() : 0); + result = 31 * result + (globalAllowedMethods != null ? globalAllowedMethods.hashCode() : 0); + result = 31 * result + (interceptorConfigs != null ? interceptorConfigs.hashCode() : 0); + result = 31 * result + (resultTypeConfigs != null ? resultTypeConfigs.hashCode() : 0); + result = 31 * result + (globalExceptionMappingConfigs != null ? globalExceptionMappingConfigs.hashCode() : 0); + result = 31 * result + (parents != null ? parents.hashCode() : 0); + result = 31 * result + (defaultInterceptorRef != null ? defaultInterceptorRef.hashCode() : 0); + result = 31 * result + (defaultActionRef != null ? defaultActionRef.hashCode() : 0); + result = 31 * result + (defaultResultType != null ? defaultResultType.hashCode() : 0); + result = 31 * result + (defaultClassRef != null ? defaultClassRef.hashCode() : 0); + result = 31 * result + name.hashCode(); + result = 31 * result + (namespace != null ? namespace.hashCode() : 0); + result = 31 * result + (isAbstract ? 1 : 0); + result = 31 * result + (needsRefresh ? 1 : 0); + result = 31 * result + (strictMethodInvocation ? 1 : 0); return result; } @@ -620,6 +605,11 @@ public Builder strictMethodInvocation(boolean strict) { } public boolean isStrictMethodInvocation() { + for (PackageConfig parent : target.parents) { + if (parent.isStrictMethodInvocation()) { + return true; + } + } return strictDMI; } diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java index b10eb49516..cfd7f60757 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java @@ -120,7 +120,6 @@ public void testInterceptorDefaultRefs() throws ConfigurationException { // this should inherit ActionConfig actionWithNoRef = new ActionConfig.Builder("", "ActionWithNoRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); interceptors = new ArrayList<>(); @@ -128,7 +127,6 @@ public void testInterceptorDefaultRefs() throws ConfigurationException { ActionConfig anotherActionWithOwnRef = new ActionConfig.Builder("", "AnotherActionWithOwnRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("logging", objectFactory.buildInterceptor(loggingInterceptor, new HashMap()))) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); RuntimeConfiguration runtimeConfig = configurationManager.getConfiguration().getRuntimeConfiguration(); From d511d282045fa30aee3f994c7fdac78c600cc417 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 8 Sep 2015 08:31:40 +0200 Subject: [PATCH 28/32] Inlines and simplifies --- .../xwork2/config/entities/PackageConfig.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index 10286c42b4..e5d3ef446a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -614,19 +614,15 @@ public boolean isStrictMethodInvocation() { } public PackageConfig build() { - embalmTarget(); - PackageConfig result = target; - target = new PackageConfig(result); - return result; - } - - protected void embalmTarget() { target.actionConfigs = Collections.unmodifiableMap(target.actionConfigs); target.globalResultConfigs = Collections.unmodifiableMap(target.globalResultConfigs); target.interceptorConfigs = Collections.unmodifiableMap(target.interceptorConfigs); target.resultTypeConfigs = Collections.unmodifiableMap(target.resultTypeConfigs); target.globalExceptionMappingConfigs = Collections.unmodifiableList(target.globalExceptionMappingConfigs); target.parents = Collections.unmodifiableList(target.parents); + PackageConfig result = target; + target = new PackageConfig(result); + return result; } @Override From e3743111ffe13eec93232e5e71a56e5ac1fca0a5 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Tue, 15 Sep 2015 09:02:38 +0200 Subject: [PATCH 29/32] Includes additional global allowed methods --- core/src/main/resources/struts-default.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 2dd6b756d3..b95a1a0aaa 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -362,7 +362,7 @@ - execute,input,back,cancel,browse + execute,input,back,cancel,browse,save,delete,list,index From b8381057190c3abebbc3a65c404fd5de5f2c6c52 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 23 Sep 2015 11:30:54 +0200 Subject: [PATCH 30/32] Respects parent package flag when Strict DMI is disabled --- .../convention/PackageBasedActionConfigBuilder.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java index 0fc238cccf..c388ee96bf 100644 --- a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java +++ b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java @@ -921,9 +921,13 @@ protected void createActionConfig(PackageConfig.Builder pkgCfg, Class actionC ActionConfig.Builder actionConfig = new ActionConfig.Builder(pkgCfg.getName(), actionName, className); actionConfig.methodName(actionMethod); - actionConfig.addAllowedMethod(actionMethod); - actionConfig.addAllowedMethod(allowedMethods); - actionConfig.addAllowedMethod(pkgCfg.getGlobalAllowedMethods()); + if (pkgCfg.isStrictMethodInvocation()) { + actionConfig.addAllowedMethod(actionMethod); + actionConfig.addAllowedMethod(allowedMethods); + actionConfig.addAllowedMethod(pkgCfg.getGlobalAllowedMethods()); + } else { + actionConfig.addAllowedMethod(ActionConfig.REGEX_WILDCARD); + } if (LOG.isDebugEnabled()) { LOG.debug("Creating action config for class [{}], name [{}] and package name [{}] in namespace [{}]", From 86afcbe611f7c3afda26e396cc4504d3c9998398 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 25 Sep 2015 10:42:43 +0200 Subject: [PATCH 31/32] Switches Strict DMI to true by default and allows to be disabled in user's package --- .../xwork2/config/entities/PackageConfig.java | 9 +++++++-- .../config/providers/XmlConfigurationProvider.java | 6 +++++- .../providers/XmlConfigurationProviderActionsTest.java | 8 -------- .../XmlConfigurationProviderExceptionMappingsTest.java | 1 - .../XmlConfigurationProviderInterceptorsTest.java | 4 ---- .../providers/XmlConfigurationProviderResultsTest.java | 1 - 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index e5d3ef446a..e297b49f6d 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -433,7 +433,7 @@ public Object getInterceptorConfig(String name) { public static class Builder implements InterceptorLocator { protected PackageConfig target; - private boolean strictDMI; + private boolean strictDMI = true; public Builder(String name) { target = new PackageConfig(name); @@ -605,12 +605,17 @@ public Builder strictMethodInvocation(boolean strict) { } public boolean isStrictMethodInvocation() { + // if Strict DMI was disabled in this package, + // return without evaluating parent packages + if (!strictDMI) { + return false; + } for (PackageConfig parent : target.parents) { if (parent.isStrictMethodInvocation()) { return true; } } - return strictDMI; + return true; } public PackageConfig build() { diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 7d2e065d50..35e20196e2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -649,7 +649,11 @@ protected PackageConfig.Builder buildPackageContext(Element packageElement) { String name = StringUtils.defaultString(packageElement.getAttribute("name")); String namespace = StringUtils.defaultString(packageElement.getAttribute("namespace")); - boolean strictDMI = Boolean.parseBoolean(packageElement.getAttribute("strict-method-invocation")); + // Strict DMI is enabled by default, it can disabled by user + boolean strictDMI = true; + if (packageElement.hasAttribute("strict-method-invocation")) { + strictDMI = Boolean.parseBoolean(packageElement.getAttribute("strict-method-invocation")); + } PackageConfig.Builder cfg = new PackageConfig.Builder(name) .namespace(namespace) diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java index cde4c6272d..bd23376ad5 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderActionsTest.java @@ -56,7 +56,6 @@ public void testActions() throws Exception { ActionConfig barAction = new ActionConfig.Builder("", "Bar", SimpleAction.class.getName()) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .addParams(params).build(); // foo action is a little more complex, two params, a result and an interceptor stack @@ -73,7 +72,6 @@ public void testActions() throws Exception { .addParams(params) .addResultConfigs(results) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // wildcard action is simple wildcard example @@ -83,7 +81,6 @@ public void testActions() throws Exception { ActionConfig wildcardAction = new ActionConfig.Builder("", "WildCard", SimpleAction.class.getName()) .addResultConfigs(results) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // fooBar action is a little more complex, two params, a result and an interceptor stack @@ -102,7 +99,6 @@ public void testActions() throws Exception { .addResultConfigs(results) .addInterceptors(interceptors) .addExceptionMappings(exceptionMappings) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParam action tests that an interceptor worked @@ -116,7 +112,6 @@ public void testActions() throws Exception { ActionConfig intAction = new ActionConfig.Builder("", "TestInterceptorParam", SimpleAction.class.getName()) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParamOverride action tests that an interceptor with a param override worked @@ -128,7 +123,6 @@ public void testActions() throws Exception { ActionConfig intOverAction = new ActionConfig.Builder("", "TestInterceptorParamOverride", SimpleAction.class.getName()) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration @@ -172,7 +166,6 @@ public void testPackageDefaultClassRef() throws Exception { ActionConfig barWithPackageDefaultClassRefConfig = new ActionConfig.Builder("", "Bar", "") .addParams(params) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration @@ -198,7 +191,6 @@ public void testDefaultActionClass() throws Exception { ActionConfig barWithoutClassNameConfig = new ActionConfig.Builder("", "BarWithoutClassName", "") - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .addParams(params) .build(); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java index d02a312eb4..14e7383689 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderExceptionMappingsTest.java @@ -46,7 +46,6 @@ public void testActions() throws ConfigurationException { .addParams(parameters) .addResultConfigs(results) .addExceptionMappings(exceptionMappings) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java index cfd7f60757..4f26b5f8ff 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderInterceptorsTest.java @@ -108,12 +108,10 @@ public void testInterceptorDefaultRefs() throws ConfigurationException { ActionConfig actionWithOwnRef = new ActionConfig.Builder("", "ActionWithOwnRef", SimpleAction.class.getName()) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); ActionConfig actionWithDefaultRef = new ActionConfig.Builder("", "ActionWithDefaultRef", SimpleAction.class.getName()) .addInterceptor(new InterceptorMapping("timer", objectFactory.buildInterceptor(timerInterceptor, new HashMap()))) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // sub package @@ -186,7 +184,6 @@ public void testInterceptorParamOverriding() throws Exception { ActionConfig intAction = new ActionConfig.Builder("", "TestInterceptorParam", SimpleAction.class.getName()) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // TestInterceptorParamOverride action tests that an interceptor with a param override worked @@ -205,7 +202,6 @@ public void testInterceptorParamOverriding() throws Exception { ActionConfig intOverAction = new ActionConfig.Builder("", "TestInterceptorParamOverride", SimpleAction.class.getName()) .addInterceptors(interceptors) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); ConfigurationProvider provider = buildConfigurationProvider("com/opensymphony/xwork2/config/providers/xwork-test-interceptor-params.xml"); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java index d096c14dc7..caa9da0d62 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderResultsTest.java @@ -67,7 +67,6 @@ public void testActions() throws ConfigurationException { ActionConfig expectedAction = new ActionConfig.Builder("default", "Bar", SimpleAction.class.getName()) .addParams(parameters) .addResultConfigs(results) - .addAllowedMethod(ActionConfig.REGEX_WILDCARD) .build(); // execute the configuration From fdb6daec7859e0cf95a0da5482991b8342cf91ad Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 26 Sep 2015 08:57:44 +0200 Subject: [PATCH 32/32] Simplifies checking of Strict DMI to simple getter --- .../xwork2/config/entities/PackageConfig.java | 14 +---- .../config/entities/PackageConfigTest.java | 62 ++++++++++++++++++- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java index e297b49f6d..1e7a4dd785 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/PackageConfig.java @@ -600,22 +600,12 @@ public Object getInterceptorConfig(String name) { } public Builder strictMethodInvocation(boolean strict) { - strictDMI = strict; + target.strictMethodInvocation = strict; return this; } public boolean isStrictMethodInvocation() { - // if Strict DMI was disabled in this package, - // return without evaluating parent packages - if (!strictDMI) { - return false; - } - for (PackageConfig parent : target.parents) { - if (parent.isStrictMethodInvocation()) { - return true; - } - } - return true; + return target.strictMethodInvocation; } public PackageConfig build() { diff --git a/core/src/test/java/com/opensymphony/xwork2/config/entities/PackageConfigTest.java b/core/src/test/java/com/opensymphony/xwork2/config/entities/PackageConfigTest.java index e2f2868c28..4a25ccba09 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/entities/PackageConfigTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/entities/PackageConfigTest.java @@ -30,5 +30,63 @@ public void testFullDefaultInterceptorRef() { assertEquals("ref2", cfg.getFullDefaultInterceptorRef()); } - -} \ No newline at end of file + + public void testStrictDMIInheritance() { + // given + PackageConfig parent = new PackageConfig.Builder("parent").build(); + + // when + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .build(); + + // then + assertTrue(child.isStrictMethodInvocation()); + } + + public void testStrictDMIInheritanceDisabledInParentPackage() { + // given + PackageConfig parent = new PackageConfig.Builder("parent") + .strictMethodInvocation(false) + .build(); + + // when + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .build(); + + // then + assertTrue(child.isStrictMethodInvocation()); + } + + public void testStrictDMIInheritanceDisabledInBothPackage() { + // given + PackageConfig parent = new PackageConfig.Builder("parent") + .strictMethodInvocation(false) + .build(); + + // when + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .strictMethodInvocation(false) + .build(); + + // then + assertFalse(child.isStrictMethodInvocation()); + } + + public void testStrictDMIInheritanceDisabledInChildPackage() { + // given + PackageConfig parent = new PackageConfig.Builder("parent").build(); + + // when + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .strictMethodInvocation(false) + .build(); + + // then + assertFalse(child.isStrictMethodInvocation()); + } + +}