Skip to content

Commit

Permalink
WW-5381 Introduce extension point for CompoundRootAccessor
Browse files Browse the repository at this point in the history
  • Loading branch information
kusalk committed Dec 29, 2023
1 parent 8f4d9a6 commit aef5d5c
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
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.ognl.accessor.RootAccessor;
import com.opensymphony.xwork2.util.OgnlTextParser;
import com.opensymphony.xwork2.util.PatternMatcher;
import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider;
Expand All @@ -100,8 +100,6 @@
import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionProvider;
import ognl.ClassResolver;
import ognl.PropertyAccessor;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -390,8 +388,7 @@ public static ContainerBuilder bootstrapFactories(ContainerBuilder builder) {
.factory(TextParser.class, OgnlTextParser.class, Scope.SINGLETON)

.factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON)
.factory(PropertyAccessor.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON)
.factory(ClassResolver.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON)
.factory(RootAccessor.class, CompoundRootAccessor.class, Scope.SINGLETON)

.factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON)
.factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.opensymphony.xwork2.inject.Scope;
import com.opensymphony.xwork2.ognl.ObjectProxy;
import com.opensymphony.xwork2.ognl.OgnlReflectionContextFactory;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.ognl.accessor.HttpParametersPropertyAccessor;
import com.opensymphony.xwork2.ognl.accessor.ObjectAccessor;
import com.opensymphony.xwork2.ognl.accessor.ObjectProxyPropertyAccessor;
Expand All @@ -55,7 +54,6 @@
import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.PatternMatcher;
import com.opensymphony.xwork2.util.WildcardHelper;
import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory;
Expand Down Expand Up @@ -145,7 +143,6 @@ public void register(ContainerBuilder builder, LocatableProperties props) throws
.factory(PropertyAccessor.class, Parameter.class.getName(), ParameterPropertyAccessor.class, Scope.SINGLETON)

.factory(MethodAccessor.class, Object.class.getName(), XWorkMethodAccessor.class, Scope.SINGLETON)
.factory(MethodAccessor.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON)

