Skip to content

Commit

Permalink
Merge pull request #715 from apache/WW-5327-non-java-beans
Browse files Browse the repository at this point in the history 
[WW-5327] Stop using JavaBeans notation for setters in SecurityMemberAccess & MemberAccessValueStack
  • Loading branch information
@lukaszlenart
lukaszlenart authored Aug 1, 2023
2 parents 85f7e8a + c7d88be commit 885c500
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 81 deletions.
4 changes: 2 additions & 2 deletions core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ protected void setParameters(final Object action, ValueStack stack, HttpParamete
//block or allow access to properties
//see WW-2761 for more details
MemberAccessValueStack accessValueStack = (MemberAccessValueStack) newStack;
accessValueStack.setAcceptProperties(acceptedPatterns.getAcceptedPatterns());
accessValueStack.setExcludeProperties(excludedPatterns.getExcludedPatterns());
accessValueStack.useAcceptProperties(acceptedPatterns.getAcceptedPatterns());
accessValueStack.useExcludeProperties(excludedPatterns.getExcludedPatterns());
}

for (Map.Entry<String, Parameter> entry : acceptableParameters.entrySet()) {
Expand Down
18 changes: 9 additions & 9 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,22 +872,22 @@ protected Map<String, Object> createDefaultContext(Object root, ClassResolver cl
}

SecurityMemberAccess memberAccess = new SecurityMemberAccess(allowStaticFieldAccess);
memberAccess.setDisallowProxyMemberAccess(disallowProxyMemberAccess);
memberAccess.disallowProxyMemberAccess(disallowProxyMemberAccess);

if (devMode) {
if (!warnReported.get()) {
warnReported.set(true);
LOG.warn("Working in devMode, using devMode excluded classes and packages!");
}
memberAccess.setExcludedClasses(devModeExcludedClasses);
memberAccess.setExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns);
memberAccess.setExcludedPackageNames(devModeExcludedPackageNames);
memberAccess.setExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses);
memberAccess.useExcludedClasses(devModeExcludedClasses);
memberAccess.useExcludedPackageNamePatterns(devModeExcludedPackageNamePatterns);
memberAccess.useExcludedPackageNames(devModeExcludedPackageNames);
memberAccess.useExcludedPackageExemptClasses(devModeExcludedPackageExemptClasses);
} else {
memberAccess.setExcludedClasses(excludedClasses);
memberAccess.setExcludedPackageNamePatterns(excludedPackageNamePatterns);
memberAccess.setExcludedPackageNames(excludedPackageNames);
memberAccess.setExcludedPackageExemptClasses(excludedPackageExemptClasses);
memberAccess.useExcludedClasses(excludedClasses);
memberAccess.useExcludedPackageNamePatterns(excludedPackageNamePatterns);
memberAccess.useExcludedPackageNames(excludedPackageNames);
memberAccess.useExcludedPackageExemptClasses(excludedPackageExemptClasses);
}

return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter);
Expand Down
24 changes: 17 additions & 7 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ protected OgnlValueStack(ValueStack vs, XWorkConverter xworkConverter, CompoundR
@Inject
protected void setOgnlUtil(OgnlUtil ognlUtil) {
this.ognlUtil = ognlUtil;
securityMemberAccess.setExcludedClasses(ognlUtil.getExcludedClasses());
securityMemberAccess.setExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns());
securityMemberAccess.setExcludedPackageNames(ognlUtil.getExcludedPackageNames());
securityMemberAccess.setExcludedPackageExemptClasses(ognlUtil.getExcludedPackageExemptClasses());
securityMemberAccess.setDisallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess());
securityMemberAccess.useExcludedClasses(ognlUtil.getExcludedClasses());
securityMemberAccess.useExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns());
securityMemberAccess.useExcludedPackageNames(ognlUtil.getExcludedPackageNames());
securityMemberAccess.useExcludedPackageExemptClasses(ognlUtil.getExcludedPackageExemptClasses());
securityMemberAccess.disallowProxyMemberAccess(ognlUtil.isDisallowProxyMemberAccess());
}

protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, boolean allowStaticFieldAccess) {
Expand Down Expand Up @@ -482,12 +482,22 @@ public void clearContextValues() {
((OgnlContext) context).getValues().clear();
}

@Deprecated
public void setAcceptProperties(Set<Pattern> acceptedProperties) {
securityMemberAccess.setAcceptProperties(acceptedProperties);
securityMemberAccess.useAcceptProperties(acceptedProperties);
}

public void useAcceptProperties(Set<Pattern> acceptedProperties) {
securityMemberAccess.useAcceptProperties(acceptedProperties);
}

@Deprecated
public void setExcludeProperties(Set<Pattern> excludeProperties) {
securityMemberAccess.setExcludeProperties(excludeProperties);
securityMemberAccess.useExcludeProperties(excludeProperties);
}

public void useExcludeProperties(Set<Pattern> excludeProperties) {
securityMemberAccess.useExcludeProperties(excludeProperties);
}

@Inject
Expand Down
56 changes: 56 additions & 0 deletions core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,31 +299,87 @@ protected boolean isExcluded(String paramName) {
return false;
}

/**
* @deprecated please use {@link #useExcludeProperties(Set)}
*/
@Deprecated
public void setExcludeProperties(Set<Pattern> excludeProperties) {
this.excludeProperties = excludeProperties;
}

public void useExcludeProperties(Set<Pattern> excludeProperties) {
this.excludeProperties = excludeProperties;
}

/**
* @deprecated please use {@link #useAcceptProperties(Set)}
*/
@Deprecated
public void setAcceptProperties(Set<Pattern> acceptedProperties) {
this.acceptProperties = acceptedProperties;
}

public void useAcceptProperties(Set<Pattern> acceptedProperties) {
this.acceptProperties = acceptedProperties;
}

/**
* @deprecated please use {@link #useExcludedClasses(Set)}
*/
@Deprecated
public void setExcludedClasses(Set<Class<?>> excludedClasses) {
this.excludedClasses = excludedClasses;
}

public void useExcludedClasses(Set<Class<?>> excludedClasses) {
this.excludedClasses = excludedClasses;
}

/**
* @deprecated please use {@link #useExcludedPackageNamePatterns(Set)}
*/
@Deprecated
public void setExcludedPackageNamePatterns(Set<Pattern> excludedPackageNamePatterns) {
this.excludedPackageNamePatterns = excludedPackageNamePatterns;
}

public void useExcludedPackageNamePatterns(Set<Pattern> excludedPackageNamePatterns) {
this.excludedPackageNamePatterns = excludedPackageNamePatterns;
}

/**
* @deprecated please use {@link #useExcludedPackageNames(Set)}
*/
@Deprecated
public void setExcludedPackageNames(Set<String> excludedPackageNames) {
this.excludedPackageNames = excludedPackageNames;
}

public void useExcludedPackageNames(Set<String> excludedPackageNames) {
this.excludedPackageNames = excludedPackageNames;
}

/**
* @deprecated please use {@link #useExcludedPackageExemptClasses(Set)}
*/
@Deprecated
public void setExcludedPackageExemptClasses(Set<Class<?>> excludedPackageExemptClasses) {
this.excludedPackageExemptClasses = excludedPackageExemptClasses;
}

public void useExcludedPackageExemptClasses(Set<Class<?>> excludedPackageExemptClasses) {
this.excludedPackageExemptClasses = excludedPackageExemptClasses;
}

/**
* @deprecated please use {@link #disallowProxyMemberAccess(boolean)}
*/
@Deprecated
public void setDisallowProxyMemberAccess(boolean disallowProxyMemberAccess) {
this.disallowProxyMemberAccess = disallowProxyMemberAccess;
}

public void disallowProxyMemberAccess(boolean disallowProxyMemberAccess) {
this.disallowProxyMemberAccess = disallowProxyMemberAccess;
}
}
12 changes: 12 additions & 0 deletions core/src/main/java/com/opensymphony/xwork2/util/MemberAccessValueStack.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,20 @@
*/
public interface MemberAccessValueStack {

/**
* @deprecated please use {@link #useExcludeProperties(Set)}
*/
@Deprecated
void setExcludeProperties(Set<Pattern> excludeProperties);

void useExcludeProperties(Set<Pattern> excludeProperties);

/**
* @deprecated please use {@link #useAcceptProperties(Set)}
*/
@Deprecated
void setAcceptProperties(Set<Pattern> acceptedProperties);

void useAcceptProperties(Set<Pattern> acceptedProperties);

}
12 changes: 7 additions & 5 deletions core/src/main/resources/struts-excluded-classes.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@
<!--
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
Expand All @@ -37,7 +37,9 @@
java.lang.ProcessBuilder,
java.lang.Thread,
sun.misc.Unsafe,
com.opensymphony.xwork2.ActionContext"/>
com.opensymphony.xwork2.ActionContext,
com.opensymphony.xwork2.ognl.SecurityMemberAccess,
com.opensymphony.xwork2.ognl.OgnlValueStack"/>

