diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 3423ec8bd..918f91bc1 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -281,4 +281,20 @@ public final class StrutsConstants { /** Allows override default DispatcherErrorHandler **/ public static final String STRUTS_DISPATCHER_ERROR_HANDLER = "struts.dispatcher.errorHandler"; + + /** Comma delimited set of excluded classes and package names which cannot be accessed via expressions **/ + public static final String STRUTS_EXCLUDED_CLASSES = "struts.excludedClasses"; + public static final String STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS = "struts.excludedPackageNamePatterns"; + + /** Dedicated services to check if passed string is excluded/accepted **/ + public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER = "struts.excludedPatterns.checker"; + public static final String STRUTS_ACCEPTED_PATTERNS_CHECKER = "struts.acceptedPatterns.checker"; + + /** Constant is used to override framework's default excluded patterns **/ + public static final String STRUTS_OVERRIDE_EXCLUDED_PATTERNS = "struts.override.excludedPatterns"; + public static final String STRUTS_OVERRIDE_ACCEPTED_PATTERNS = "struts.override.acceptedPatterns"; + + public static final String STRUTS_ADDITIONAL_EXCLUDED_PATTERNS = "struts.additional.excludedPatterns"; + public static final String STRUTS_ADDITIONAL_ACCEPTED_PATTERNS = "struts.additional.acceptedPatterns"; + } diff --git a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java index b6b5b4590..06b730290 100644 --- a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java @@ -22,6 +22,8 @@ package org.apache.struts2.config; import com.opensymphony.xwork2.ActionProxyFactory; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.FileManager; import com.opensymphony.xwork2.FileManagerFactory; import com.opensymphony.xwork2.LocaleProvider; @@ -312,6 +314,12 @@ import java.util.StringTokenizer; * Used to parse expressions like ${foo.bar} or %{bar.foo} but it is up tp the TextParser's * implementation what kind of opening char to use (#, $, %, etc) * + * + * com.opensymphony.xwork2.ExcludedPatternsChecker + * struts.excludedPatterns.checker + * request + * Used across different interceptors to check if given string matches one of the excluded patterns + * * * * @@ -343,7 +351,7 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider alias(ResultFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_RESULTFACTORY, builder, props); alias(ConverterFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_CONVERTERFACTORY, builder, props); alias(InterceptorFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_INTERCEPTORFACTORY, builder, props); - alias(ValidatorFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_INTERCEPTORFACTORY, builder, props); + alias(ValidatorFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_VALIDATORFACTORY, builder, props); alias(FileManagerFactory.class, StrutsConstants.STRUTS_FILE_MANAGER_FACTORY, builder, props, Scope.SINGLETON); @@ -383,6 +391,10 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider alias(DispatcherErrorHandler.class, StrutsConstants.STRUTS_DISPATCHER_ERROR_HANDLER, builder, props); + /** Checker is used mostly in interceptors, so there be one instance of checker per interceptor with Scope.DEFAULT **/ + alias(ExcludedPatternsChecker.class, StrutsConstants.STRUTS_EXCLUDED_PATTERNS_CHECKER, builder, props, Scope.DEFAULT); + alias(AcceptedPatternsChecker.class, StrutsConstants.STRUTS_ACCEPTED_PATTERNS_CHECKER, builder, props, Scope.DEFAULT); + switchDevMode(props); // Convert Struts properties into XWork properties @@ -392,6 +404,14 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider convertIfExist(props, StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, XWorkConstants.ALLOW_STATIC_METHOD_ACCESS); convertIfExist(props, StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, XWorkConstants.RELOAD_XML_CONFIGURATION); + convertIfExist(props, StrutsConstants.STRUTS_EXCLUDED_CLASSES, XWorkConstants.OGNL_EXCLUDED_CLASSES); + convertIfExist(props, StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS); + + convertIfExist(props, StrutsConstants.STRUTS_ADDITIONAL_EXCLUDED_PATTERNS, XWorkConstants.ADDITIONAL_EXCLUDED_PATTERNS); + convertIfExist(props, StrutsConstants.STRUTS_ADDITIONAL_ACCEPTED_PATTERNS, XWorkConstants.ADDITIONAL_ACCEPTED_PATTERNS); + convertIfExist(props, StrutsConstants.STRUTS_OVERRIDE_EXCLUDED_PATTERNS, XWorkConstants.OVERRIDE_EXCLUDED_PATTERNS); + convertIfExist(props, StrutsConstants.STRUTS_OVERRIDE_ACCEPTED_PATTERNS, XWorkConstants.OVERRIDE_ACCEPTED_PATTERNS); + LocalizedTextUtil.addDefaultResourceBundle("org/apache/struts2/struts-messages"); loadCustomResourceBundles(props); } diff --git a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java index 340b57f81..ca195faa3 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/CookieInterceptor.java @@ -23,8 +23,9 @@ package org.apache.struts2.interceptor; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; +import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; -import com.opensymphony.xwork2.ExcludedPatterns; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.logging.Logger; @@ -33,7 +34,6 @@ import org.apache.struts2.ServletActionContext; import javax.servlet.http.Cookie; import java.util.Collections; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -176,12 +176,12 @@ public class CookieInterceptor extends AbstractInterceptor { // Allowed names of cookies private Pattern acceptedPattern = Pattern.compile(ACCEPTED_PATTERN, Pattern.CASE_INSENSITIVE); - private Set excludedPatterns = new HashSet(); - public CookieInterceptor() { - for (String pattern : ExcludedPatterns.EXCLUDED_PATTERNS) { - excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); - } + private ExcludedPatternsChecker excludedPatternsChecker; + + @Inject + public void setExcludedPatternsChecker(ExcludedPatternsChecker excludedPatternsChecker) { + this.excludedPatternsChecker = excludedPatternsChecker; } /** @@ -260,16 +260,7 @@ public class CookieInterceptor extends AbstractInterceptor { * @return true|false */ protected boolean isAcceptableValue(String value) { - for (Pattern excludedPattern : excludedPatterns) { - boolean matches = !excludedPattern.matcher(value).matches(); - if (!matches) { - if (LOG.isTraceEnabled()) { - LOG.trace("Cookie value [#0] matches excludedPattern [#1]", value, excludedPattern.toString()); - } - return false; - } - } - return true; + return !isExcluded(value) && isAccepted(value); } /** @@ -283,7 +274,7 @@ public class CookieInterceptor extends AbstractInterceptor { } /** - * Checks if name of Cookie match {@link #acceptedPattern} + * Checks if name/value of Cookie is acceptable * * @param name of Cookie * @return true|false @@ -303,24 +294,21 @@ public class CookieInterceptor extends AbstractInterceptor { } /** - * Checks if name of Cookie match {@link #excludedPatterns} + * Checks if name/value of Cookie is excluded * * @param name of Cookie * @return true|false */ protected boolean isExcluded(String name) { - for (Pattern excludedPattern : excludedPatterns) { - boolean matches = excludedPattern.matcher(name).matches(); - if (matches) { - if (LOG.isTraceEnabled()) { - LOG.trace("Cookie [#0] matches excludedPattern [#1]", name, excludedPattern.toString()); - } - return true; - } else { - if (LOG.isTraceEnabled()) { - LOG.trace("Cookie [#0] doesn't match excludedPattern [#1]", name, excludedPattern.toString()); - } + ExcludedPatternsChecker.IsExcluded excluded = excludedPatternsChecker.isExcluded(name); + if (excluded.isExcluded()) { + if (LOG.isTraceEnabled()) { + LOG.trace("Cookie [#0] matches excludedPattern [#1]", name, excluded.getExcludedPattern()); } + return true; + } + if (LOG.isTraceEnabled()) { + LOG.trace("Cookie [#0] doesn't match excludedPattern [#1]", name, excluded.getExcludedPattern()); } return false; } diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 6e858017d..ea2a631c5 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -37,6 +37,23 @@ "http://struts.apache.org/dtds/struts-2.3.dtd"> + + + + + @@ -141,6 +158,9 @@ + + + diff --git a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java index cd42ed557..f9eb4c70b 100644 --- a/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java +++ b/core/src/test/java/org/apache/struts2/TestConfigurationProvider.java @@ -24,6 +24,8 @@ package org.apache.struts2; import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionProxyFactory; import com.opensymphony.xwork2.DefaultActionProxyFactory; +import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.ObjectFactory; import com.opensymphony.xwork2.config.Configuration; import com.opensymphony.xwork2.config.ConfigurationException; @@ -164,5 +166,8 @@ public class TestConfigurationProvider implements ConfigurationProvider { if (!builder.contains(ActionProxyFactory.class)) { builder.factory(ActionProxyFactory.class, DefaultActionProxyFactory.class); } + if (!builder.contains(ExcludedPatternsChecker.class)) { + builder.factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class); + } } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java index 99ba15164..a531a69d7 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/CookieInterceptorTest.java @@ -27,6 +27,7 @@ import java.util.Map; import javax.servlet.http.Cookie; +import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; import com.opensymphony.xwork2.mock.MockActionInvocation; import org.easymock.MockControl; import org.springframework.mock.web.MockHttpServletRequest; @@ -65,6 +66,8 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { // by default the interceptor doesn't accept any cookies CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); + interceptor.intercept(invocation); assertTrue(action.getCookiesMap().isEmpty()); @@ -99,6 +102,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("*"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -140,6 +144,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie2, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -180,6 +185,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value, cookie2value, cookie3value"); interceptor.intercept(invocation); @@ -220,6 +226,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("*"); interceptor.intercept(invocation); @@ -260,6 +267,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue(""); interceptor.intercept(invocation); @@ -301,6 +309,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { actionInvocationControl.replay(); CookieInterceptor interceptor = new CookieInterceptor(); + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("cookie1, cookie3"); interceptor.setCookiesValue("cookie1value"); interceptor.intercept(invocation); @@ -361,6 +370,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { return accepted; } }; + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation(); @@ -420,6 +430,7 @@ public class CookieInterceptorTest extends StrutsInternalTestCase { return accepted; } }; + interceptor.setExcludedPatternsChecker(new DefaultExcludedPatternsChecker()); interceptor.setCookiesName("*"); MockActionInvocation invocation = new MockActionInvocation(); diff --git a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java index 01d1a6eaa..5a01015bf 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java @@ -32,6 +32,7 @@ import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.interceptor.ParametersInterceptor; import com.opensymphony.xwork2.mock.MockResult; +import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.location.LocatableProperties; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsInternalTestCase; @@ -222,6 +223,7 @@ public class ExecuteAndWaitInterceptorTest extends StrutsInternalTestCase { public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { builder.factory(ObjectFactory.class); builder.factory(ActionProxyFactory.class, DefaultActionProxyFactory.class); + builder.factory(OgnlUtil.class, OgnlUtil.class); } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/PropertyTagTest.java b/core/src/test/java/org/apache/struts2/views/jsp/PropertyTagTest.java index cce9a0ccf..a2b77ba8c 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/PropertyTagTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/PropertyTagTest.java @@ -180,11 +180,13 @@ public class PropertyTagTest extends StrutsInternalTestCase { pageContext.setRequest(request); // test - {PropertyTag tag = new PropertyTag(); - tag.setPageContext(pageContext); - tag.setValue("%{toString()}"); - tag.doStartTag(); - tag.doEndTag();} + { + PropertyTag tag = new PropertyTag(); + tag.setPageContext(pageContext); + tag.setValue("%{formatTitle()}"); + tag.doStartTag(); + tag.doEndTag(); + } // verify test request.verify(); @@ -212,7 +214,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { tag.setEscape(false); tag.setEscapeJavaScript(true); tag.setPageContext(pageContext); - tag.setValue("%{toString()}"); + tag.setValue("%{formatTitle()}"); tag.doStartTag(); tag.doEndTag();} @@ -242,7 +244,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { tag.setEscape(false); tag.setEscapeXml(true); tag.setPageContext(pageContext); - tag.setValue("%{toString()}"); + tag.setValue("%{formatTitle()}"); tag.doStartTag(); tag.doEndTag();} @@ -272,7 +274,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { tag.setEscape(false); tag.setEscapeCsv(true); tag.setPageContext(pageContext); - tag.setValue("%{toString()}"); + tag.setValue("%{formatTitle()}"); tag.doStartTag(); tag.doEndTag();} @@ -300,7 +302,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { // test {PropertyTag tag = new PropertyTag(); tag.setPageContext(pageContext); - tag.setValue("toString()"); + tag.setValue("formatTitle()"); tag.doStartTag(); tag.doEndTag();} @@ -328,7 +330,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { // test {PropertyTag tag = new PropertyTag(); tag.setPageContext(pageContext); - tag.setValue("toString()"); + tag.setValue("formatTitle()"); tag.doStartTag(); tag.doEndTag();} @@ -356,7 +358,7 @@ public class PropertyTagTest extends StrutsInternalTestCase { // test {PropertyTag tag = new PropertyTag(); tag.setPageContext(pageContext); - tag.setValue("%{toString()}"); + tag.setValue("%{formatTitle()}"); tag.doStartTag(); tag.doEndTag();} @@ -385,8 +387,12 @@ public class PropertyTagTest extends StrutsInternalTestCase { return title; } - public String toString() { + public String formatTitle() { return "Foo is: " + title; } + + public String toString() { + return formatTitle(); + } } } diff --git a/core/src/test/java/org/apache/struts2/views/jsp/ui/SelectTest.java b/core/src/test/java/org/apache/struts2/views/jsp/ui/SelectTest.java index 094cfc921..06b7e805f 100644 --- a/core/src/test/java/org/apache/struts2/views/jsp/ui/SelectTest.java +++ b/core/src/test/java/org/apache/struts2/views/jsp/ui/SelectTest.java @@ -494,7 +494,7 @@ public class SelectTest extends AbstractUITagTest { tag.setList("list2"); tag.setListKey("id"); tag.setListValue("name"); - tag.setValue("fooInt.toString()"); + tag.setValue("fooInt"); // header stuff tag.setHeaderKey("headerKey"); diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionInvocationTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionInvocationTest.java index af2a7fd01..6db05f159 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionInvocationTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionInvocationTest.java @@ -10,6 +10,7 @@ import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.mock.MockInterceptor; +import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.XWorkTestCaseHelper; import junit.framework.TestCase; import org.apache.struts2.ServletActionContext; @@ -228,6 +229,7 @@ public class RestActionInvocationTest extends TestCase { request.setMethod("GET"); + restActionInvocation.setOgnlUtil(new OgnlUtil()); restActionInvocation.invoke(); assertEquals(123, response.getStatus()); diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java b/xwork-core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java index 531a72560..4539e56b8 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/DefaultActionInvocation.java @@ -22,14 +22,14 @@ import com.opensymphony.xwork2.config.entities.ResultConfig; import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.PreResultListener; +import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.ValueStackFactory; import com.opensymphony.xwork2.util.logging.Logger; import com.opensymphony.xwork2.util.logging.LoggerFactory; import com.opensymphony.xwork2.util.profiling.UtilTimerStack; +import ognl.OgnlException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -46,18 +46,8 @@ import java.util.Map; */ public class DefaultActionInvocation implements ActionInvocation { - private static final long serialVersionUID = -585293628862447329L; - - //static { - // if (ObjectFactory.getContinuationPackage() != null) { - // continuationHandler = new ContinuationHandler(); - // } - //} private static final Logger LOG = LoggerFactory.getLogger(DefaultActionInvocation.class); - private static final Class[] EMPTY_CLASS_ARRAY = new Class[0]; - private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; - protected Object action; protected ActionProxy proxy; protected List preResultListeners; @@ -75,6 +65,7 @@ public class DefaultActionInvocation implements ActionInvocation { protected ValueStackFactory valueStackFactory; protected Container container; protected UnknownHandlerManager unknownHandlerManager; + protected OgnlUtil ognlUtil; public DefaultActionInvocation(final Map extraContext, final boolean pushAction) { this.extraContext = extraContext; @@ -106,6 +97,11 @@ public class DefaultActionInvocation implements ActionInvocation { this.actionEventListener = listener; } + @Inject + public void setOgnlUtil(OgnlUtil ognlUtil) { + this.ognlUtil = ognlUtil; + } + public Object getAction() { return action; } @@ -420,22 +416,19 @@ public class DefaultActionInvocation implements ActionInvocation { try { UtilTimerStack.push(timerKey); - boolean methodCalled = false; - Object methodResult = null; - Method method = null; + Object methodResult; try { - method = getAction().getClass().getMethod(methodName, EMPTY_CLASS_ARRAY); - } catch (NoSuchMethodException e) { + methodResult = ognlUtil.getValue(methodName + "()", getStack().getContext(), action); + } catch (OgnlException e) { // hmm -- OK, try doXxx instead try { - String altMethodName = "do" + methodName.substring(0, 1).toUpperCase() + methodName.substring(1); - method = getAction().getClass().getMethod(altMethodName, EMPTY_CLASS_ARRAY); - } catch (NoSuchMethodException e1) { + String altMethodName = "do" + methodName.substring(0, 1).toUpperCase() + methodName.substring(1) + "()"; + methodResult = ognlUtil.getValue(altMethodName, ActionContext.getContext().getContextMap(), action); + } catch (OgnlException e1) { // well, give the unknown handler a shot if (unknownHandlerManager.hasUnknownHandlers()) { try { methodResult = unknownHandlerManager.handleUnknownMethod(action, methodName); - methodCalled = true; } catch (NoSuchMethodException e2) { // throw the original one throw e; @@ -445,29 +438,18 @@ public class DefaultActionInvocation implements ActionInvocation { } } } - - if (!methodCalled) { - methodResult = method.invoke(action, EMPTY_OBJECT_ARRAY); - } - return saveResult(actionConfig, methodResult); - } catch (NoSuchMethodException e) { - throw new IllegalArgumentException("The " + methodName + "() is not defined in action " + getAction().getClass() + ""); - } catch (InvocationTargetException e) { + } catch (OgnlException e) { // We try to return the source exception. - Throwable t = e.getTargetException(); + //Throwable t = e.getTargetException(); if (actionEventListener != null) { - String result = actionEventListener.handleException(t, getStack()); + String result = actionEventListener.handleException(e, getStack()); if (result != null) { return result; } } - if (t instanceof Exception) { - throw (Exception) t; - } else { - throw e; - } + throw e; } finally { UtilTimerStack.pop(timerKey); } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ExcludedPatterns.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ExcludedPatterns.java deleted file mode 100644 index b618a52a0..000000000 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ExcludedPatterns.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.opensymphony.xwork2; - -/** - * ExcludedPatterns contains hard-coded patterns that must be rejected by {@link com.opensymphony.xwork2.interceptor.ParametersInterceptor} - * and partially in CookInterceptor - */ -public class ExcludedPatterns { - - public static final String CLASS_ACCESS_PATTERN = "(.*\\.|^|.*|\\[('|\"))class(\\.|('|\")]|\\[).*"; - - public static final String[] EXCLUDED_PATTERNS = { - CLASS_ACCESS_PATTERN, - "^dojo\\..*", - "^struts\\..*", - "^session\\..*", - "^request\\..*", - "^application\\..*", - "^servlet(Request|Response)\\..*", - "^parameters\\..*" - }; - -} diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java index 19363680e..433b005ef 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java @@ -17,4 +17,14 @@ public final class XWorkConstants { public static final String RELOAD_XML_CONFIGURATION = "reloadXmlConfiguration"; public static final String ALLOW_STATIC_METHOD_ACCESS = "allowStaticMethodAccess"; public static final String XWORK_LOGGER_FACTORY = "xwork.loggerFactory"; + + public static final String OGNL_EXCLUDED_CLASSES = "ognlExcludedClasses"; + public static final String OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS = "ognlExcludedPackageNamePatterns"; + + public static final String ADDITIONAL_EXCLUDED_PATTERNS = "additionalExcludedPatterns"; + public static final String ADDITIONAL_ACCEPTED_PATTERNS = "additionalAcceptedPatterns"; + + public static final String OVERRIDE_EXCLUDED_PATTERNS = "overrideExcludedPatterns"; + public static final String OVERRIDE_ACCEPTED_PATTERNS = "overrideAcceptedPatterns"; + } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java index 0d489994a..19e8e76a8 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java @@ -2,9 +2,13 @@ package com.opensymphony.xwork2.config.providers; import com.opensymphony.xwork2.ActionProxyFactory; import com.opensymphony.xwork2.DefaultActionProxyFactory; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker; +import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker; import com.opensymphony.xwork2.DefaultLocaleProvider; import com.opensymphony.xwork2.DefaultTextProvider; import com.opensymphony.xwork2.DefaultUnknownHandlerManager; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.FileManager; import com.opensymphony.xwork2.FileManagerFactory; import com.opensymphony.xwork2.LocaleProvider; @@ -168,7 +172,12 @@ public class XWorkConfigurationProvider implements ConfigurationProvider { .factory(ArrayConverter.class, Scope.SINGLETON) .factory(DateConverter.class, Scope.SINGLETON) .factory(NumberConverter.class, Scope.SINGLETON) - .factory(StringConverter.class, Scope.SINGLETON); + .factory(StringConverter.class, Scope.SINGLETON) + + .factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class, Scope.DEFAULT) + .factory(AcceptedPatternsChecker.class, DefaultAcceptedPatternsChecker.class, Scope.DEFAULT) + ; + props.setProperty(XWorkConstants.DEV_MODE, Boolean.FALSE.toString()); props.setProperty(XWorkConstants.LOG_MISSING_PROPERTIES, Boolean.FALSE.toString()); props.setProperty(XWorkConstants.ENABLE_OGNL_EXPRESSION_CACHE, Boolean.TRUE.toString()); diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java b/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java index c73b05701..d95c2a78c 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java @@ -17,7 +17,8 @@ package com.opensymphony.xwork2.interceptor; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionInvocation; -import com.opensymphony.xwork2.ExcludedPatterns; +import com.opensymphony.xwork2.security.AcceptedPatternsChecker; +import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.ValidationAware; import com.opensymphony.xwork2.XWorkConstants; import com.opensymphony.xwork2.conversion.impl.InstantiatingNullHandler; @@ -142,26 +143,17 @@ public class ParametersInterceptor extends MethodFilterInterceptor { private static final Logger LOG = LoggerFactory.getLogger(ParametersInterceptor.class); - public static final String ACCEPTED_PARAM_NAMES = "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"; - protected static final int PARAM_NAME_MAX_LENGTH = 100; + private ExcludedPatternsChecker excludedPatterns; + private int paramNameMaxLength = PARAM_NAME_MAX_LENGTH; - - protected boolean ordered = false; - protected Set excludeParams; - protected Set acceptParams = Collections.emptySet(); - private boolean devMode = false; - // Allowed names of parameters - private Pattern acceptedPattern = Pattern.compile(ACCEPTED_PARAM_NAMES, Pattern.CASE_INSENSITIVE); + protected boolean ordered = false; private ValueStackFactory valueStackFactory; - - public ParametersInterceptor() { - initializeHardCodedExcludePatterns(); - } + private AcceptedPatternsChecker acceptedPatterns; @Inject public void setValueStackFactory(ValueStackFactory valueStackFactory) { @@ -173,23 +165,14 @@ public class ParametersInterceptor extends MethodFilterInterceptor { devMode = "true".equalsIgnoreCase(mode); } - /** - * Sets a comma-delimited list of regular expressions to match - * parameters that are allowed in the parameter map (aka whitelist). - *

- * Don't change the default unless you know what you are doing in terms - * of security implications. - * - * @param commaDelim A comma-delimited list of regular expressions - */ - public void setAcceptParamNames(String commaDelim) { - Collection acceptPatterns = ArrayUtils.asCollection(commaDelim); - if (acceptPatterns != null) { - acceptParams = new HashSet(); - for (String pattern : acceptPatterns) { - acceptParams.add(Pattern.compile(pattern)); - } - } + @Inject + public void setExcludedPatterns(ExcludedPatternsChecker excludedPatterns) { + this.excludedPatterns = excludedPatterns; + } + + @Inject + public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) { + this.acceptedPatterns = acceptedPatterns; } /** @@ -290,7 +273,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { for (Map.Entry entry : params.entrySet()) { String name = entry.getKey(); - if (isAcceptableParameter(name, action)) { + Object value = entry.getValue(); + if (isAcceptableParameter(name, action) && isAcceptableValue(value)) { acceptableParameters.put(name, entry.getValue()); } } @@ -315,8 +299,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor { //block or allow access to properties //see WW-2761 for more details MemberAccessValueStack accessValueStack = (MemberAccessValueStack) newStack; - accessValueStack.setAcceptProperties(acceptParams); - accessValueStack.setExcludeProperties(excludeParams); + accessValueStack.setAcceptProperties(acceptedPatterns.getAcceptedPatterns()); + accessValueStack.setExcludeProperties(excludedPatterns.getExcludedPatterns()); } for (Map.Entry entry : acceptableParameters.entrySet()) { @@ -365,6 +349,33 @@ public class ParametersInterceptor extends MethodFilterInterceptor { return acceptableName(name) && (parameterNameAware == null || parameterNameAware.acceptableParameterName(name)); } + /** + * Checks if given value doesn't match global excluded patterns to avoid passing malicious code + * + * @param value incoming parameter's value + * @return true if value is safe + * + * FIXME: can be removed when parameters won't be represented as simple Strings + */ + protected boolean isAcceptableValue(Object value) { + if (value == null) { + return true; + } + Object[] values; + if (value.getClass().isArray()) { + values = (Object[]) value; + } else { + values = new Object[] { value }; + } + boolean result = true; + for (Object obj : values) { + if (isExcluded(obj.toString())) { + result = false; + } + } + return result; + } + /** * Gets an instance of the comparator to use for the ordered sorting. Override this * method to customize the ordering of the parameters as they are set to the @@ -422,33 +433,19 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } protected boolean isAccepted(String paramName) { - if (!this.acceptParams.isEmpty()) { - for (Pattern pattern : acceptParams) { - Matcher matcher = pattern.matcher(paramName); - if (matcher.matches()) { - return true; - } - } - notifyDeveloper("Parameter [#0] didn't match acceptParams list of patterns!", paramName); - return false; - } else { - boolean matches = acceptedPattern.matcher(paramName).matches(); - if (!matches) { - notifyDeveloper("Parameter [#0] didn't match acceptedPattern pattern!", paramName); - } - return matches; + AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName); + if (result.isAccepted()) { + return true; } + notifyDeveloper("Parameter [#0] didn't match accepted pattern [#1]!", paramName, String.valueOf(result.getAcceptedPattern())); + return false; } protected boolean isExcluded(String paramName) { - if (!this.excludeParams.isEmpty()) { - for (Pattern pattern : excludeParams) { - Matcher matcher = pattern.matcher(paramName); - if (matcher.matches()) { - notifyDeveloper("Parameter [#0] is on the excludeParams list of patterns!", paramName); - return true; - } - } + ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName); + if (result.isExcluded()) { + notifyDeveloper("Parameter [#0] matches excluded pattern [#1]!", paramName, String.valueOf(result.getExcludedPattern())); + return true; } return false; } @@ -482,20 +479,16 @@ public class ParametersInterceptor extends MethodFilterInterceptor { } /** - * Gets a set of regular expressions of parameters to remove - * from the parameter map + * Sets a comma-delimited list of regular expressions to match + * parameters that are allowed in the parameter map (aka whitelist). + *

+ * Don't change the default unless you know what you are doing in terms + * of security implications. * - * @return A set of compiled regular expression patterns + * @param commaDelim A comma-delimited list of regular expressions */ - protected Set getExcludeParamsSet() { - return excludeParams; - } - - protected void initializeHardCodedExcludePatterns() { - excludeParams = new HashSet(); - for (String pattern : ExcludedPatterns.EXCLUDED_PATTERNS) { - excludeParams.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); - } + public void setAcceptParamNames(String commaDelim) { + acceptedPatterns.addAcceptedPatterns(commaDelim); } /** @@ -505,12 +498,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor { * @param commaDelim A comma-delimited list of regular expressions */ public void setExcludeParams(String commaDelim) { - Collection excludePatterns = ArrayUtils.asCollection(commaDelim); - if (excludePatterns != null) { - for (String pattern : excludePatterns) { - excludeParams.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); - } - } + excludedPatterns.addExcludedPatterns(commaDelim); } } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index fa907e320..b0345fc89 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -16,12 +16,17 @@ package com.opensymphony.xwork2.ognl; import com.opensymphony.xwork2.XWorkConstants; +import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.conversion.impl.XWorkConverter; +import com.opensymphony.xwork2.inject.Container; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor; import com.opensymphony.xwork2.util.CompoundRoot; +import com.opensymphony.xwork2.util.TextParseUtil; import com.opensymphony.xwork2.util.logging.Logger; import com.opensymphony.xwork2.util.logging.LoggerFactory; import com.opensymphony.xwork2.util.reflection.ReflectionException; +import ognl.ClassResolver; import ognl.Ognl; import ognl.OgnlContext; import ognl.OgnlException; @@ -36,9 +41,12 @@ import java.beans.PropertyDescriptor; import java.lang.reflect.Method; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.regex.Pattern; /** @@ -58,6 +66,12 @@ public class OgnlUtil { private boolean enableExpressionCache = true; private boolean enableEvalExpression; + private Set> excludedClasses = new HashSet>(); + private Set excludedPackageNamePatterns = new HashSet(); + + private Container container; + private boolean allowStaticMethodAccess; + @Inject public void setXWorkConverter(XWorkConverter conv) { this.defaultConverter = new OgnlTypeConverterWrapper(conv); @@ -82,6 +96,44 @@ public class OgnlUtil { } } + @Inject(value = XWorkConstants.OGNL_EXCLUDED_CLASSES, required = false) + public void setExcludedClasses(String commaDelimitedClasses) { + Set classes = TextParseUtil.commaDelimitedStringToSet(commaDelimitedClasses); + for (String className : classes) { + try { + excludedClasses.add(Class.forName(className)); + } catch (ClassNotFoundException e) { + throw new ConfigurationException("Cannot load excluded class: " + className, e); + } + } + } + + @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) + public void setExcludedPackageName(String commaDelimitedPackagePatterns) { + Set packagePatterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackagePatterns); + for (String pattern : packagePatterns) { + excludedPackageNamePatterns.add(Pattern.compile(pattern)); + } + } + + public Set> getExcludedClasses() { + return excludedClasses; + } + + public Set getExcludedPackageNamePatterns() { + return excludedPackageNamePatterns; + } + + @Inject + public void setContainer(Container container) { + this.container = container; + } + + @Inject(value = XWorkConstants.ALLOW_STATIC_METHOD_ACCESS, required = false) + public void setAllowStaticMethodAccess(String allowStaticMethodAccess) { + this.allowStaticMethodAccess = Boolean.parseBoolean(allowStaticMethodAccess); + } + /** * Sets the object's properties using the default type converter, defaulting to not throw * exceptions for problems setting the properties. @@ -141,7 +193,7 @@ public class OgnlUtil { * problems setting the properties */ public void setProperties(Map properties, Object o, boolean throwPropertyExceptions) { - Map context = Ognl.createDefaultContext(o); + Map context = createDefaultContext(o, null); setProperties(properties, o, context, throwPropertyExceptions); } @@ -329,9 +381,9 @@ public class OgnlUtil { } TypeConverter conv = getTypeConverterFromContext(context); - final Map contextFrom = Ognl.createDefaultContext(from); + final Map contextFrom = createDefaultContext(from, null); Ognl.setTypeConverter(contextFrom, conv); - final Map contextTo = Ognl.createDefaultContext(to); + final Map contextTo = createDefaultContext(to, null); Ognl.setTypeConverter(contextTo, conv); PropertyDescriptor[] fromPds; @@ -440,7 +492,7 @@ public class OgnlUtil { */ public Map getBeanMap(final Object source) throws IntrospectionException, OgnlException { Map beanMap = new HashMap(); - final Map sourceMap = Ognl.createDefaultContext(source); + final Map sourceMap = createDefaultContext(source, null); PropertyDescriptor[] propertyDescriptors = getPropertyDescriptors(source); for (PropertyDescriptor propertyDescriptor : propertyDescriptors) { final String propertyName = propertyDescriptor.getDisplayName(); @@ -518,6 +570,23 @@ public class OgnlUtil { return defaultConverter; } + protected Map createDefaultContext(Object root) { + return createDefaultContext(root, null); + } + + protected Map createDefaultContext(Object root, ClassResolver classResolver) { + ClassResolver resolver = classResolver; + if (resolver == null) { + resolver = container.getInstance(CompoundRootAccessor.class); + } + + SecurityMemberAccess memberAccess = new SecurityMemberAccess(allowStaticMethodAccess); + memberAccess.setExcludedClasses(excludedClasses); + memberAccess.setExcludedPackageNamePatterns(excludedPackageNamePatterns); + + return Ognl.createDefaultContext(root, resolver, defaultConverter, memberAccess); + } + private interface OgnlTask { T execute(Object tree) throws OgnlException; } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index 76f0d3fb9..acf54c4c8 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java @@ -79,6 +79,8 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS @Inject public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; + securityMemberAccess.setExcludedClasses(ognlUtil.getExcludedClasses()); + securityMemberAccess.setExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns()); } protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, @@ -197,7 +199,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS throw new XWorkException(message, re); } else { if (LOG.isWarnEnabled()) { - LOG.warn("Error setting value", re); + LOG.warn("Error setting value [#0] with expression [#1]", re, value.toString(), expr); } } } @@ -446,7 +448,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS XWorkConverter xworkConverter = cont.getInstance(XWorkConverter.class); CompoundRootAccessor accessor = (CompoundRootAccessor) cont.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()); TextProvider prov = cont.getInstance(TextProvider.class, "system"); - boolean allow = "true".equals(cont.getInstance(String.class, "allowStaticMethodAccess")); + boolean allow = "true".equals(cont.getInstance(String.class, XWorkConstants.ALLOW_STATIC_METHOD_ACCESS)); OgnlValueStack aStack = new OgnlValueStack(xworkConverter, accessor, prov, allow); aStack.setOgnlUtil(cont.getInstance(OgnlUtil.class)); aStack.setRoot(xworkConverter, accessor, this.root, allow); diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 7bbcbda10..d0862e7de 100644 --- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -15,6 +15,8 @@ */ package com.opensymphony.xwork2.ognl; +import com.opensymphony.xwork2.util.logging.Logger; +import com.opensymphony.xwork2.util.logging.LoggerFactory; import ognl.DefaultMemberAccess; import java.lang.reflect.Member; @@ -32,9 +34,13 @@ import java.util.regex.Pattern; */ public class SecurityMemberAccess extends DefaultMemberAccess { + private static final Logger LOG = LoggerFactory.getLogger(SecurityMemberAccess.class); + private final boolean allowStaticMethodAccess; private Set excludeProperties = Collections.emptySet(); private Set acceptProperties = Collections.emptySet(); + private Set> excludedClasses = Collections.emptySet(); + private Set excludedPackageNamePatterns = Collections.emptySet(); public SecurityMemberAccess(boolean method) { super(false); @@ -46,8 +52,20 @@ public class SecurityMemberAccess extends DefaultMemberAccess { } @Override - public boolean isAccessible(Map context, Object target, Member member, - String propertyName) { + public boolean isAccessible(Map context, Object target, Member member, String propertyName) { + if (isPackageExcluded(target.getClass().getPackage(), member.getDeclaringClass().getPackage())) { + if (LOG.isWarnEnabled()) { + LOG.warn("Package of target [#0] or package of member [#1] are excluded!", target, member); + } + return false; + } + + if (isClassExcluded(target.getClass(), member.getDeclaringClass())) { + if (LOG.isWarnEnabled()) { + LOG.warn("Target class [#0] or declaring class of member type [#1] are excluded!", target, member); + } + return false; + } boolean allow = true; int modifiers = member.getModifiers(); @@ -74,6 +92,27 @@ public class SecurityMemberAccess extends DefaultMemberAccess { return isAcceptableProperty(propertyName); } + protected boolean isPackageExcluded(Package targetPackage, Package memberPackage) { + for (Pattern pattern : excludedPackageNamePatterns) { + if (pattern.matcher(targetPackage.getName()).matches() || pattern.matcher(memberPackage.getName()).matches()) { + return true; + } + } + return false; + } + + protected boolean isClassExcluded(Class targetClass, Class declaringClass) { + if (targetClass == Object.class || declaringClass == Object.class) { + return true; + } + for (Class excludedClass : excludedClasses) { + if (targetClass.isAssignableFrom(excludedClass) || declaringClass.isAssignableFrom(excludedClass)) { + return true; + } + } + return false; + } + protected boolean isAcceptableProperty(String name) { return name == null || ((!isExcluded(name)) && isAccepted(name)); } @@ -115,4 +154,11 @@ public class SecurityMemberAccess extends DefaultMemberAccess { this.acceptProperties = acceptedProperties; } + public void setExcludedClasses(Set> excludedClasses) { + this.excludedClasses = excludedClasses; + } + + public void setExcludedPackageNamePatterns(Set excludedPackageNamePatterns) { + this.excludedPackageNamePatterns = excludedPackageNamePatterns; + } } diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java b/xwork-core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java new file mode 100644 index 000000000..6ea9ec9ca --- /dev/null +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java @@ -0,0 +1,82 @@ +package com.opensymphony.xwork2.security; + +import java.util.Set; +import java.util.regex.Pattern; + +/** + * Used across different interceptors to check if given string matches one of the excluded patterns. + */ +public interface AcceptedPatternsChecker { + + /** + * Checks if value matches any of patterns on exclude list + * + * @param value to check + * @return object containing result of matched pattern and pattern itself + */ + public IsAccepted isAccepted(String value); + + /** + * Allows add additional excluded patterns during runtime + * + * @param commaDelimitedPatterns comma delimited string with patterns + */ + public void addAcceptedPatterns(String commaDelimitedPatterns); + + /** + * Allows add additional excluded patterns during runtime + * + * @param additionalPatterns array of additional excluded patterns + */ + public void addAcceptedPatterns(String[] additionalPatterns); + + /** + * Allows add additional excluded patterns during runtime + * + * @param additionalPatterns set of additional patterns + */ + public void addAcceptedPatterns(Set additionalPatterns); + + /** + * Allow access list of all defined excluded patterns + * + * @return set of excluded patterns + */ + public Set getAcceptedPatterns(); + + public final static class IsAccepted { + + private final boolean accepted; + private final Pattern acceptedPattern; + + public static IsAccepted yes(Pattern acceptedPattern) { + return new IsAccepted(true, acceptedPattern); + } + + public static IsAccepted no() { + return new IsAccepted(false, null); + } + + private IsAccepted(boolean accepted, Pattern acceptedPattern) { + this.accepted = accepted; + this.acceptedPattern = acceptedPattern; + } + + public boolean isAccepted() { + return accepted; + } + + public Pattern getAcceptedPattern() { + return acceptedPattern; + } + + @Override + public String toString() { + return "IsAccepted {" + + "accepted=" + accepted + + ", acceptedPattern=" + acceptedPattern + + " }"; + } + } + +} diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java new file mode 100644 index 000000000..970a52cc5 --- /dev/null +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java @@ -0,0 +1,86 @@ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.XWorkConstants; +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.util.TextParseUtil; +import com.opensymphony.xwork2.util.logging.Logger; +import com.opensymphony.xwork2.util.logging.LoggerFactory; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.regex.Pattern; + +public class DefaultAcceptedPatternsChecker implements AcceptedPatternsChecker { + + private static final Logger LOG = LoggerFactory.getLogger(DefaultAcceptedPatternsChecker.class); + + public static final String[] ACCEPTED_PATTERNS = { + "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*" + }; + + private Set acceptedPatterns; + + public DefaultAcceptedPatternsChecker() { + acceptedPatterns = new HashSet(); + for (String pattern : ACCEPTED_PATTERNS) { + acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + @Inject(value = XWorkConstants.OVERRIDE_ACCEPTED_PATTERNS, required = false) + public void setOverrideAcceptedPatterns(String acceptablePatterns) { + if (LOG.isWarnEnabled()) { + LOG.warn("Overriding [#0] with [#1], be aware that this can affect safety of your application!", + XWorkConstants.OVERRIDE_ACCEPTED_PATTERNS, acceptablePatterns); + } + acceptedPatterns = new HashSet(); + for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) { + acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + @Inject(value = XWorkConstants.ADDITIONAL_ACCEPTED_PATTERNS, required = false) + public void setAdditionalAcceptedPatterns(String acceptablePatterns) { + if (LOG.isDebugEnabled()) { + LOG.warn("Adding additional patterns [#0] to accepted patterns!", acceptablePatterns); + } + for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) { + acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + public void addAcceptedPatterns(String commaDelimitedPatterns) { + addAcceptedPatterns(TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns)); + } + + public void addAcceptedPatterns(String[] additionalPatterns) { + addAcceptedPatterns(new HashSet(Arrays.asList(additionalPatterns))); + } + + public void addAcceptedPatterns(Set additionalPatterns) { + if (LOG.isTraceEnabled()) { + LOG.trace("Adding additional excluded patterns [#0]", additionalPatterns); + } + for (String pattern : additionalPatterns) { + acceptedPatterns.add(Pattern.compile(pattern)); + } + } + + public IsAccepted isAccepted(String value) { + for (Pattern acceptedPattern : acceptedPatterns) { + if (acceptedPattern.matcher(value).matches()) { + if (LOG.isTraceEnabled()) { + LOG.trace("[#0] matches accepted pattern [#1]", value, acceptedPattern); + } + return IsAccepted.yes(acceptedPattern); + } + } + return IsAccepted.no(); + } + + public Set getAcceptedPatterns() { + return acceptedPatterns; + } + +} diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java new file mode 100644 index 000000000..983ce630b --- /dev/null +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java @@ -0,0 +1,95 @@ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.*; +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.util.TextParseUtil; +import com.opensymphony.xwork2.util.logging.Logger; +import com.opensymphony.xwork2.util.logging.LoggerFactory; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.regex.Pattern; + +public class DefaultExcludedPatternsChecker implements ExcludedPatternsChecker { + + private static final Logger LOG = LoggerFactory.getLogger(DefaultExcludedPatternsChecker.class); + + public static final String[] EXCLUDED_PATTERNS = { + "(.*\\.|^|.*|\\[('|\"))class(\\.|('|\")]|\\[).*", + "(^|.*#)dojo(\\.|\\[).*", + "(^|.*#)struts(\\.|\\[).*", + "(^|.*#)session(\\.|\\[).*", + "(^|.*#)request(\\.|\\[).*", + "(^|.*#)application(\\.|\\[).*", + "(^|.*#)servlet(Request|Response)(\\.|\\[).*", + "(^|.*#)parameters(\\.|\\[).*", + "(^|.*#)context(\\.|\\[).*", + "(^|.*#)_memberAccess(\\.|\\[).*" + }; + + private Set excludedPatterns; + + public DefaultExcludedPatternsChecker() { + excludedPatterns = new HashSet(); + for (String pattern : EXCLUDED_PATTERNS) { + excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + @Inject(value = XWorkConstants.OVERRIDE_EXCLUDED_PATTERNS, required = false) + public void setOverrideExcludePatterns(String excludePatterns) { + if (LOG.isWarnEnabled()) { + LOG.warn("Overriding [#0] with [#1], be aware that this can affect safety of your application!", + XWorkConstants.OVERRIDE_EXCLUDED_PATTERNS, excludePatterns); + } + excludedPatterns = new HashSet(); + for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) { + excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + @Inject(value = XWorkConstants.ADDITIONAL_EXCLUDED_PATTERNS, required = false) + public void setAdditionalExcludePatterns(String excludePatterns) { + if (LOG.isDebugEnabled()) { + LOG.debug("Adding additional patterns [#0] to excluded patterns!", excludePatterns); + } + for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) { + excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE)); + } + } + + public void addExcludedPatterns(String commaDelimitedPatterns) { + addExcludedPatterns(TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns)); + } + + public void addExcludedPatterns(String[] additionalPatterns) { + addExcludedPatterns(new HashSet(Arrays.asList(additionalPatterns))); + } + + public void addExcludedPatterns(Set additionalPatterns) { + if (LOG.isTraceEnabled()) { + LOG.trace("Adding additional excluded patterns [#0]", additionalPatterns); + } + for (String pattern : additionalPatterns) { + excludedPatterns.add(Pattern.compile(pattern)); + } + } + + public IsExcluded isExcluded(String value) { + for (Pattern excludedPattern : excludedPatterns) { + if (excludedPattern.matcher(value).matches()) { + if (LOG.isTraceEnabled()) { + LOG.trace("[#0] matches excluded pattern [#1]", value, excludedPattern); + } + return IsExcluded.yes(excludedPattern); + } + } + return IsExcluded.no(); + } + + public Set getExcludedPatterns() { + return excludedPatterns; + } + +} diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java b/xwork-core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java new file mode 100644 index 000000000..51751e956 --- /dev/null +++ b/xwork-core/src/main/java/com/opensymphony/xwork2/security/ExcludedPatternsChecker.java @@ -0,0 +1,82 @@ +package com.opensymphony.xwork2.security; + +import java.util.Set; +import java.util.regex.Pattern; + +/** + * Used across different interceptors to check if given string matches one of the excluded patterns. + */ +public interface ExcludedPatternsChecker { + + /** + * Checks if value matches any of patterns on exclude list + * + * @param value to check + * @return object containing result of matched pattern and pattern itself + */ + public IsExcluded isExcluded(String value); + + /** + * Allows add additional excluded patterns during runtime + * + * @param commaDelimitedPatterns comma delimited string with patterns + */ + public void addExcludedPatterns(String commaDelimitedPatterns); + + /** + * Allows add additional excluded patterns during runtime + * + * @param additionalPatterns array of additional excluded patterns + */ + public void addExcludedPatterns(String[] additionalPatterns); + + /** + * Allows add additional excluded patterns during runtime + * + * @param additionalPatterns set of additional patterns + */ + public void addExcludedPatterns(Set additionalPatterns); + + /** + * Allow access list of all defined excluded patterns + * + * @return set of excluded patterns + */ + public Set getExcludedPatterns(); + + public final static class IsExcluded { + + private final boolean excluded; + private final Pattern excludedPattern; + + public static IsExcluded yes(Pattern excludedPattern) { + return new IsExcluded(true, excludedPattern); + } + + public static IsExcluded no() { + return new IsExcluded(false, null); + } + + private IsExcluded(boolean excluded, Pattern excludedPattern) { + this.excluded = excluded; + this.excludedPattern = excludedPattern; + } + + public boolean isExcluded() { + return excluded; + } + + public Pattern getExcludedPattern() { + return excludedPattern; + } + + @Override + public String toString() { + return "IsExcluded { " + + "excluded=" + excluded + + ", excludedPattern=" + excludedPattern + + " }"; + } + } + +} diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java index 1b93a5c5c..e0aa8ba7e 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/DefaultActionInvocationTest.java @@ -1,9 +1,14 @@ package com.opensymphony.xwork2; +import com.mockobjects.dynamic.Mock; import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.mock.MockContainer; import com.opensymphony.xwork2.mock.MockInterceptor; +import com.opensymphony.xwork2.ognl.OgnlUtil; +import com.opensymphony.xwork2.util.ValueStackFactory; +import org.easymock.EasyMock; +import org.easymock.IMocksControl; import java.util.ArrayList; import java.util.HashMap; @@ -39,6 +44,9 @@ public class DefaultActionInvocationTest extends XWorkTestCase { mockInterceptor3.setExpectedFoo("test3"); DefaultActionInvocation defaultActionInvocation = new DefaultActionInvocationTester(interceptorMappings); + container.inject(defaultActionInvocation); + defaultActionInvocation.stack = container.getInstance(ValueStackFactory.class).createValueStack(); + defaultActionInvocation.invoke(); assertTrue(mockInterceptor1.isExecuted()); assertTrue(mockInterceptor2.isExecuted()); diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java index 4a7f5175b..14d9be186 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/conversion/impl/AnnotationXWorkConverterTest.java @@ -374,8 +374,8 @@ public class AnnotationXWorkConverterTest extends XWorkTestCase { stack.setValue("genericMap[456.12]", "42"); assertEquals(2, gb.getGenericMap().size()); - assertEquals(Integer.class, stack.findValue("genericMap.get(123.12).class")); - assertEquals(Integer.class, stack.findValue("genericMap.get(456.12).class")); + assertEquals("66", stack.findValue("genericMap.get(123.12).toString()")); + assertEquals("42", stack.findValue("genericMap.get(456.12).toString()")); assertEquals(66, stack.findValue("genericMap.get(123.12)")); assertEquals(42, stack.findValue("genericMap.get(456.12)")); assertEquals(true, stack.findValue("genericMap.containsValue(66)")); @@ -393,8 +393,8 @@ public class AnnotationXWorkConverterTest extends XWorkTestCase { stack.setValue("genericMap[456.12]", "42"); assertEquals(2, gb.getGenericMap().size()); - assertEquals(Integer.class, stack.findValue("genericMap.get(123.12).class")); - assertEquals(Integer.class, stack.findValue("genericMap.get(456.12).class")); + assertEquals("66", stack.findValue("genericMap.get(123.12).toString()")); + assertEquals("42", stack.findValue("genericMap.get(456.12).toString()")); assertEquals(66, stack.findValue("genericMap.get(123.12)")); assertEquals(42, stack.findValue("genericMap.get(456.12)")); assertEquals(true, stack.findValue("genericMap.containsValue(66)")); @@ -409,7 +409,7 @@ public class AnnotationXWorkConverterTest extends XWorkTestCase { stack.push(gb); assertEquals(1, gb.getGetterList().size()); - assertEquals(Double.class, stack.findValue("getterList.get(0).class")); + assertEquals("42.42", stack.findValue("getterList.get(0).toString()")); assertEquals(new Double(42.42), stack.findValue("getterList.get(0)")); assertEquals(new Double(42.42), gb.getGetterList().get(0)); diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java index 7084924a6..d6fc7c546 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java @@ -18,7 +18,6 @@ package com.opensymphony.xwork2.interceptor; import com.opensymphony.xwork2.Action; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.ActionProxy; -import com.opensymphony.xwork2.ExcludedPatterns; import com.opensymphony.xwork2.ModelDrivenAction; import com.opensymphony.xwork2.SimpleAction; import com.opensymphony.xwork2.TestBean; @@ -33,11 +32,13 @@ import com.opensymphony.xwork2.conversion.impl.XWorkConverter; import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.ognl.OgnlValueStack; import com.opensymphony.xwork2.ognl.OgnlValueStackFactory; +import com.opensymphony.xwork2.ognl.SecurityMemberAccess; import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor; import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.ValueStackFactory; import junit.framework.Assert; +import ognl.OgnlContext; import ognl.PropertyAccessor; import java.io.File; @@ -45,12 +46,10 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.regex.Pattern; /** @@ -111,13 +110,11 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(action, vs, params); // then - assertEquals(2, action.getActionMessages().size()); + assertEquals(1, action.getActionMessages().size()); String msg1 = action.getActionMessage(0); - String msg2 = action.getActionMessage(1); - assertTrue(msg1.contains("Error setting expression 'name' with value '(#context[\"xwork.MethodAccessor.denyMethodExecution\"]= new java.lang.Boolean(false), #_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true), @java.lang.Runtime@getRuntime().exec('mkdir /tmp/PWNAGE'))(meh)'")); - assertTrue(msg2.contains("Error setting expression 'top['name'](0)' with value 'true'")); + assertTrue(msg1.contains("Error setting expression 'top['name'](0)' with value 'true'")); assertNull(action.getName()); } @@ -146,7 +143,6 @@ public class ParametersInterceptorTest extends XWorkTestCase { }; - pi.setExcludeParams("(.*\\.|^)class\\..*"); container.inject(pi); ValueStack vs = ActionContext.getContext().getValueStack(); @@ -164,12 +160,14 @@ public class ParametersInterceptorTest extends XWorkTestCase { // given final String pollution1 = "class.classLoader.jarPath"; final String pollution2 = "model.class.classLoader.jarPath"; + final String pollution3 = "class.classLoader.defaultAssertionStatus"; - loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml")); + loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-class-param-test.xml")); final Map params = new HashMap() { { put(pollution1, "bad"); put(pollution2, "very bad"); + put(pollution3, true); } }; @@ -183,10 +181,6 @@ public class ParametersInterceptorTest extends XWorkTestCase { return result; } - @Override - protected void initializeHardCodedExcludePatterns() { - excludeParams = new HashSet(); - } }; container.inject(pi); @@ -197,16 +191,19 @@ public class ParametersInterceptorTest extends XWorkTestCase { pi.setParameters(action, vs, params); // then - assertEquals(2, action.getActionMessages().size()); + assertEquals(3, action.getActionMessages().size()); String msg1 = action.getActionMessage(0); String msg2 = action.getActionMessage(1); + String msg3 = action.getActionMessage(2); - assertEquals("Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg1); - assertEquals("Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg2); + assertEquals("Error setting expression 'class.classLoader.defaultAssertionStatus' with value 'true'", msg1); + assertEquals("Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg2); + assertEquals("Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg3); assertFalse(excluded.get(pollution1)); assertFalse(excluded.get(pollution2)); + assertFalse(excluded.get(pollution3)); } public void testDoesNotAllowMethodInvocations() throws Exception { @@ -299,11 +296,6 @@ public class ParametersInterceptorTest extends XWorkTestCase { final Map excluded = new HashMap(); ParametersInterceptor pi = new ParametersInterceptor() { - @Override - protected void initializeHardCodedExcludePatterns() { - this.excludeParams = new HashSet(); - } - @Override protected boolean isExcluded(String paramName) { boolean result = super.isExcluded(paramName); @@ -313,7 +305,6 @@ public class ParametersInterceptorTest extends XWorkTestCase { }; - pi.setExcludeParams("(.*\\.|^|.*|\\[('|\"))class(\\.|('|\")]|\\[).*"); container.inject(pi); ValueStack vs = ActionContext.getContext().getValueStack(); @@ -351,9 +342,8 @@ public class ParametersInterceptorTest extends XWorkTestCase { //then assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah()); - Object allowMethodAccess = stack.findValue("\u0023_memberAccess['allowStaticMethodAccess']"); - assertNotNull(allowMethodAccess); - assertEquals(Boolean.FALSE, allowMethodAccess); + boolean allowMethodAccess = ((SecurityMemberAccess) ((OgnlContext) stack.getContext()).getMemberAccess()).getAllowStaticMethodAccess(); + assertFalse(allowMethodAccess); } public void testParameters() throws Exception { @@ -487,7 +477,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { proxy.execute(); SimpleAction action = (SimpleAction) proxy.getAction(); - assertNull(action.getName()); + assertEquals("try_1", action.getName()); assertEquals("This is blah", (action).getBlah()); assertEquals(123, action.getBaz()); } @@ -731,11 +721,6 @@ public class ParametersInterceptorTest extends XWorkTestCase { assertEquals(expected, actual); } - public void testExcludedPatternsGetInitialized() throws Exception { - ParametersInterceptor parametersInterceptor = new ParametersInterceptor(); - assertEquals(ExcludedPatterns.EXCLUDED_PATTERNS.length, parametersInterceptor.excludeParams.size()); - } - private ValueStack injectValueStack(Map actual) { ValueStack stack = createStubValueStack(actual); container.inject(stack); diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 8bd5e23f4..e8733d6b4 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -82,7 +82,7 @@ public class OgnlUtilTest extends XWorkTestCase { }); Owner owner = new Owner(); - Map context = Ognl.createDefaultContext(owner); + Map context = ognlUtil.createDefaultContext(owner); Map props = new HashMap(); props.put("dog.name", dogName); @@ -107,7 +107,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testCanSetDependentObjectArray() { EmailAction action = new EmailAction(); - Map context = Ognl.createDefaultContext(action); + Map context = ognlUtil.createDefaultContext(action); Map props = new HashMap(); props.put("email[0].address", "addr1"); @@ -125,7 +125,7 @@ public class OgnlUtilTest extends XWorkTestCase { Foo foo1 = new Foo(); Foo foo2 = new Foo(); - Map context = Ognl.createDefaultContext(foo1); + Map context = ognlUtil.createDefaultContext(foo1); Calendar cal = Calendar.getInstance(); cal.clear(); @@ -171,7 +171,7 @@ public class OgnlUtilTest extends XWorkTestCase { foo2.setTitle("foo2 title"); foo2.setNumber(2); - Map context = Ognl.createDefaultContext(foo1); + Map context = ognlUtil.createDefaultContext(foo1); List excludes = new ArrayList(); excludes.add("title"); @@ -200,7 +200,7 @@ public class OgnlUtilTest extends XWorkTestCase { b2.setTitle(""); b2.setId(new Long(2)); - context = Ognl.createDefaultContext(b1); + context = ognlUtil.createDefaultContext(b1); List includes = new ArrayList(); includes.add("title"); includes.add("somethingElse"); @@ -220,7 +220,7 @@ public class OgnlUtilTest extends XWorkTestCase { Foo foo = new Foo(); Bar bar = new Bar(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Calendar cal = Calendar.getInstance(); cal.clear(); @@ -244,7 +244,7 @@ public class OgnlUtilTest extends XWorkTestCase { Foo foo = new Foo(); foo.setBar(new Bar()); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("bar.title", "i am barbaz"); @@ -280,7 +280,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testOgnlHandlesCrapAtTheEndOfANumber() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("aLong", "123a"); @@ -317,7 +317,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testSetPropertiesBoolean() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("useful", "true"); @@ -338,7 +338,7 @@ public class OgnlUtilTest extends XWorkTestCase { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("birthday", "02/12/1982"); @@ -408,7 +408,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testSetPropertiesInt() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("number", "2"); @@ -420,7 +420,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testSetPropertiesLongArray() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("points", new String[]{"1", "2"}); @@ -435,7 +435,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testSetPropertiesString() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("title", "this is a title"); @@ -446,7 +446,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testSetProperty() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); assertFalse(123456 == foo.getNumber()); ognlUtil.setProperty("number", "123456", foo, context); assertEquals(123456, foo.getNumber()); @@ -457,7 +457,7 @@ public class OgnlUtilTest extends XWorkTestCase { ChainingInterceptor foo = new ChainingInterceptor(); ChainingInterceptor foo2 = new ChainingInterceptor(); - OgnlContext context = (OgnlContext) Ognl.createDefaultContext(null); + OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(null); SimpleNode expression = (SimpleNode) Ognl.parseExpression("{'a','ruby','b','tom'}"); @@ -499,7 +499,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testStringToLong() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); Map props = new HashMap(); props.put("aLong", "123"); @@ -518,7 +518,7 @@ public class OgnlUtilTest extends XWorkTestCase { Foo foo = new Foo(); foo.setALong(88); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); ognlUtil.setProperties(null, foo, context); assertEquals(88, foo.getALong()); @@ -531,7 +531,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testCopyNull() { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); ognlUtil.copy(null, null, context); ognlUtil.copy(foo, null, context); @@ -540,7 +540,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testGetTopTarget() throws Exception { Foo foo = new Foo(); - Map context = Ognl.createDefaultContext(foo); + Map context = ognlUtil.createDefaultContext(foo); CompoundRoot root = new CompoundRoot(); Object top = ognlUtil.getRealTarget("top", context, root); @@ -630,7 +630,131 @@ public class OgnlUtilTest extends XWorkTestCase { stack.setValue("1114778947765", foo); stack.setValue("1234", foo); } - + + public void testAvoidCallingMethodsOnObjectClass() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setExcludedClasses(Object.class.getName()); + ognlUtil.setValue("class.classLoader.defaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(NoSuchPropertyException.class, expected.getClass()); + assertEquals("com.opensymphony.xwork2.util.Foo.class", expected.getMessage()); + } + + public void testAvoidCallingMethodsOnObjectClassUpperCased() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setExcludedClasses(Object.class.getName()); + ognlUtil.setValue("Class.ClassLoader.DefaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(NoSuchPropertyException.class, expected.getClass()); + assertEquals("com.opensymphony.xwork2.util.Foo.Class", expected.getMessage()); + } + + public void testAvoidCallingMethodsOnObjectClassAsMap() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setExcludedClasses(Object.class.getName()); + ognlUtil.setValue("class['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(NoSuchPropertyException.class, expected.getClass()); + assertEquals("com.opensymphony.xwork2.util.Foo.class", expected.getMessage()); + } + + public void testAvoidCallingMethodsOnObjectClassAsMap2() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setValue("foo['class']['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(NoSuchPropertyException.class, expected.getClass()); + assertEquals("com.opensymphony.xwork2.util.Foo.foo", expected.getMessage()); + } + + public void testAvoidCallingMethodsOnObjectClassAsMapWithQuotes() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setExcludedClasses(Object.class.getName()); + ognlUtil.setValue("class[\"classLoader\"]['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(NoSuchPropertyException.class, expected.getClass()); + assertEquals("com.opensymphony.xwork2.util.Foo.class", expected.getMessage()); + } + + public void testAvoidCallingToString() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setValue("toString", ognlUtil.createDefaultContext(foo), foo, null); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(OgnlException.class, expected.getClass()); + assertEquals("toString", expected.getMessage()); + } + + public void testAvoidCallingMethodsWithBraces() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setValue("toString()", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(InappropriateExpressionException.class, expected.getClass()); + assertEquals(expected.getMessage(), "Inappropriate OGNL expression: toString()"); + } + + public void testAvoidCallingSomeClasses() throws Exception { + Foo foo = new Foo(); + + Exception expected = null; + try { + ognlUtil.setExcludedClasses(Runtime.class.getName()); + ognlUtil.setValue("@java.lang.Runtime@getRuntime().exec('mate')", ognlUtil.createDefaultContext(foo), foo, true); + fail(); + } catch (OgnlException e) { + expected = e; + } + assertNotNull(expected); + assertSame(MethodFailedException.class, expected.getClass()); + assertEquals(expected.getMessage(), "Method \"getRuntime\" failed for object class java.lang.Runtime"); + } public static class Email { String address; diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java index a4a153af4..cb7108134 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java @@ -58,6 +58,7 @@ public class OgnlValueStackTest extends XWorkTestCase { (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), container.getInstance(TextProvider.class, "system"), allowStaticMethodAccess); container.inject(stack); + ognlUtil.setAllowStaticMethodAccess(Boolean.toString(allowStaticMethodAccess)); return stack; } diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java new file mode 100644 index 000000000..748d5a959 --- /dev/null +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -0,0 +1,236 @@ +package com.opensymphony.xwork2.ognl; + +import junit.framework.TestCase; + +import java.lang.reflect.Member; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.regex.Pattern; + +public class SecurityMemberAccessTest extends TestCase { + + private Map context; + private FooBar target; + + @Override + public void setUp() throws Exception { + context = new HashMap(); + target = new FooBar(); + } + + public void testWithoutClassExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "stringField"; + Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertTrue(accessible); + } + + public void testClassExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "stringField"; + Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); + + Set> excluded = new HashSet>(); + excluded.add(FooBar.class); + sma.setExcludedClasses(excluded); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse(accessible); + } + + public void testObjectClassExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "toString"; + Member member = FooBar.class.getMethod(propertyName); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("toString() from Object is accessible!!!", accessible); + } + + public void testObjectOverwrittenMethodsExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "hashCode"; + Member member = FooBar.class.getMethod(propertyName); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertTrue("hashCode() from FooBar isn't accessible!!!", accessible); + } + + public void testInterfaceInheritanceExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "barLogic"; + Member member = BarInterface.class.getMethod(propertyName); + + Set> excluded = new HashSet>(); + excluded.add(BarInterface.class); + sma.setExcludedClasses(excluded); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("barLogic() from BarInterface is accessible!!!", accessible); + } + + public void testMiddleOfInheritanceExclusion1() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "fooLogic"; + Member member = FooBar.class.getMethod(propertyName); + + Set> excluded = new HashSet>(); + excluded.add(BarInterface.class); + sma.setExcludedClasses(excluded); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertTrue("fooLogic() from FooInterface isn't accessible!!!", accessible); + } + + public void testMiddleOfInheritanceExclusion2() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "barLogic"; + Member member = BarInterface.class.getMethod(propertyName); + + Set> excluded = new HashSet>(); + excluded.add(BarInterface.class); + sma.setExcludedClasses(excluded); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("barLogic() from BarInterface is accessible!!!", accessible); + } + + public void testMiddleOfInheritanceExclusion3() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "barLogic"; + Member member = BarInterface.class.getMethod(propertyName); + +/* + Set> excluded = new HashSet>(); + excluded.add(BarInterface.class); + sma.setExcludedClasses(excluded); +*/ + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertTrue("barLogic() from BarInterface isn't accessible!!!", accessible); + } + + public void testMiddleOfInheritanceExclusion4() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + String propertyName = "barLogic"; + Member member = BarInterface.class.getMethod(propertyName); + + Set> excluded = new HashSet>(); + excluded.add(FooBarInterface.class); + sma.setExcludedClasses(excluded); + + // when + boolean accessible = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("barLogic() from BarInterface is accessible!!!", accessible); + } + + public void testPackageExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + Set excluded = new HashSet(); + excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); + sma.setExcludedPackageNamePatterns(excluded); + + String propertyName = "stringField"; + Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); + + // when + boolean actual = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("stringField is accessible!", actual); + } + +} + +class FooBar implements FooBarInterface { + + private String stringField; + + public String getStringField() { + return stringField; + } + + public void setStringField(String stringField) { + this.stringField = stringField; + } + + public String fooLogic() { + return "fooLogic"; + } + + public String barLogic() { + return "barLogic"; + } + + @Override + public int hashCode() { + return 1; + } + +} + +interface FooInterface { + + String fooLogic(); + +} + +interface BarInterface { + + String barLogic(); + +} + +interface FooBarInterface extends FooInterface, BarInterface { + +} \ No newline at end of file diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java new file mode 100644 index 000000000..c2c079b04 --- /dev/null +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java @@ -0,0 +1,56 @@ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.XWorkTestCase; + +import java.util.ArrayList; +import java.util.List; + +public class DefaultAcceptedPatternsCheckerTest extends XWorkTestCase { + + public void testHardcodedAcceptedPatterns() throws Exception { + // given + List params = new ArrayList() { + { + add("%{#application['test']}"); + add("%{#application.test}"); + add("%{#Application['test']}"); + add("%{#Application.test}"); + add("%{#session['test']}"); + add("%{#session.test}"); + add("%{#Session['test']}"); + add("%{#Session.test}"); + add("%{#struts['test']}"); + add("%{#struts.test}"); + add("%{#Struts['test']}"); + add("%{#Struts.test}"); + add("%{#request['test']}"); + add("%{#request.test}"); + add("%{#Request['test']}"); + add("%{#Request.test}"); + add("%{#servletRequest['test']}"); + add("%{#servletRequest.test}"); + add("%{#ServletRequest['test']}"); + add("%{#ServletRequest.test}"); + add("%{#servletResponse['test']}"); + add("%{#servletResponse.test}"); + add("%{#ServletResponse['test']}"); + add("%{#ServletResponse.test}"); + add("%{#parameters['test']}"); + add("%{#parameters.test}"); + add("%{#Parameters['test']}"); + add("%{#Parameters.test}"); + } + }; + + AcceptedPatternsChecker checker = new DefaultAcceptedPatternsChecker(); + + for (String param : params) { + // when + AcceptedPatternsChecker.IsAccepted actual = checker.isAccepted(param); + + // then + assertFalse("Access to " + param + " is possible!", actual.isAccepted()); + } + } + +} \ No newline at end of file diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java new file mode 100644 index 000000000..612552187 --- /dev/null +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsCheckerTest.java @@ -0,0 +1,60 @@ +package com.opensymphony.xwork2.security; + +import com.opensymphony.xwork2.XWorkTestCase; + +import java.util.ArrayList; +import java.util.List; + +public class DefaultExcludedPatternsCheckerTest extends XWorkTestCase { + + public void testHardcodedPatterns() throws Exception { + // given + List params = new ArrayList() { + { + add("%{#application['test']}"); + add("%{#application.test}"); + add("%{#Application['test']}"); + add("%{#Application.test}"); + add("%{#session['test']}"); + add("%{#session.test}"); + add("%{#Session['test']}"); + add("%{#Session.test}"); + add("%{#struts['test']}"); + add("%{#struts.test}"); + add("%{#Struts['test']}"); + add("%{#Struts.test}"); + add("%{#request['test']}"); + add("%{#request.test}"); + add("%{#Request['test']}"); + add("%{#Request.test}"); + add("%{#servletRequest['test']}"); + add("%{#servletRequest.test}"); + add("%{#ServletRequest['test']}"); + add("%{#ServletRequest.test}"); + add("%{#servletResponse['test']}"); + add("%{#servletResponse.test}"); + add("%{#ServletResponse['test']}"); + add("%{#ServletResponse.test}"); + add("%{#parameters['test']}"); + add("%{#parameters.test}"); + add("%{#Parameters['test']}"); + add("%{#Parameters.test}"); + add("#context.get('com.opensymphony.xwork2.dispatcher.HttpServletResponse')"); + add("%{#context.get('com.opensymphony.xwork2.dispatcher.HttpServletResponse')}"); + add("#_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true)"); + add("%{#_memberAccess[\"allowStaticMethodAccess\"]= new java.lang.Boolean(true)}"); + } + }; + + ExcludedPatternsChecker checker = new DefaultExcludedPatternsChecker(); + + for (String param : params) { + // when + ExcludedPatternsChecker.IsExcluded actual = checker.isExcluded(param); + + // then + assertTrue("Access to " + param + " is possible!", actual.isExcluded()); + } + } + +} \ No newline at end of file diff --git a/xwork-core/src/test/resources/xwork-class-param-test.xml b/xwork-core/src/test/resources/xwork-class-param-test.xml new file mode 100644 index 000000000..f12c08384 --- /dev/null +++ b/xwork-core/src/test/resources/xwork-class-param-test.xml @@ -0,0 +1,11 @@ + + + + + + + + + \ No newline at end of file diff --git a/xwork-core/src/test/resources/xwork-param-test.xml b/xwork-core/src/test/resources/xwork-param-test.xml index fa081c49f..01787f70e 100644 --- a/xwork-core/src/test/resources/xwork-param-test.xml +++ b/xwork-core/src/test/resources/xwork-param-test.xml @@ -4,4 +4,5 @@ + \ No newline at end of file diff --git a/xwork-core/src/test/resources/xwork-test-beans.xml b/xwork-core/src/test/resources/xwork-test-beans.xml index 3fa5b2838..1606f1d59 100644 --- a/xwork-core/src/test/resources/xwork-test-beans.xml +++ b/xwork-core/src/test/resources/xwork-test-beans.xml @@ -3,25 +3,7 @@ "http://struts.apache.org/dtds/xwork-2.0.dtd"> - - - + + + \ No newline at end of file