.factory(NullHandler.class, Object.class.getName(), InstantiatingNullHandler.class, Scope.SINGLETON)
.factory(ActionValidatorManager.class, AnnotationActionValidatorManager.class, Scope.SINGLETON)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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.RootAccessor;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.reflection.ReflectionException;
import ognl.ClassResolver;
Expand Down Expand Up @@ -857,7 +858,7 @@ protected Map<String, Object> createDefaultContext(Object root) {

protected Map<String, Object> createDefaultContext(Object root, ClassResolver resolver) {
if (resolver == null) {
resolver = container.getInstance(ClassResolver.class, CompoundRoot.class.getName());
resolver = container.getInstance(RootAccessor.class);
if (resolver == null) {
throw new IllegalStateException("Cannot find ClassResolver");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.opensymphony.xwork2.util.MemberAccessValueStack;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import ognl.ClassResolver;
import ognl.MethodFailedException;
import ognl.NoSuchPropertyException;
import ognl.Ognl;
Expand Down Expand Up @@ -514,7 +513,7 @@ private Object readResolve() {
ActionContext ac = ActionContext.getContext();
Container cont = ac.getContainer();
XWorkConverter xworkConverter = cont.getInstance(XWorkConverter.class);
RootAccessor accessor = (RootAccessor) cont.getInstance(ClassResolver.class, CompoundRoot.class.getName());
RootAccessor accessor = cont.getInstance(RootAccessor.class);
TextProvider prov = cont.getInstance(TextProvider.class, "system");
SecurityMemberAccess sma = cont.getInstance(SecurityMemberAccess.class);
OgnlValueStack aStack = new OgnlValueStack(xworkConverter, accessor, prov, sma);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import com.opensymphony.xwork2.inject.Container;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.ValueStackFactory;
import ognl.ClassResolver;
import ognl.MethodAccessor;
import ognl.OgnlRuntime;
import ognl.PropertyAccessor;
Expand All @@ -50,9 +50,11 @@ protected void setXWorkConverter(XWorkConverter converter) {
this.xworkConverter = converter;
}

@Inject(value = "com.opensymphony.xwork2.util.CompoundRoot")
protected void setClassResolver(ClassResolver classResolver) {
this.compoundRootAccessor = (RootAccessor) classResolver;
@Inject
protected void setCompoundRootAccessor(RootAccessor compoundRootAccessor) {
this.compoundRootAccessor = compoundRootAccessor;
OgnlRuntime.setPropertyAccessor(CompoundRoot.class, compoundRootAccessor);
OgnlRuntime.setMethodAccessor(CompoundRoot.class, compoundRootAccessor);
}

@Inject("system")
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/struts2/StrutsConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ public final class StrutsConstants {

public static final String STRUTS_FREEMARKER_WRAPPER_ALT_MAP = "struts.freemarker.wrapper.altMap";

/** Extension point for the Struts CompoundRootAccessor */
public static final String STRUTS_COMPOUND_ROOT_ACCESSOR = "struts.compoundRootAccessor";

/** The name of the xwork converter implementation */
public static final String STRUTS_XWORKCONVERTER = "struts.xworkConverter";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory;
import com.opensymphony.xwork2.ognl.ExpressionCacheFactory;
import com.opensymphony.xwork2.ognl.SecurityMemberAccess;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker;
Expand Down Expand Up @@ -387,6 +388,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) {

alias(FileManagerFactory.class, StrutsConstants.STRUTS_FILE_MANAGER_FACTORY, builder, props, Scope.SINGLETON);

alias(RootAccessor.class, StrutsConstants.STRUTS_COMPOUND_ROOT_ACCESSOR, builder, props);

alias(XWorkConverter.class, StrutsConstants.STRUTS_XWORKCONVERTER, builder, props);
alias(CollectionConverter.class, StrutsConstants.STRUTS_CONVERTER_COLLECTION, builder, props);
alias(ArrayConverter.class, StrutsConstants.STRUTS_CONVERTER_ARRAY, builder, props);
Expand Down Expand Up @@ -428,8 +431,7 @@ public void register(ContainerBuilder builder, LocatableProperties props) {
/** Checker is used mostly in interceptors, so there be one instance of checker per interceptor with Scope.PROTOTYPE **/
alias(ExcludedPatternsChecker.class, StrutsConstants.STRUTS_EXCLUDED_PATTERNS_CHECKER, builder, props, Scope.PROTOTYPE);
alias(AcceptedPatternsChecker.class, StrutsConstants.STRUTS_ACCEPTED_PATTERNS_CHECKER, builder, props, Scope.PROTOTYPE);
alias(NotExcludedAcceptedPatternsChecker.class, StrutsConstants.STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER
, builder, props, Scope.SINGLETON);
alias(NotExcludedAcceptedPatternsChecker.class, StrutsConstants.STRUTS_NOT_EXCLUDED_ACCEPTED_PATTERNS_CHECKER, builder, props, Scope.SINGLETON);

alias(DateFormatter.class, StrutsConstants.STRUTS_DATE_FORMATTER, builder, props, Scope.SINGLETON);

Expand Down
6 changes: 1 addition & 5 deletions core/src/main/resources/struts-beans.xml
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,9 @@
<bean type="com.opensymphony.xwork2.util.TextParser" name="struts"
class="com.opensymphony.xwork2.util.OgnlTextParser" scope="singleton"/>

<bean type="ognl.ClassResolver" name="com.opensymphony.xwork2.util.CompoundRoot"
<bean type="com.opensymphony.xwork2.ognl.accessor.RootAccessor" name="struts"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>

<bean type="ognl.PropertyAccessor" name="com.opensymphony.xwork2.util.CompoundRoot"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>
<bean type="ognl.PropertyAccessor" name="java.lang.Object"
class="com.opensymphony.xwork2.ognl.accessor.ObjectAccessor"/>
<bean type="ognl.PropertyAccessor" name="java.util.Iterator"
Expand All @@ -202,8 +200,6 @@

<bean type="ognl.MethodAccessor" name="java.lang.Object"
class="com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor"/>
<bean type="ognl.MethodAccessor" name="com.opensymphony.xwork2.util.CompoundRoot"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>

<bean type="org.apache.struts2.dispatcher.StaticContentLoader"
class="org.apache.struts2.dispatcher.DefaultStaticContentLoader" name="struts"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,11 @@
import com.opensymphony.xwork2.ognl.OgnlValueStack;
import com.opensymphony.xwork2.ognl.OgnlValueStackFactory;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
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;
Expand Down Expand Up @@ -961,7 +960,7 @@ public ValueStack createValueStack(ValueStack stack) {
private ValueStack createStubValueStack(final Map<String, Object> actual) {
ValueStack stack = new OgnlValueStack(
container.getInstance(XWorkConverter.class),
(CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()),
(CompoundRootAccessor) container.getInstance(RootAccessor.class),
container.getInstance(TextProvider.class, "system"), true) {
@Override
public void setValue(String expr, Object value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.opensymphony.xwork2.inject.ContainerBuilder;
import com.opensymphony.xwork2.interceptor.ChainingInterceptor;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
import com.opensymphony.xwork2.test.StubConfigurationProvider;
import com.opensymphony.xwork2.test.User;
import com.opensymphony.xwork2.util.Bar;
Expand All @@ -36,7 +37,6 @@
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import ognl.ClassResolver;
import ognl.InappropriateExpressionException;
import ognl.MethodFailedException;
import ognl.NoSuchPropertyException;
Expand Down Expand Up @@ -1629,7 +1629,7 @@ public void testCustomOgnlMapBlocked() throws Exception {
String vulnerableExpr = "#@com.opensymphony.xwork2.ognl.MyCustomMap@{}.get(\"ye\")";
assertEquals("System compromised", ognlUtil.getValue(vulnerableExpr, ognlUtil.createDefaultContext(null), null));

((CompoundRootAccessor) container.getInstance(ClassResolver.class, CompoundRoot.class.getName()))
((CompoundRootAccessor) container.getInstance(RootAccessor.class))
.useDisallowCustomOgnlMap(Boolean.TRUE.toString());

assertThrows(OgnlException.class, () -> ognlUtil.getValue(vulnerableExpr, ognlUtil.createDefaultContext(null), null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,18 @@
import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.inject.ContainerBuilder;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
import com.opensymphony.xwork2.test.StubConfigurationProvider;
import com.opensymphony.xwork2.test.TestBean2;
import com.opensymphony.xwork2.util.Bar;
import com.opensymphony.xwork2.util.BarJunior;
import com.opensymphony.xwork2.util.Cat;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.Dog;
import com.opensymphony.xwork2.util.Foo;
import com.opensymphony.xwork2.util.ValueStackFactory;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import ognl.OgnlException;
import ognl.PropertyAccessor;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.LogEvent;
Expand Down Expand Up @@ -98,7 +97,7 @@ public void setUp() throws Exception {
private OgnlValueStack createValueStack(boolean allowStaticFieldAccess) {
OgnlValueStack stack = new OgnlValueStack(
container.getInstance(XWorkConverter.class),
(CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()),
(CompoundRootAccessor) container.getInstance(RootAccessor.class),
container.getInstance(TextProvider.class, "system"), allowStaticFieldAccess);
container.inject(stack);
return stack;
Expand Down Expand Up @@ -1078,7 +1077,7 @@ public void testConstructorWithAStack() {

OgnlValueStack stack2 = new OgnlValueStack(vs,
container.getInstance(XWorkConverter.class),
(CompoundRootAccessor) container.getInstance(PropertyAccessor.class, CompoundRoot.class.getName()), true);
(CompoundRootAccessor) container.getInstance(RootAccessor.class), true);
container.inject(stack2);

assertEquals(vs.getRoot(), stack2.getRoot());
Expand Down

0 comments on commit aef5d5c

Please sign in to comment.