<constant name="struts.devMode.excludedClasses"
value="
Expand Down
37 changes: 32 additions & 5 deletions core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.location.LocatableProperties;
import com.opensymphony.xwork2.util.reflection.ReflectionContextState;
import java.beans.BeanInfo;
import ognl.InappropriateExpressionException;
import ognl.MethodFailedException;
import ognl.NoSuchPropertyException;
Expand All @@ -48,6 +47,7 @@
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.StrutsException;

import java.beans.BeanInfo;
import java.beans.IntrospectionException;
import java.lang.reflect.Method;
import java.text.DateFormat;
Expand Down Expand Up @@ -877,6 +877,33 @@ public void testStringToLong() {
assertEquals(123, foo.getALong());
}

public void testBeanMapExpressions() throws OgnlException {
Foo foo = new Foo();
ognlUtil.setExcludedClasses(
"com.opensymphony.xwork2.ognl.SecurityMemberAccess"
);

Map<String, Object> context = ognlUtil.createDefaultContext(foo);

String expression = "%{\n" +
"(#request.a=#@org.apache.commons.collections.BeanMap@{}) +\n" +
"(#request.a.setBean(#request.get('struts.valueStack')) == true) +\n" +
"(#request.b=#@org.apache.commons.collections.BeanMap@{}) +\n" +
"(#request.b.setBean(#request.get('a').get('context'))) +\n" +
"(#request.c=#@org.apache.commons.collections.BeanMap@{}) +\n" +
"(#request.c.setBean(#request.get('b').get('memberAccess'))) +\n" +
"(#request.get('c').put('excluded'+'PackageNames',#@org.apache.commons.collections.BeanMap@{}.keySet())) +\n" +
"(#request.get('c').put('excludedClasses',#@org.apache.commons.collections.BeanMap@{}.keySet()))\n" +
"}";

ognlUtil.setValue("title", context, foo, expression);

assertEquals(foo.getTitle(), expression);

SecurityMemberAccess sma = (SecurityMemberAccess) ((OgnlContext) context).getMemberAccess();
assertTrue(sma.isClassExcluded(SecurityMemberAccess.class));
}

public void testNullProperties() {
Foo foo = new Foo();
foo.setALong(88);
Expand Down Expand Up @@ -1834,19 +1861,19 @@ public void testOgnlDefaultCacheFactoryCoverage() {
defaultOgnlCacheFactory.setUseLRUCache("false");
ognlCache = defaultOgnlCacheFactory.buildOgnlCache();
assertNotNull("No param build method result null ?", ognlCache);
assertEquals("Eviction limit for cache mismatches limit for factory ?", 12, ognlCache.getEvictionLimit() );
assertEquals("Eviction limit for cache mismatches limit for factory ?", 12, ognlCache.getEvictionLimit());
ognlCache = defaultOgnlCacheFactory.buildOgnlCache(6, 6, 0.75f, false);
assertNotNull("No param build method result null ?", ognlCache);
assertEquals("Eviction limit for cache mismatches limit for factory ?", 6, ognlCache.getEvictionLimit() );
assertEquals("Eviction limit for cache mismatches limit for factory ?", 6, ognlCache.getEvictionLimit());
// LRU cache
defaultOgnlCacheFactory.setCacheMaxSize("30");
defaultOgnlCacheFactory.setUseLRUCache("true");
ognlCache = defaultOgnlCacheFactory.buildOgnlCache();
assertNotNull("No param build method result null ?", ognlCache);
assertEquals("Eviction limit for cache mismatches limit for factory ?", 30, ognlCache.getEvictionLimit() );
assertEquals("Eviction limit for cache mismatches limit for factory ?", 30, ognlCache.getEvictionLimit());
ognlCache = defaultOgnlCacheFactory.buildOgnlCache(15, 15, 0.75f, false);
assertNotNull("No param build method result null ?", ognlCache);
assertEquals("Eviction limit for cache mismatches limit for factory ?", 15, ognlCache.getEvictionLimit() );
assertEquals("Eviction limit for cache mismatches limit for factory ?", 15, ognlCache.getEvictionLimit());
}

/**
Expand Down
Loading

0 comments on commit 885c500

Please sign in to comment.