This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5181-static-methods in repository https://gitbox.apache.org/repos/asf/struts.git
commit 7541d9ab35bfdfb239325b6b05d2a7ebdbe91c75 Author: Lukasz Lenart <lukaszlen...@apache.org> AuthorDate: Thu May 5 12:12:10 2022 +0200 WW-5181 Blocks permanently access to static methods --- .../StrutsDefaultConfigurationProvider.java | 1 - .../com/opensymphony/xwork2/ognl/OgnlUtil.java | 8 +- .../opensymphony/xwork2/ognl/OgnlValueStack.java | 20 ++-- .../xwork2/ognl/OgnlValueStackFactory.java | 44 ++------ .../xwork2/ognl/SecurityMemberAccess.java | 80 +++++--------- .../xwork2/ognl/accessor/XWorkMethodAccessor.java | 12 +-- .../java/org/apache/struts2/StrutsConstants.java | 3 - .../struts2/config/entities/ConstantConfig.java | 10 -- .../interceptor/ParametersInterceptorTest.java | 15 ++- .../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 8 +- .../xwork2/ognl/OgnlValueStackTest.java | 120 +++------------------ .../xwork2/ognl/SecurityMemberAccessTest.java | 107 +++++++++--------- .../xwork2/ognl/SetPropertiesTest.java | 16 +-- .../ognl => test}/SecurityMemberAccessTest.java | 109 ++++++++++--------- .../com/test/TestSecurityMemberAccess.java} | 22 ++-- .../util/SecurityMemberAccessInServletsTest.java | 4 +- core/src/test/resources/xwork-test-beans.xml | 1 + .../xwork2/ognl/SecurityMemberAccessProxyTest.java | 4 +- .../SecurityMemberAccessProxyTest.java | 7 +- 19 files changed, 223 insertions(+), 368 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index f93237367..56dffabe0 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -233,7 +233,6 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider props.setProperty(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, Boolean.TRUE.toString()); props.setProperty(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, Boolean.FALSE.toString()); props.setProperty(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, Boolean.FALSE.toString()); props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Boolean.TRUE.toString()); props.setProperty(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE.toString()); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 007206cb8..4911dfa17 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -75,7 +75,6 @@ public class OgnlUtil { private Container container; private boolean allowStaticFieldAccess = true; - private boolean allowStaticMethodAccess; private boolean disallowProxyMemberAccess; public OgnlUtil() { @@ -212,11 +211,6 @@ public class OgnlUtil { this.allowStaticFieldAccess = BooleanUtils.toBoolean(allowStaticFieldAccess); } - @Inject(value = StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, required = false) - protected void setAllowStaticMethodAccess(String allowStaticMethodAccess) { - this.allowStaticMethodAccess = BooleanUtils.toBoolean(allowStaticMethodAccess); - } - @Inject(value = StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, required = false) protected void setDisallowProxyMemberAccess(String disallowProxyMemberAccess) { this.disallowProxyMemberAccess = BooleanUtils.toBoolean(disallowProxyMemberAccess); @@ -798,7 +792,7 @@ public class OgnlUtil { resolver = container.getInstance(CompoundRootAccessor.class); } - SecurityMemberAccess memberAccess = new SecurityMemberAccess(allowStaticMethodAccess, allowStaticFieldAccess); + SecurityMemberAccess memberAccess = new SecurityMemberAccess(allowStaticFieldAccess); memberAccess.setDisallowProxyMemberAccess(disallowProxyMemberAccess); if (devMode) { diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index c08da937e..1b874fc44 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java @@ -72,13 +72,13 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS private boolean devMode; private boolean logMissingProperties; - protected OgnlValueStack(XWorkConverter xworkConverter, CompoundRootAccessor accessor, TextProvider prov, boolean allowStaticMethodAccess, boolean allowStaticFieldAccess) { - setRoot(xworkConverter, accessor, new CompoundRoot(), allowStaticMethodAccess, allowStaticFieldAccess); + protected OgnlValueStack(XWorkConverter xworkConverter, CompoundRootAccessor accessor, TextProvider prov, boolean allowStaticFieldAccess) { + setRoot(xworkConverter, accessor, new CompoundRoot(), allowStaticFieldAccess); push(prov); } - protected OgnlValueStack(ValueStack vs, XWorkConverter xworkConverter, CompoundRootAccessor accessor, boolean allowStaticMethodAccess, boolean allowStaticFieldAccess) { - setRoot(xworkConverter, accessor, new CompoundRoot(vs.getRoot()), allowStaticMethodAccess, allowStaticFieldAccess); + protected OgnlValueStack(ValueStack vs, XWorkConverter xworkConverter, CompoundRootAccessor accessor, boolean allowStaticFieldAccess) { + setRoot(xworkConverter, accessor, new CompoundRoot(vs.getRoot()), allowStaticFieldAccess); } @Inject @@ -90,10 +90,9 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS securityMemberAccess.setDisallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess()); } - protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, - boolean allowStaticMethodAccess, boolean allowStaticFieldAccess) { + protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, boolean allowStaticFieldAccess) { this.root = compoundRoot; - this.securityMemberAccess = new SecurityMemberAccess(allowStaticMethodAccess, allowStaticFieldAccess); + this.securityMemberAccess = new SecurityMemberAccess(allowStaticFieldAccess); this.context = Ognl.createDefaultContext(this.root, securityMemberAccess, accessor, new OgnlTypeConverterWrapper(xworkConverter)); context.put(VALUE_STACK, this); ((OgnlContext) context).setTraceEvaluations(false); @@ -219,7 +218,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS if (shouldLog) { LOG.warn(msg, e); } - + if (throwExceptionOnFailure) { throw new StrutsException(msg, e); } @@ -462,11 +461,10 @@ 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"); - final boolean allowStaticMethod = BooleanUtils.toBoolean(cont.getInstance(String.class, StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS)); final boolean allowStaticField = BooleanUtils.toBoolean(cont.getInstance(String.class, StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS)); - OgnlValueStack aStack = new OgnlValueStack(xworkConverter, accessor, prov, allowStaticMethod, allowStaticField); + OgnlValueStack aStack = new OgnlValueStack(xworkConverter, accessor, prov, allowStaticField); aStack.setOgnlUtil(cont.getInstance(OgnlUtil.class)); - aStack.setRoot(xworkConverter, accessor, this.root, allowStaticMethod, allowStaticField); + aStack.setRoot(xworkConverter, accessor, this.root, allowStaticField); return aStack; } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java index 267a3c899..69dd54026 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java @@ -43,8 +43,6 @@ import java.util.Set; */ public class OgnlValueStackFactory implements ValueStackFactory { - private static final Logger LOG = LogManager.getLogger(OgnlValueStackFactory.class); - protected XWorkConverter xworkConverter; protected CompoundRootAccessor compoundRootAccessor; protected TextProvider textProvider; @@ -61,8 +59,7 @@ public class OgnlValueStackFactory implements ValueStackFactory { } public ValueStack createValueStack() { - ValueStack stack = new OgnlValueStack(xworkConverter, compoundRootAccessor, textProvider, - containerAllowsStaticMethodAccess(), containerAllowsStaticFieldAccess()); + ValueStack stack = new OgnlValueStack(xworkConverter, compoundRootAccessor, textProvider, containerAllowsStaticFieldAccess()); container.inject(stack); return stack.getActionContext() .withContainer(container) @@ -71,8 +68,7 @@ public class OgnlValueStackFactory implements ValueStackFactory { } public ValueStack createValueStack(ValueStack stack) { - ValueStack result = new OgnlValueStack(stack, xworkConverter, compoundRootAccessor, - containerAllowsStaticMethodAccess(), containerAllowsStaticFieldAccess()); + ValueStack result = new OgnlValueStack(stack, xworkConverter, compoundRootAccessor, containerAllowsStaticFieldAccess()); container.inject(result); return result.getActionContext() .withContainer(container) @@ -84,32 +80,23 @@ public class OgnlValueStackFactory implements ValueStackFactory { protected void setContainer(Container container) throws ClassNotFoundException { Set<String> names = container.getInstanceNames(PropertyAccessor.class); for (String name : names) { - Class cls = Class.forName(name); - if (cls != null) { - if (Map.class.isAssignableFrom(cls)) { - PropertyAccessor acc = container.getInstance(PropertyAccessor.class, name); - } - OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name)); - if (compoundRootAccessor == null && CompoundRoot.class.isAssignableFrom(cls)) { - compoundRootAccessor = (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, name); - } + Class<?> cls = Class.forName(name); + OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name)); + if (compoundRootAccessor == null && CompoundRoot.class.isAssignableFrom(cls)) { + compoundRootAccessor = (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, name); } } names = container.getInstanceNames(MethodAccessor.class); for (String name : names) { - Class cls = Class.forName(name); - if (cls != null) { - OgnlRuntime.setMethodAccessor(cls, container.getInstance(MethodAccessor.class, name)); - } + Class<?> cls = Class.forName(name); + OgnlRuntime.setMethodAccessor(cls, container.getInstance(MethodAccessor.class, name)); } names = container.getInstanceNames(NullHandler.class); for (String name : names) { - Class cls = Class.forName(name); - if (cls != null) { - OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(container.getInstance(NullHandler.class, name))); - } + Class<?> cls = Class.forName(name); + OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(container.getInstance(NullHandler.class, name))); } if (compoundRootAccessor == null) { throw new IllegalStateException("Couldn't find the compound root accessor"); @@ -117,19 +104,8 @@ public class OgnlValueStackFactory implements ValueStackFactory { this.container = container; } - /** - * Retrieve allowsStaticMethodAccess state from the container (allows for lazy fetching) - * - * @return - */ - protected boolean containerAllowsStaticMethodAccess() { - return BooleanUtils.toBoolean(container.getInstance(String.class, StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS)); - } - /** * Retrieve allowStaticFieldAccess state from the container (allows for lazy fetching) - * - * @return */ protected boolean containerAllowsStaticFieldAccess() { return BooleanUtils.toBoolean(container.getInstance(String.class, StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS)); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index af8705641..6dcdb5a1b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -42,7 +42,6 @@ public class SecurityMemberAccess implements MemberAccess { private static final Logger LOG = LogManager.getLogger(SecurityMemberAccess.class); private final boolean allowStaticFieldAccess; - private final boolean allowStaticMethodAccess; private Set<Pattern> excludeProperties = Collections.emptySet(); private Set<Pattern> acceptProperties = Collections.emptySet(); private Set<Class<?>> excludedClasses = Collections.emptySet(); @@ -52,25 +51,15 @@ public class SecurityMemberAccess implements MemberAccess { /** * SecurityMemberAccess - * - access decisions based on whether member is static (or not) - * - block or allow access to properties (configurable-after-construction) - * - * @param allowStaticMethodAccess - * @param allowStaticFieldAccess + * - access decisions based on whether member is static (or not) + * - block or allow access to properties (configurable-after-construction) + * + * @param allowStaticFieldAccess if set to true static fields (constants) will be accessible */ - public SecurityMemberAccess(boolean allowStaticMethodAccess, boolean allowStaticFieldAccess) { - this.allowStaticMethodAccess = allowStaticMethodAccess; + public SecurityMemberAccess(boolean allowStaticFieldAccess) { this.allowStaticFieldAccess = allowStaticFieldAccess; } - public final boolean getAllowStaticMethodAccess() { - return allowStaticMethodAccess; - } - - public final boolean getAllowStaticFieldAccess() { - return allowStaticFieldAccess; - } - @Override public Object setup(Map context, Object target, Member member, String propertyName) { Object result = null; @@ -90,13 +79,12 @@ public class SecurityMemberAccess implements MemberAccess { public void restore(Map context, Object target, Member member, String propertyName, Object state) { if (state != null) { final AccessibleObject accessible = (AccessibleObject) member; - final boolean stateboolean = ((Boolean) state).booleanValue(); // Using twice (avoid unboxing) - if (!stateboolean) { - accessible.setAccessible(stateboolean); - } - else { - throw new IllegalArgumentException("Improper restore state [" + stateboolean + "] for target [" + target + - "], member [" + member + "], propertyName [" + propertyName + "]"); + final boolean stateBoolean = ((Boolean) state).booleanValue(); // Using twice (avoid unboxing) + if (!stateBoolean) { + accessible.setAccessible(stateBoolean); + } else { + throw new IllegalArgumentException("Improper restore state [" + stateBoolean + "] for target [" + target + + "], member [" + member + "], propertyName [" + propertyName + "]"); } } } @@ -117,6 +105,7 @@ public class SecurityMemberAccess implements MemberAccess { return false; } + // it needs to be before calling #checkStaticMethodAccess() if (checkEnumAccess(target, member)) { LOG.trace("Allowing access to enum: target [{}], member [{}]", target, member); return true; @@ -127,7 +116,7 @@ public class SecurityMemberAccess implements MemberAccess { return false; } - final Class memberClass = member.getDeclaringClass(); + final Class<?> memberClass = member.getDeclaringClass(); if (isClassExcluded(memberClass)) { LOG.warn("Declaring class of member type [{}] is excluded!", member); @@ -135,11 +124,11 @@ public class SecurityMemberAccess implements MemberAccess { } // target can be null in case of accessing static fields, since OGNL 3.2.8 - final Class targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass(); + final Class<?> targetClass = Modifier.isStatic(memberModifiers) ? memberClass : target.getClass(); if (isPackageExcluded(targetClass.getPackage(), memberClass.getPackage())) { LOG.warn("Package [{}] of target class [{}] of target [{}] or package [{}] of member [{}] are excluded!", targetClass.getPackage(), targetClass, - target, memberClass.getPackage(), member); + target, memberClass.getPackage(), member); return false; } @@ -158,33 +147,25 @@ public class SecurityMemberAccess implements MemberAccess { /** * Check access for static method (via modifiers). - * + * * Note: For non-static members, the result is always true. - * + * * @param member * @param memberModifiers - * + * * @return */ protected boolean checkStaticMethodAccess(Member member, int memberModifiers) { - if (Modifier.isStatic(memberModifiers) && !(member instanceof Field)) { - if (allowStaticMethodAccess) { - LOG.debug("Support for accessing static methods [member: {}] is deprecated!", member); - } - return allowStaticMethodAccess; - } else { - return true; - } + return !Modifier.isStatic(memberModifiers) || member instanceof Field; } /** * Check access for static field (via modifiers). - * + * <p> * Note: For non-static members, the result is always true. - * + * * @param member * @param memberModifiers - * * @return */ protected boolean checkStaticFieldAccess(Member member, int memberModifiers) { @@ -195,13 +176,12 @@ public class SecurityMemberAccess implements MemberAccess { } } - /** + /** * Check access for public members (via modifiers) - * + * <p> * Returns true if-and-only-if the member is public. - * + * * @param memberModifiers - * * @return */ protected boolean checkPublicMemberAccess(int memberModifiers) { @@ -210,10 +190,8 @@ public class SecurityMemberAccess implements MemberAccess { protected boolean checkEnumAccess(Object target, Member member) { if (target instanceof Class) { - final Class clazz = (Class) target; - if (Enum.class.isAssignableFrom(clazz) && member.getName().equals("values")) { - return true; - } + final Class<?> clazz = (Class<?>) target; + return Enum.class.isAssignableFrom(clazz) && member.getName().equals("values"); } return false; } @@ -222,7 +200,7 @@ public class SecurityMemberAccess implements MemberAccess { if (targetPackage == null || memberPackage == null) { LOG.warn("The use of the default (unnamed) package is discouraged!"); } - + String targetPackageName = targetPackage == null ? "" : targetPackage.getName(); String memberPackageName = memberPackage == null ? "" : memberPackage.getName(); @@ -235,7 +213,7 @@ public class SecurityMemberAccess implements MemberAccess { targetPackageName = targetPackageName + "."; memberPackageName = memberPackageName + "."; - for (String packageName: excludedPackageNames) { + for (String packageName : excludedPackageNames) { if (targetPackageName.startsWith(packageName) || memberPackageName.startsWith(packageName)) { return true; } @@ -245,7 +223,7 @@ public class SecurityMemberAccess implements MemberAccess { } protected boolean isClassExcluded(Class<?> clazz) { - if (clazz == Object.class || (clazz == Class.class && !allowStaticMethodAccess)) { + if (clazz == Object.class || (clazz == Class.class && !allowStaticFieldAccess)) { return true; } for (Class<?> excludedClass : excludedClasses) { diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMethodAccessor.java b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMethodAccessor.java index af4608b3e..180e787f1 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMethodAccessor.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/XWorkMethodAccessor.java @@ -36,7 +36,7 @@ import java.util.Map; * @author tmjee */ public class XWorkMethodAccessor extends ObjectMethodAccessor { - + private static final Logger LOG = LogManager.getLogger(XWorkMethodAccessor.class); @Override @@ -58,9 +58,9 @@ public class XWorkMethodAccessor extends ObjectMethodAccessor { //so that property strings are not cleared //i.e. OgnlUtil should be used initially, OgnlRuntime //thereafter - + Object propVal=OgnlRuntime.getProperty(ogContext, object, string); - //use the Collection property accessor instead of the individual property accessor, because + //use the Collection property accessor instead of the individual property accessor, because //in the case of Lists otherwise the index property could be used PropertyAccessor accessor=OgnlRuntime.getPropertyAccessor(Collection.class); ReflectionContextState.setGettingByKeyProperty(ogContext,true); @@ -83,8 +83,8 @@ public class XWorkMethodAccessor extends ObjectMethodAccessor { return callMethodWithDebugInfo(context, object, string, objects); } } - Boolean exec = (Boolean) context.get(ReflectionContextState.DENY_METHOD_EXECUTION); - boolean e = ((exec == null) ? false : exec.booleanValue()); + Boolean exec = ReflectionContextState.isDenyMethodExecution(context); + boolean e = (exec != null && exec); if (!e) { return callMethodWithDebugInfo(context, object, string, objects); @@ -110,7 +110,7 @@ public class XWorkMethodAccessor extends ObjectMethodAccessor { @Override public Object callStaticMethod(Map context, Class aClass, String string, Object[] objects) throws MethodFailedException { - Boolean exec = (Boolean) context.get(ReflectionContextState.DENY_METHOD_EXECUTION); + Boolean exec = ReflectionContextState.isDenyMethodExecution(context); boolean e = ((exec == null) ? false : exec.booleanValue()); if (!e) { diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 64ac93b04..64849d798 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -226,9 +226,6 @@ public final class StrutsConstants { /** The name of the parameter to determine whether static field access will be allowed in OGNL expressions or not */ public static final String STRUTS_ALLOW_STATIC_FIELD_ACCESS = "struts.ognl.allowStaticFieldAccess"; - /** The name of the parameter to determine whether static method access will be allowed in OGNL expressions or not */ - public static final String STRUTS_ALLOW_STATIC_METHOD_ACCESS = "struts.ognl.allowStaticMethodAccess"; - /** The com.opensymphony.xwork2.validator.ActionValidatorManager implementation class */ public static final String STRUTS_ACTIONVALIDATORMANAGER = "struts.actionValidatorManager"; diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 013608216..2021ba019 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -90,7 +90,6 @@ public class ConstantConfig { private BeanConfig localeProviderFactory; private String mapperIdParameterName; private Boolean ognlAllowStaticFieldAccess; - private Boolean ognlAllowStaticMethodAccess; private BeanConfig actionValidatorManager; private BeanConfig valueStackFactory; private BeanConfig reflectionProvider; @@ -220,7 +219,6 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_LOCALE_PROVIDER_FACTORY, beanConfToString(localeProviderFactory)); map.put(StrutsConstants.STRUTS_ID_PARAMETER_NAME, mapperIdParameterName); map.put(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Objects.toString(ognlAllowStaticFieldAccess, null)); - map.put(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, Objects.toString(ognlAllowStaticMethodAccess, null)); map.put(StrutsConstants.STRUTS_ACTIONVALIDATORMANAGER, beanConfToString(actionValidatorManager)); map.put(StrutsConstants.STRUTS_VALUESTACKFACTORY, beanConfToString(valueStackFactory)); map.put(StrutsConstants.STRUTS_REFLECTIONPROVIDER, beanConfToString(reflectionProvider)); @@ -806,14 +804,6 @@ public class ConstantConfig { this.ognlAllowStaticFieldAccess = ognlAllowStaticFieldAccess; } - public Boolean getOgnlAllowStaticMethodAccess() { - return ognlAllowStaticMethodAccess; - } - - public void setOgnlAllowStaticMethodAccess(Boolean ognlAllowStaticMethodAccess) { - this.ognlAllowStaticMethodAccess = ognlAllowStaticMethodAccess; - } - public BeanConfig getActionValidatorManager() { return actionValidatorManager; } diff --git a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java index 515b7cffe..69d9b03a6 100644 --- a/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java @@ -39,13 +39,17 @@ 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 com.opensymphony.xwork2.util.reflection.ReflectionContextState; import ognl.OgnlContext; import ognl.PropertyAccessor; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.dispatcher.HttpParameters; import org.junit.Assert; +import org.springframework.ejb.access.SimpleRemoteStatelessSessionProxyFactoryBean; import java.io.File; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -335,8 +339,8 @@ public class ParametersInterceptorTest extends XWorkTestCase { // given Map<String, Object> params = new HashMap<>(); params.put("blah", "This is blah"); - params.put("('\\u0023_memberAccess[\\'allowStaticMethodAccess\\']')(meh)", "true"); - params.put("('(aaa)(('\\u0023context[\\'xwork.MethodAccessor.denyMethodExecution\\']\\u003d\\u0023foo')(\\u0023foo\\u003dnew java.lang.Boolean(\"false\")))", ""); + params.put("('\\u0023_memberAccess[\\'allowStaticFieldAccess\\']')(meh)", "true"); + params.put("('(aaa)(('\\u0023context[\\'xwork.MethodAccessor.denyMethodExecution\\']\\u003d\\u0023foo')(\\u0023foo\\u003dnew java.lang.Boolean(\"true\")))", ""); params.put("(asdf)(('\\u0023rt.exit(1)')(\\u0023rt\\u0...@java.lang.Runtime@getRuntime()))", "1"); HashMap<String, Object> extraContext = new HashMap<>(); @@ -351,8 +355,9 @@ public class ParametersInterceptorTest extends XWorkTestCase { //then assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah()); - boolean allowMethodAccess = ((SecurityMemberAccess) ((OgnlContext) stack.getContext()).getMemberAccess()).getAllowStaticMethodAccess(); - assertFalse(allowMethodAccess); + Field field = ReflectionContextState.class.getField("DENY_METHOD_EXECUTION"); + boolean allowStaticFieldAccess = ((OgnlContext) stack.getContext()).getMemberAccess().isAccessible(stack.getContext(), proxy.getAction(), field, ""); + assertFalse(allowStaticFieldAccess); } public void testParameters() throws Exception { @@ -806,7 +811,7 @@ public class ParametersInterceptorTest extends XWorkTestCase { ValueStack stack = new OgnlValueStack( container.getInstance(XWorkConverter.class), (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), - container.getInstance(TextProvider.class, "system"), true, true) { + container.getInstance(TextProvider.class, "system"), true) { @Override public void setValue(String expr, Object value) { actual.put(expr, value); diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 4b2f1ee83..51d9c1268 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1215,7 +1215,7 @@ public class OgnlUtilTest extends XWorkTestCase { Object accessedValue; try { - reloadTestContainerConfiguration(true); // Test with allowStaticFieldAccess true + reloadTestContainerConfiguration(true, true); // Test with allowStaticFieldAccess true context = ognlUtil.createDefaultContext(null); } catch (Exception ex) { fail("unable to reload test configuration? Exception: " + ex); @@ -1270,7 +1270,7 @@ public class OgnlUtilTest extends XWorkTestCase { } try { - reloadTestContainerConfiguration(false); // Re-test with allowStaticFieldAccess false + reloadTestContainerConfiguration(true, false); // Re-test with allowStaticFieldAccess false context = ognlUtil.createDefaultContext(null); } catch (Exception ex) { fail("unable to reload test configuration? Exception: " + ex); @@ -1525,13 +1525,13 @@ public class OgnlUtilTest extends XWorkTestCase { } } - private void reloadTestContainerConfiguration(boolean devMode, boolean allowStaticMethod) { + private void reloadTestContainerConfiguration(boolean devMode, boolean allowStaticFieldAccess) { loadConfigurationProviders(new StubConfigurationProvider() { @Override public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { props.setProperty(StrutsConstants.STRUTS_DEVMODE, "" + devMode); - props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, "" + allowStaticMethod); + props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, "" + allowStaticFieldAccess); } }); ognlUtil = container.getInstance(OgnlUtil.class); diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java index b5704c321..37ef14be4 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java @@ -85,16 +85,15 @@ public class OgnlValueStackTest extends XWorkTestCase { } private OgnlValueStack createValueStack() { - return createValueStack(true, true); + return createValueStack(true); } - private OgnlValueStack createValueStack(boolean allowStaticMethodAccess, boolean allowStaticFieldAccess) { + private OgnlValueStack createValueStack(boolean allowStaticFieldAccess) { OgnlValueStack stack = new OgnlValueStack( container.getInstance(XWorkConverter.class), (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), - container.getInstance(TextProvider.class, "system"), allowStaticMethodAccess, allowStaticFieldAccess); + container.getInstance(TextProvider.class, "system"), allowStaticFieldAccess); container.inject(stack); - ognlUtil.setAllowStaticMethodAccess(Boolean.toString(allowStaticMethodAccess)); ognlUtil.setAllowStaticFieldAccess(Boolean.toString(allowStaticFieldAccess)); return stack; } @@ -111,14 +110,13 @@ public class OgnlValueStackTest extends XWorkTestCase { * Intended for testing OgnlValueStack instance(s) that are minimally configured. * This should help ensure no underlying configuration/injection side-effects are responsible * for the behaviour of fundamental access control flags). - * - * @param allowStaticMethod new allowStaticMethod configuration + * * @param allowStaticField new allowStaticField configuration * @return a new OgnlValueStackFactory with specified new configuration */ - private OgnlValueStackFactory reloadValueStackFactory(Boolean allowStaticMethod, Boolean allowStaticField) { + private OgnlValueStackFactory reloadValueStackFactory(Boolean allowStaticField) { try { - reloadTestContainerConfiguration(allowStaticMethod, allowStaticField); + reloadTestContainerConfiguration(allowStaticField); } catch (Exception ex) { fail("Unable to reload container configuration and configure ognlValueStackFactory - exception: " + ex); @@ -502,7 +500,7 @@ public class OgnlValueStackTest extends XWorkTestCase { Dog dog = new Dog(); dog.setDeity("fido"); vs.push(dog); - assertEquals("fido", vs.findValue("@com.opensymphony.xwork2.util.Dog@getDeity()", String.class)); + assertNull(vs.findValue("@com.opensymphony.xwork2.util.Dog@getDeity()", String.class)); } /** @@ -517,7 +515,7 @@ public class OgnlValueStackTest extends XWorkTestCase { } public void testStaticMethodDisallow() { - OgnlValueStack vs = createValueStack(false, true); + OgnlValueStack vs = createValueStack(true); Dog dog = new Dog(); dog.setDeity("fido"); @@ -1188,7 +1186,7 @@ public class OgnlValueStackTest extends XWorkTestCase { OgnlValueStack stack2 = new OgnlValueStack(stack, container.getInstance(XWorkConverter.class), - (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), true, true); + (CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), true); container.inject(stack2); assertEquals(stack.getRoot(), stack2.getRoot()); @@ -1271,7 +1269,6 @@ public class OgnlValueStackTest extends XWorkTestCase { // An OgnlValueStackFactory using a container config with default (from XWorkConfigurationProvider) // static access flag values present should prevent staticMethodAccess but allow staticFieldAccess. - assertFalse("OgnlValueStackFactory staticMethodAccess (default flags) not false?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); assertTrue("OgnlValueStackFactory staticFieldAccess (default flags) not true?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); // An OgnlValueStack created from the above OgnlValueStackFactory should allow public field access, // but prevent non-public field access. It should also deny static method access. @@ -1300,7 +1297,7 @@ public class OgnlValueStackTest extends XWorkTestCase { * when no static access flags are set (not present in configuration). */ public void testOgnlValueStackFromOgnlValueStackFactoryNoFlagsSet() { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(null, null); + OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(null); OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); Object accessedValue; @@ -1309,7 +1306,6 @@ public class OgnlValueStackTest extends XWorkTestCase { // prevent staticMethodAccess AND prevent staticFieldAccess. // Note: Under normal circumstances, explicit static access configuration flags should be present, // but this specific check verifies what happens if those configuration flags are not present. - assertFalse("OgnlValueStackFactory staticMethodAccess (no flag present) not false?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); assertFalse("OgnlValueStackFactory staticFieldAccess (no flag present) not false?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); // An OgnlValueStack created from the above OgnlValueStackFactory should prevent public field access, // and prevent non-public field access. It should also deny static method access. @@ -1335,16 +1331,15 @@ public class OgnlValueStackTest extends XWorkTestCase { /** * Test a raw OgnlValueStackFactory and OgnlValueStack generated by it - * when both static access flags are set to false. + * when static access flag is set to false. */ public void testOgnlValueStackFromOgnlValueStackFactoryNoStaticAccess() { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(false, false); + OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(false); OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); Object accessedValue; // An OgnlValueStackFactory using a container config with both static access flags set false should // prevent staticMethodAccess AND prevent staticFieldAccess. - assertFalse("OgnlValueStackFactory staticMethodAccess (set false) not false?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not false?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); // An OgnlValueStack created from the above OgnlValueStackFactory should prevent public field access, // and prevent non-public field access. It should also deny static method access. @@ -1370,22 +1365,20 @@ public class OgnlValueStackTest extends XWorkTestCase { /** * Test a raw OgnlValueStackFactory and OgnlValueStack generated by it - * when both static access flags are set to true. + * when static access flag is set to true. */ public void testOgnlValueStackFromOgnlValueStackFactoryAllStaticAccess() { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(true, true); + OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(true); OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); Object accessedValue; // An OgnlValueStackFactory using a container config with both static access flags set true should // allow both staticMethodAccess AND staticFieldAccess. - assertTrue("OgnlValueStackFactory staticMethodAccess (set true) not true?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not true?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); // An OgnlValueStack created from the above OgnlValueStackFactory should allow public field access, // but prevent non-public field access. It should also allow static method access. accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); - assertNotNull("unable to access static method (result null) ?", accessedValue); - assertEquals("accessed static method result not equal to expected?", accessedValue, staticInteger100Method()); + assertNull("able to access static method (result non-null)!!!", accessedValue); accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); assertEquals("accessed static final public field value not equal to actual?", accessedValue, STATIC_FINAL_PUBLIC_ATTRIBUTE); accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); @@ -1404,93 +1397,14 @@ public class OgnlValueStackTest extends XWorkTestCase { assertNull("accessed private field (result not null) ?", accessedValue); } - /** - * Test a raw OgnlValueStackFactory and OgnlValueStack generated by it - * when static method access flag is true, static field access flag is false. - */ - public void testOgnlValueStackFromOgnlValueStackFactoryOnlyStaticMethodAccess() { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(true, false); - OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); - Object accessedValue; - - // An OgnlValueStackFactory using a container config with static method access flag true, static field access false should - // allow staticMethodAccess but deny staticFieldAccess. - assertTrue("OgnlValueStackFactory staticMethodAccess (set true) not true?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); - assertFalse("OgnlValueStackFactory staticFieldAccess (set false) not false?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); - // An OgnlValueStack created from the above OgnlValueStackFactory should deny public field access, - // and also prevent non-public field access. It should also allow static method access. - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); - assertNotNull("unable to access static method (result null) ?", accessedValue); - assertEquals("accessed static method result not equal to expected?", accessedValue, staticInteger100Method()); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); - assertNull("able to access static final public field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); - assertNull("able to access static public field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); - assertNull("accessed final package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); - assertNull("accessed package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); - assertNull("accessed final protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); - assertNull("accessed protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); - assertNull("accessed final private field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); - assertNull("accessed private field (result not null) ?", accessedValue); - } - - /** - * Test a raw OgnlValueStackFactory and OgnlValueStack generated by it - * when static method access flag is false, static field access flag is true. - */ - public void testOgnlValueStackFromOgnlValueStackFactoryOnlyStaticFieldAccess() { - OgnlValueStackFactory ognlValueStackFactory = reloadValueStackFactory(false, true); - OgnlValueStack ognlValueStack = (OgnlValueStack) ognlValueStackFactory.createValueStack(); - Object accessedValue; - - // An OgnlValueStackFactory using a container config with static method access flag false, static field access true should - // deny staticMethodAccess but allow staticFieldAccess. - assertFalse("OgnlValueStackFactory staticMethodAccess (set false) not false?", ognlValueStackFactory.containerAllowsStaticMethodAccess()); - assertTrue("OgnlValueStackFactory staticFieldAccess (set true) not true?", ognlValueStackFactory.containerAllowsStaticFieldAccess()); - // An OgnlValueStack created from the above OgnlValueStackFactory should allow public field access, - // but prevent non-public field access. It should also deny static method access. - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@staticInteger100Method()"); - assertNull("able to access static method (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PUBLIC_ATTRIBUTE"); - assertEquals("accessed static final public field value not equal to actual?", accessedValue, STATIC_FINAL_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PUBLIC_ATTRIBUTE"); - assertEquals("accessed static public field value not equal to actual?", accessedValue, STATIC_PUBLIC_ATTRIBUTE); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PACKAGE_ATTRIBUTE"); - assertNull("accessed final package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PACKAGE_ATTRIBUTE"); - assertNull("accessed package field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PROTECTED_ATTRIBUTE"); - assertNull("accessed final protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PROTECTED_ATTRIBUTE"); - assertNull("accessed protected field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_FINAL_PRIVATE_ATTRIBUTE"); - assertNull("accessed final private field (result not null) ?", accessedValue); - accessedValue = ognlValueStack.findValue("@com.opensymphony.xwork2.ognl.OgnlValueStackTest@STATIC_PRIVATE_ATTRIBUTE"); - assertNull("accessed private field (result not null) ?", accessedValue); - } - - private void reloadTestContainerConfiguration(Boolean allowStaticMethod, Boolean allowStaticField) throws Exception { + private void reloadTestContainerConfiguration(Boolean allowStaticField) throws Exception { loadConfigurationProviders(new StubConfigurationProvider() { @Override public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { // null values simulate undefined (by removing). // undefined values then should be evaluated to false - if (props.containsKey(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS)) { - props.remove(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS); - } - if (props.containsKey(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS)) { - props.remove(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS); - } - if (allowStaticMethod != null) { - props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_METHOD_ACCESS, "" + allowStaticMethod); - } + props.remove(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS); if (allowStaticField != null) { props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, "" + allowStaticField); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java index 41c1ecb84..19abfae09 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -23,7 +23,6 @@ import junit.framework.TestCase; import java.lang.reflect.Field; import java.lang.reflect.Member; -import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -45,7 +44,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testWithoutClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "stringField"; Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); @@ -59,7 +58,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "stringField"; Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); @@ -77,7 +76,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testObjectClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "toString"; Member member = FooBar.class.getMethod(propertyName); @@ -91,7 +90,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testObjectOverwrittenMethodsExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "hashCode"; Member member = FooBar.class.getMethod(propertyName); @@ -105,7 +104,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testInterfaceInheritanceExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -123,7 +122,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion1() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "fooLogic"; Member member = FooBar.class.getMethod(propertyName); @@ -141,7 +140,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion3() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -155,7 +154,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion4() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -173,7 +172,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testPackageExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); @@ -188,10 +187,10 @@ public class SecurityMemberAccessTest extends TestCase { // then assertFalse("stringField is accessible!", actual); } - + public void testPackageNameExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<String> excluded = new HashSet<>(); excluded.add(FooBar.class.getPackage().getName()); @@ -209,27 +208,27 @@ public class SecurityMemberAccessTest extends TestCase { public void testDefaultPackageExclusion() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); sma.setExcludedPackageNamePatterns(excluded); - + // when boolean actual = sma.isPackageExcluded(null, null); // then assertFalse("default package is excluded!", actual); } - + public void testDefaultPackageExclusion2() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^$")); sma.setExcludedPackageNamePatterns(excluded); - + // when boolean actual = sma.isPackageExcluded(null, null); @@ -239,7 +238,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessEnum() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); // when Member values = MyValues.class.getMethod("values"); @@ -249,23 +248,23 @@ public class SecurityMemberAccessTest extends TestCase { assertTrue("Access to enums is blocked!", actual); } - public void testAccessStatic() throws Exception { + public void testAccessStaticMethod() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getMethod("sayHello"); boolean actual = sma.isAccessible(context, Class.class, method, null); // then - assertTrue("Access to static is blocked!", actual); + assertFalse("Access to static method is not blocked!", actual); } public void testAccessStaticField() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -277,8 +276,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -289,8 +288,8 @@ public class SecurityMemberAccessTest extends TestCase { // public static final test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.class.getField("MIN_VALUE"); @@ -301,8 +300,8 @@ public class SecurityMemberAccessTest extends TestCase { // package static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PACKAGE_STRING"); @@ -313,8 +312,8 @@ public class SecurityMemberAccessTest extends TestCase { // package final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PACKAGE_STRING"); @@ -325,8 +324,8 @@ public class SecurityMemberAccessTest extends TestCase { // protected static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PROTECTED_STRING"); @@ -337,8 +336,8 @@ public class SecurityMemberAccessTest extends TestCase { // protected final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PROTECTED_STRING"); @@ -349,8 +348,8 @@ public class SecurityMemberAccessTest extends TestCase { // private static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PRIVATE_STRING"); @@ -361,8 +360,8 @@ public class SecurityMemberAccessTest extends TestCase { // private final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PRIVATE_STRING"); @@ -374,7 +373,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedClasses(new HashSet<>(Arrays.asList(Class.class, StaticTester.class))); // when @@ -385,10 +384,10 @@ public class SecurityMemberAccessTest extends TestCase { assertFalse("Access to static field isn't blocked!", actual); } - public void testBlockStaticAccess() throws Exception { + public void testBlockStaticMethodAccess() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getMethod("sayHello"); @@ -400,8 +399,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockStaticAccessIfClassIsExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -413,8 +412,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testAllowStaticAccessIfClassIsNotExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(ClassLoader.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -426,7 +425,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveInt() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.,ognl,javax")); String propertyName = "intField"; @@ -441,7 +440,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithNames() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("ognl.,javax.")); @@ -493,7 +492,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithPackageRegExs() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> patterns = new HashSet<>(); patterns.add(Pattern.compile("^java\\.lang\\..*")); sma.setExcludedPackageNamePatterns(patterns); @@ -510,7 +509,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsAccessible() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Class<?>> excluded = new HashSet<>(); excluded.add(ognl.MemberAccess.class); sma.setExcludedClasses(excluded); @@ -528,7 +527,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsBlocked() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Class<?>> excluded = new HashSet<>(); excluded.add(SecurityMemberAccess.class); sma.setExcludedClasses(excluded); @@ -546,7 +545,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testPackageNameExclusionAsCommaDelimited() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.")); diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java index 3d7ed9778..cbd1f519e 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SetPropertiesTest.java @@ -46,9 +46,9 @@ import java.util.*; * @author tm_jee */ public class SetPropertiesTest extends XWorkTestCase { - + private OgnlUtil ognlUtil; - + @Override public void setUp() throws Exception { super.setUp(); @@ -57,7 +57,7 @@ public class SetPropertiesTest extends XWorkTestCase { } public void testOgnlUtilEmptyStringAsLong() { Bar bar = new Bar(); - Map context = Ognl.createDefaultContext(bar, new SecurityMemberAccess(false, true)); + Map context = Ognl.createDefaultContext(bar, new SecurityMemberAccess(true)); context.put(XWorkConverter.REPORT_CONVERSION_ERRORS, Boolean.TRUE); bar.setId(null); @@ -110,7 +110,7 @@ public class SetPropertiesTest extends XWorkTestCase { assertEquals(Cat.class, foo.getCats().get(0).getClass()); assertEquals(Cat.class, foo.getCats().get(1).getClass()); } - + public void testValueStackSetValueEmptyStringAsLong() { Bar bar = new Bar(); ValueStack vs = ActionContext.getContext().getValueStack(); @@ -193,11 +193,11 @@ public class SetPropertiesTest extends XWorkTestCase { } - + public void testAddingToMapsWithObjectsTrue() throws Exception { doTestAddingToMapsWithObjects(true); } - + public void testAddingToMapsWithObjectsFalse() throws Exception { doTestAddingToMapsWithObjects(false); @@ -227,8 +227,8 @@ public class SetPropertiesTest extends XWorkTestCase { } - - + + public void testAddingAndModifyingCollectionWithObjectsSet() { doTestAddingAndModifyingCollectionWithObjects(new HashSet()); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/com/test/SecurityMemberAccessTest.java similarity index 84% copy from core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java copy to core/src/test/java/com/test/SecurityMemberAccessTest.java index 41c1ecb84..e823cda72 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/test/SecurityMemberAccessTest.java @@ -16,14 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -package com.opensymphony.xwork2.ognl; +package com.test; +import com.opensymphony.xwork2.ognl.SecurityMemberAccess; import com.opensymphony.xwork2.util.TextParseUtil; import junit.framework.TestCase; import java.lang.reflect.Field; import java.lang.reflect.Member; -import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -45,7 +45,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testWithoutClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "stringField"; Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); @@ -59,7 +59,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "stringField"; Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); @@ -77,7 +77,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testObjectClassExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "toString"; Member member = FooBar.class.getMethod(propertyName); @@ -91,7 +91,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testObjectOverwrittenMethodsExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "hashCode"; Member member = FooBar.class.getMethod(propertyName); @@ -105,7 +105,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testInterfaceInheritanceExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -123,7 +123,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion1() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "fooLogic"; Member member = FooBar.class.getMethod(propertyName); @@ -141,7 +141,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion3() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -155,7 +155,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testMiddleOfInheritanceExclusion4() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); @@ -173,7 +173,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testPackageExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); @@ -188,10 +188,10 @@ public class SecurityMemberAccessTest extends TestCase { // then assertFalse("stringField is accessible!", actual); } - + public void testPackageNameExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<String> excluded = new HashSet<>(); excluded.add(FooBar.class.getPackage().getName()); @@ -209,27 +209,27 @@ public class SecurityMemberAccessTest extends TestCase { public void testDefaultPackageExclusion() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + TestSecurityMemberAccess sma = new TestSecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); sma.setExcludedPackageNamePatterns(excluded); - + // when boolean actual = sma.isPackageExcluded(null, null); // then assertFalse("default package is excluded!", actual); } - + public void testDefaultPackageExclusion2() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + TestSecurityMemberAccess sma = new TestSecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^$")); sma.setExcludedPackageNamePatterns(excluded); - + // when boolean actual = sma.isPackageExcluded(null, null); @@ -239,7 +239,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessEnum() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); // when Member values = MyValues.class.getMethod("values"); @@ -249,23 +249,23 @@ public class SecurityMemberAccessTest extends TestCase { assertTrue("Access to enums is blocked!", actual); } - public void testAccessStatic() throws Exception { + public void testAccessStaticMethod() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getMethod("sayHello"); boolean actual = sma.isAccessible(context, Class.class, method, null); // then - assertTrue("Access to static is blocked!", actual); + assertFalse("Access to static method is not blocked!", actual); } public void testAccessStaticField() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -277,8 +277,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -289,8 +289,8 @@ public class SecurityMemberAccessTest extends TestCase { // public static final test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.class.getField("MIN_VALUE"); @@ -301,8 +301,8 @@ public class SecurityMemberAccessTest extends TestCase { // package static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PACKAGE_STRING"); @@ -313,8 +313,8 @@ public class SecurityMemberAccessTest extends TestCase { // package final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PACKAGE_STRING"); @@ -325,8 +325,8 @@ public class SecurityMemberAccessTest extends TestCase { // protected static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PROTECTED_STRING"); @@ -337,8 +337,8 @@ public class SecurityMemberAccessTest extends TestCase { // protected final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PROTECTED_STRING"); @@ -349,8 +349,8 @@ public class SecurityMemberAccessTest extends TestCase { // private static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PRIVATE_STRING"); @@ -361,8 +361,8 @@ public class SecurityMemberAccessTest extends TestCase { // private final static test // given - sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PRIVATE_STRING"); @@ -374,7 +374,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedClasses(new HashSet<>(Arrays.asList(Class.class, StaticTester.class))); // when @@ -387,8 +387,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockStaticAccess() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getMethod("sayHello"); @@ -400,8 +400,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testBlockStaticAccessIfClassIsExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(Class.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -413,8 +413,8 @@ public class SecurityMemberAccessTest extends TestCase { public void testAllowStaticAccessIfClassIsNotExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(true, true); - sma.setExcludedClasses(new HashSet<Class<?>>(Collections.singletonList(ClassLoader.class))); + SecurityMemberAccess sma = new SecurityMemberAccess(true); + sma.setExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -426,7 +426,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveInt() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.,ognl,javax")); String propertyName = "intField"; @@ -441,7 +441,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithNames() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("ognl.,javax.")); @@ -493,7 +493,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithPackageRegExs() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> patterns = new HashSet<>(); patterns.add(Pattern.compile("^java\\.lang\\..*")); sma.setExcludedPackageNamePatterns(patterns); @@ -510,7 +510,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsAccessible() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Class<?>> excluded = new HashSet<>(); excluded.add(ognl.MemberAccess.class); sma.setExcludedClasses(excluded); @@ -528,7 +528,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsBlocked() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Class<?>> excluded = new HashSet<>(); excluded.add(SecurityMemberAccess.class); sma.setExcludedClasses(excluded); @@ -546,8 +546,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testPackageNameExclusionAsCommaDelimited() { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); - + TestSecurityMemberAccess sma = new TestSecurityMemberAccess(true); sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.")); diff --git a/core/src/test/resources/xwork-test-beans.xml b/core/src/test/java/com/test/TestSecurityMemberAccess.java similarity index 65% copy from core/src/test/resources/xwork-test-beans.xml copy to core/src/test/java/com/test/TestSecurityMemberAccess.java index 2ed15b282..95d8efacd 100644 --- a/core/src/test/resources/xwork-test-beans.xml +++ b/core/src/test/java/com/test/TestSecurityMemberAccess.java @@ -1,5 +1,3 @@ -<?xml version="1.0" encoding="UTF-8"?> -<!-- /* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -18,12 +16,18 @@ * specific language governing permissions and limitations * under the License. */ ---> -<!DOCTYPE struts PUBLIC - "-//Apache Software Foundation//DTD Struts Configuration 2.5//EN" - "http://struts.apache.org/dtds/struts-2.5.dtd"> -<struts> +package com.test; - <constant name="struts.excludedClasses" value="java.lang.Object,java.lang.Runtime,ognl.OgnlContext,ognl.MemberAccess,ognl.ClassResolver,ognl.TypeConverter,com.opensymphony.xwork2.ognl.SecurityMemberAccess" /> +import com.opensymphony.xwork2.ognl.SecurityMemberAccess; -</struts> +class TestSecurityMemberAccess extends SecurityMemberAccess { + + TestSecurityMemberAccess(boolean allowStaticFieldAccess) { + super(allowStaticFieldAccess); + } + + @Override + public boolean isPackageExcluded(Package targetPackage, Package memberPackage) { + return super.isPackageExcluded(targetPackage, memberPackage); + } +} diff --git a/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java b/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java index bf29a33fd..b5251a5e7 100644 --- a/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java +++ b/core/src/test/java/org/apache/struts2/util/SecurityMemberAccessInServletsTest.java @@ -41,7 +41,7 @@ public class SecurityMemberAccessInServletsTest extends StrutsInternalTestCase { public void testJavaxServletPackageAccess() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<Pattern>(); excluded.add(Pattern.compile("^(?!javax\\.servlet\\..+)(javax\\..+)")); @@ -59,7 +59,7 @@ public class SecurityMemberAccessInServletsTest extends StrutsInternalTestCase { public void testJavaxServletPackageExclusion() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Set<Pattern> excluded = new HashSet<Pattern>(); excluded.add(Pattern.compile("^javax\\..+")); diff --git a/core/src/test/resources/xwork-test-beans.xml b/core/src/test/resources/xwork-test-beans.xml index 2ed15b282..92e673d6b 100644 --- a/core/src/test/resources/xwork-test-beans.xml +++ b/core/src/test/resources/xwork-test-beans.xml @@ -26,4 +26,5 @@ <constant name="struts.excludedClasses" value="java.lang.Object,java.lang.Runtime,ognl.OgnlContext,ognl.MemberAccess,ognl.ClassResolver,ognl.TypeConverter,com.opensymphony.xwork2.ognl.SecurityMemberAccess" /> + <constant name="struts.ognl.allowStaticFieldAccess" value="false"/> </struts> diff --git a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java index eff352b22..d51cc771c 100644 --- a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java @@ -45,7 +45,7 @@ public class SecurityMemberAccessProxyTest extends XWorkTestCase { ActionProxy proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setDisallowProxyMemberAccess(true); Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); @@ -58,7 +58,7 @@ public class SecurityMemberAccessProxyTest extends XWorkTestCase { ActionProxy proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); diff --git a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java b/plugins/spring/src/test/java/com/test/SecurityMemberAccessProxyTest.java similarity index 92% copy from plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java copy to plugins/spring/src/test/java/com/test/SecurityMemberAccessProxyTest.java index eff352b22..1d898fc26 100644 --- a/plugins/spring/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessProxyTest.java +++ b/plugins/spring/src/test/java/com/test/SecurityMemberAccessProxyTest.java @@ -16,11 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -package com.opensymphony.xwork2.ognl; +package com.test; import com.opensymphony.xwork2.ActionProxy; import com.opensymphony.xwork2.XWorkTestCase; import com.opensymphony.xwork2.config.providers.XmlConfigurationProvider; +import com.opensymphony.xwork2.ognl.SecurityMemberAccess; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import java.lang.reflect.Member; @@ -45,7 +46,7 @@ public class SecurityMemberAccessProxyTest extends XWorkTestCase { ActionProxy proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setDisallowProxyMemberAccess(true); Member member = proxy.getAction().getClass().getMethod("isExposeProxy"); @@ -58,7 +59,7 @@ public class SecurityMemberAccessProxyTest extends XWorkTestCase { ActionProxy proxy = actionProxyFactory.createActionProxy(null, "chaintoAOPedTestSubBeanAction", null, context); - SecurityMemberAccess sma = new SecurityMemberAccess(false, true); + SecurityMemberAccess sma = new SecurityMemberAccess(true); Member member = proxy.getAction().getClass().getMethod("isExposeProxy");